aigw: default session.id request header mapping#1808
Merged
mathetake merged 2 commits intoenvoyproxy:mainfrom Jan 26, 2026
Merged
aigw: default session.id request header mapping#1808mathetake merged 2 commits intoenvoyproxy:mainfrom
mathetake merged 2 commits intoenvoyproxy:mainfrom
Conversation
f5e8e58 to
8c3307e
Compare
Contributor
Author
|
after this I would like to switch to otlp hopefully if EG completes merging my outstanding PR 🤞 |
8c3307e to
fe73225
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1808 +/- ##
==========================================
+ Coverage 84.08% 84.12% +0.04%
==========================================
Files 118 119 +1
Lines 13235 13283 +48
==========================================
+ Hits 11128 11174 +46
- Misses 1434 1435 +1
- Partials 673 674 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fe73225 to
1faa0a6
Compare
95bd3b8 to
6dc63e3
Compare
| - AIGW_DEBUG | ||
| # session.id is used in logs and traces (not metrics; high-cardinality) | ||
| - OTEL_AIGW_REQUEST_HEADER_ATTRIBUTES=x-user-id:user.id | ||
| - OTEL_AIGW_LOG_REQUEST_HEADER_ATTRIBUTES=x-session-id:session.id |
Contributor
Author
There was a problem hiding this comment.
spans and logs are fine with request scope, so that's why session.id makes sense
| // cmdRun corresponds to `aigw run` command. | ||
| cmdRun struct { | ||
| Debug bool `help:"Enable debug logging emitted to stderr."` | ||
| Debug bool `env:"AIGW_DEBUG" help:"Enable debug logging emitted to stderr."` |
Contributor
Author
There was a problem hiding this comment.
docker compose up cannot add args, so our instructions were busted. env is the easy way out
| // ``` | ||
| // Then, with the following BackendTrafficPolicy of Envoy Gateway, you can have three | ||
| // rate limit buckets for each unique x-user-id header value. One bucket is for the input token, | ||
| // rate limit buckets for each unique x-tenant-id header value. One bucket is for the input token, |
Contributor
Author
There was a problem hiding this comment.
our examples used a combination of x-user-id, x-team-id, x-tenant and the most common attribute is x-tenant-id so settled on this for a coarse grained example
mathetake
approved these changes
Jan 26, 2026
Member
mathetake
left a comment
There was a problem hiding this comment.
Not following on why the pointer to String is necessary but harmless it seems
Member
|
E2E rate limit failure seems legit |
Contributor
Author
it is to know the difference between not set and set. for example, if you want no defaults, you set the header to empty. Without handling tri-state boolean it would be hard to unset everything. |
Signed-off-by: Adrian Cole <adrian@tetrate.io>
6dc63e3 to
084c668
Compare
Contributor
Author
yep sorry about that, missed a find/replace |
Contributor
Author
|
updated the PR desc on how to clear the default (via |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Default span/log request‑header mappings to
agent-session-id:session.idso agent frameworks like Goose get session correlation with zero config, while still allowing explicit overrides (different mapping or empty to disable). Metrics never default to session IDs because they are high cardinality.The default mapping is in the new ENV variable
OTEL_AIGW_REQUEST_HEADER_ATTRIBUTES, so those who want noagent-session-id:session.idshould setOTEL_AIGW_REQUEST_HEADER_ATTRIBUTES=(empty string) to clear it.Refactor request‑header mapping handling so defaults/merging live only in extproc;
aigwand controller/helm just pass flags through. Ordering is normalized everywhere (request → span → metrics → log) and docs/examples describe defaults without explicitly settingagent-session-id:session.id.Related Issues/PRs (if applicable)
Related: #1797
Special notes for reviewers (if applicable)
Ran the examples/goose with OTEL console env and --debug.
Since goose now propagates agent-session-id by default, we can see in the telemetry
agent-session-id=20260123_19:MCP span (
tools/list) showingsession.idset from the header:MCP access log showing
session.idon a tool call:LLM access log showing
session.idon a chat completion:Minor improvements:
AIGW_DEBUGso docker compose examples can actually show debug output