fix(observability): extract upstream traceparent, re-parent server span, and inject traceparent on egress#159
Open
gyliu513 wants to merge 2 commits into
Open
fix(observability): extract upstream traceparent, re-parent server span, and inject traceparent on egress#159gyliu513 wants to merge 2 commits into
gyliu513 wants to merge 2 commits into
Conversation
github-actions
Bot
added
size/L
shaneutt
reviewed
Jun 15, 2026
|
|
||
| // Inject the active trace context into the egress headers so the next | ||
| // filter in the chain is parented to this span rather than the upstream one. | ||
| traceCarrier := propagation.MapCarrier{} |
Member
There was a problem hiding this comment.
Do we want to normalize headers from the tracecarrier as LC, as a "just in case"? 🤔
Member
Author
There was a problem hiding this comment.
Good call, it's a no-op for the default W3C propagator (traceparent/tracestate are already lowercase), but SetHeader writes the key verbatim into the Envoy header mutation, and over HTTP/2 a non-W3C/custom propagator could emit mixed-case keys.
Lowercasing is cheap defensive insurance, so I've normalized the carrier keys with strings.ToLower.
shmuelk
reviewed
Jun 18, 2026
| for key, value := range traceCarrier { | ||
| // Normalize keys to lowercase; HTTP/2 (used by ext_proc) requires | ||
| // lowercase header names, and a non-W3C propagator may emit mixed case. | ||
| reqCtx.Request.SetHeader(strings.ToLower(key), value) |
Collaborator
There was a problem hiding this comment.
This will cause duplicate keys, differenciated only by their case.
Member
Author
There was a problem hiding this comment.
Thanks @shmuelk , I will normalize incoming header keys to lowercase at ingestion
…an, and inject traceparent on egress Signed-off-by: Guangya Liu <gyliu513@gmail.com>
Signed-off-by: Guangya Liu <gyliu513@gmail.com>
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.
What type of PR is this?
/kind feature
What this PR does / why we need it:
IPP runs as an Envoy
ext_procfilter, sequentially on the same Envoy HTTP filter chain as EPP. Like EPP (fixed in llm-d/llm-d-router#1514), IPP started its server span at the top ofProcess()before the request headers were received, so it never extracted the upstreamtraceparentand started a new root trace per request.In addition, unlike EPP, IPP did not inject
traceparenton egress: it already has a HeaderMutation pipeline, but trace context was never written into it, so a downstreamext_procfilter in the chain could not see IPP's span as its parent.Which issue(s) this PR fixes:
Fixes #157
Release note (write
NONEif no user-facing change):