feat(golang): implement /otel_drop_in_extract_and_make_distant_call endpoint#7264
feat(golang): implement /otel_drop_in_extract_and_make_distant_call endpoint#7264genesor wants to merge 2 commits into
Conversation
066cfd4 to
3940445
Compare
|
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3940445a17
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) | ||
| propagator := otel.GetTextMapPropagator() | ||
| ctx := propagator.Extract(r.Context(), propagation.HeaderCarrier(r.Header)) |
There was a problem hiding this comment.
Use a clean context for the OTel extraction
In the Go weblogs this handler runs under the existing Datadog server middleware, and r.Context() already contains the auto-instrumented inbound server span (the existing /make_distant_call code reads it from the same context). Passing that context into OTel extraction means the dd-trace-go OTel bridge will parent otel_extract_distant_call to the auto server span instead of making it the restarted root span with the incoming context as a span link, so the newly enabled Test_ExtractBehavior_Restart_Otel fails its parentID is None and span-link assertions. Use a clean/background context for the OTel extraction in all added Go handlers.
Useful? React with 👍 / 👎.
| return | ||
| } | ||
|
|
||
| otel.SetTextMapPropagator(propagation.NewCompositeTextMapPropagator(propagation.TraceContext{}, propagation.Baggage{})) |
There was a problem hiding this comment.
Honor Datadog extraction order in the OTel endpoint
The conflicting-header cases in Test_ExtractBehavior_Restart_Otel send both Datadog and W3C headers while the scenario configures extraction as datadog,tracecontext,baggage, and the assertions expect the restart span link to come from the Datadog context. This line replaces the OTel propagator with only TraceContext and Baggage, so the OTel extraction ignores x-datadog-* and cannot produce the expected Datadog restart link when the traceparent differs. The same W3C-only propagator was added to each Go weblog handler.
Useful? React with 👍 / 👎.
| propagator := otel.GetTextMapPropagator() | ||
| ctx := propagator.Extract(r.Context(), propagation.HeaderCarrier(r.Header)) | ||
|
|
||
| p := ddotel.NewTracerProvider() |
There was a problem hiding this comment.
Avoid restarting the tracer per request
When this endpoint is hit, ddotel.NewTracerProvider() calls tracer.Start, which replaces any running Datadog tracer; here that happens in the middle of an already instrumented HTTP request, after the server middleware created its span. That can drop or fragment the in-flight trace and races with any concurrent request using the global tracer, making the newly enabled OTel restart tests unreliable across all added Go handlers. Initialize the provider once at startup or reuse an existing provider instead of recreating it inside the handler.
Useful? React with 👍 / 👎.
| p := ddotel.NewTracerProvider() | ||
| otel.SetTracerProvider(p) | ||
| otelTracer := p.Tracer("") | ||
| ctx, span := otelTracer.Start(ctx, "otel_extract_distant_call") |
There was a problem hiding this comment.
Tag the OTel span with the request id
Test_ExtractBehavior_Restart_Otel retrieves spans with interfaces.agent.get_spans_list(self.r), which filters to spans carrying the request id from the request's User-Agent tag. This manually-created OTel span is started without any HTTP/user-agent attributes, so it won't be returned by that filter; the test will inspect the auto-instrumented HTTP server span instead of otel_extract_distant_call and fail the OTel-specific parent/span-link assertions. Add the request user-agent/header tag to the OTel span in each new Go handler.
Useful? React with 👍 / 👎.
|
Motivation
Enable
Test_ExtractBehavior_Restart_Otelfor Go weblogs. This test covers the case where a trace is initiated via the OTel propagation API rather than the auto-instrumented HTTP path, and verifies that span links are correctly created whenDD_TRACE_PROPAGATION_BEHAVIOR_EXTRACT=restart.The endpoint was already implemented for Java, .NET, and Ruby but was missing for Go. Tracked as
missing_feature (OTel extraction endpoint not implemented)inmanifests/golang.yml.Changes
/otel_drop_in_extract_and_make_distant_callendpoint to all 5 Go weblogs (net-http,chi,echo,gin,net-http-orchestrion)otel_extract_distant_callusing the dd-trace-go OTel bridgeurl,status_code,request_headers,response_headersddotel,go.opentelemetry.io/otel,go.opentelemetry.io/otel/propagation) tochi,echo, andginwhich previously had nonemanifests/golang.yml:Test_ExtractBehavior_Restart_Otel: missing_feature→v2.10.0-devReviewer checklist
tests/ormanifests/is modified ? I have the approval from R&P teambuild-XXX-imagelabel is present