fix(policy-engine): extract trace context from request headers - #3061
fix(policy-engine): extract trace context from request headers#3061renuka-fernando wants to merge 3 commits into
Conversation
The request's W3C traceparent is delivered by Envoy inside the RequestHeaders ProcessingRequest body, not as ext_proc gRPC stream metadata. Reading it from stream metadata always yielded an empty span context, logging "No valid trace context extracted" per request and starting an unparented root trace every time. - Build the trace-context carrier from the HTTP request headers and create the root span lazily on the first message, so child spans correctly continue the router's trace - Demote the not-found log from Info to Debug (an ordinary condition, e.g. Envoy tracing disabled) - A stream failing before the first message still starts a root span so stream-level errors remain observable Signed-off-by: Renuka Fernando <renukapiyumal@gmail.com>
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe policy engine extracts W3C trace context from Envoy HTTP headers. Root spans are created lazily from the first request’s context. Carrier-based tracing tests replace gRPC metadata-based tests. ChangesTrace context propagation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Envoy
participant ExtProcStream
participant traceContextCarrier
participant ExtractTraceContext
participant RootSpan
Envoy->>ExtProcStream: Send first HTTP request
ExtProcStream->>traceContextCarrier: Read traceparent and tracestate
traceContextCarrier-->>ExtProcStream: Return propagation carrier
ExtProcStream->>ExtractTraceContext: Extract W3C trace context
ExtractTraceContext-->>ExtProcStream: Return extracted context
ExtProcStream->>RootSpan: Create root span
ExtProcStream->>RootSpan: Reuse span for later messages
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gateway/gateway-runtime/policy-engine/internal/kernel/extproc.go`:
- Around line 113-124: The traceContextCarrier header loop currently overwrites
repeated tracestate values. Update the carrier assignment to append subsequent
tracestate values with a comma while preserving field order, without changing
traceparent handling; add a regression test covering two tracestate headers and
their combined value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 18881ea1-47f0-4935-8e64-a1f34ffbb0d7
📒 Files selected for processing (4)
gateway/gateway-runtime/policy-engine/internal/kernel/extproc.gogateway/gateway-runtime/policy-engine/internal/kernel/extproc_test.gogateway/gateway-runtime/policy-engine/internal/tracing/tracer.gogateway/gateway-runtime/policy-engine/internal/tracing/tracer_test.go
W3C Trace Context permits tracestate to be split across multiple header fields; a receiver must combine them into a single comma-separated value in field order rather than overwrite. traceContextCarrier now appends repeated tracestate values (traceparent stays last-wins, as multiple traceparent is malformed). Adds a regression test with two tracestate headers. Addresses PR review feedback. Signed-off-by: Renuka Fernando <renukapiyumal@gmail.com>
The tracer is a no-op when tracing is disabled, so building the carrier and running the propagator per request was wasted work on the ext_proc hot path. Gate it behind the (previously unused) tracingConfig.Enabled flag. - Store tracingEnabled on the server from tracingConfig.Enabled - Add startRootSpan helper that skips carrier build + extraction when disabled and de-duplicates the two root-span creation sites Signed-off-by: Renuka Fernando <renukapiyumal@gmail.com>
1 participant
Purpose
The policy engine logged
No valid trace context extractedon every request and never continued the router's distributed trace. The extraction readtraceparent/tracestatefrom the ext_proc gRPC stream metadata, but the downstream request's W3C trace context is delivered by Envoy inside theRequestHeadersProcessingRequestbody — not as stream metadata. The long-lived bidirectional ext_proc stream also sets its gRPC metadata only once at establishment, so it can never carry a per-requesttraceparent. As a result the propagator always produced an empty (zero-ID) span context whoseIsValid()is false, causing the noisy log and an unparented root trace for every request.Goals
Extract the trace context from the correct source (the HTTP request headers) so the policy engine's spans continue the incoming trace, and stop logging a benign condition at
Infoon the per-request path.Approach
RequestHeadersphase (newtraceContextCarrierhelper), copying onlytraceparent/tracestate, lowercasing keys and preferringRawValuewith aValuefallback.tracing.ExtractTraceContextto accept apropagation.TextMapCarrierinstead of reading gRPC stream metadata, keeping thetracingpackage free of Envoy proto types.InfotoDebug— it is an ordinary condition (e.g. Envoy tracing disabled or no inboundtraceparent).User stories
N/A
Documentation
N/A — internal observability fix with no user-facing API or configuration change.
Automation tests
Security checks
Samples
N/A
Related PRs
N/A
Test environment
Go (policy-engine module), macOS (darwin/arm64). Verified via
go build ./...,go vet ./..., andgo test ./....Related Issues
N/A
Checklist