Skip to content

feat(tracing): make ddtrace adopt the OTel trace id in dual mode#471

Open
NiteshDhanpal wants to merge 1 commit into
obs/03-temporal-otel-interceptorfrom
obs/04-ddtrace-dual-sync
Open

feat(tracing): make ddtrace adopt the OTel trace id in dual mode#471
NiteshDhanpal wants to merge 1 commit into
obs/03-temporal-otel-interceptorfrom
obs/04-ddtrace-dual-sync

Conversation

@NiteshDhanpal

@NiteshDhanpal NiteshDhanpal commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Stack (4/4)

  1. correlate business spans with active obs trace (feat(tracing): correlate business spans with active observability trace #465)
  2. W3C trace context at FastACP ingress (feat(tracing): establish W3C trace context at FastACP ingress #466)
  3. Temporal OTel interceptor (feat(tracing): propagate trace context into Temporal via OTel interceptor #467)
  4. make ddtrace adopt the OTel trace id in dual mode ← this PR

Based on obs/03. Review/merge after #467.

What

Adds sync_ddtrace_to_lgtm() — in SGP_OBS_MODE=dual, reads the active OpenTelemetry span context and activates a matching ddtrace Context, so ddtrace-emitted (best-effort Datadog) spans share the same trace_id as the OTel/LGTM trace. Called at FastACP ingress right after the W3C context is attached.

Why

In dual mode we keep ddtrace running for best-effort Datadog but let OTel own HTTP (DD_TRACE_FASTAPI_ENABLED=false / DD_TRACE_STARLETTE_ENABLED=false). Without this, ddtrace's outbound spans start their own trace id → the two backends show different ids. This bridges them: OTel is canonical, ddtrace follows.

Mechanism is the explicit context-sync (not DD_TRACE_OTEL_ENABLED, which conflicts with the operator-injected OTel SDK). Mirrors the egp-api-backend sync_ddtrace_to_lgtm().

Safety

  • No-op outside dual mode, or when OTel/ddtrace is unavailable, or no OTel span is active. Never raises.
  • 128-bit alignment assumed (DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED=true) so ids match.

Verify

py_compile + smoke: no-op in lgtm/dd_only, and in dual with no active OTel span (safe fallback).

Greptile Summary

This PR adds sync_ddtrace_to_lgtm() to bridge OTel and ddtrace in dual mode: at FastACP ingress, after the W3C trace context is attached, the active OTel span's trace_id/span_id are propagated into a ddtrace Context so both backends share the same trace identifier.

  • obs_ids.py: New sync_ddtrace_to_lgtm() — guards on mode, handles ddtrace 2.x/3.x import paths, checks span validity, and activates the ddtrace context. All error paths return early without raising.
  • base_acp_server.py: One-line call in RequestIDMiddleware.__call__ immediately after _attach_incoming_trace_context, before await self.app(...). The OTel context uses a token/detach cleanup but the ddtrace context has no corresponding cleanup, creating an asymmetry that can leak across HTTP/1.1 keep-alive requests on the same connection.

Confidence Score: 3/5

The ddtrace context activated in the middleware is never cleaned up, so a stale trace id from one request can bleed into the next request on the same keep-alive connection if that subsequent request carries no OTel context.

The context-sync logic itself is sound and the no-op guards are correct, but the missing ddtrace context reset in the finally block creates a real cross-request contamination path. The OTel side correctly uses a token/detach pattern; the ddtrace side needs the same treatment before this is safe to merge.

Both changed files warrant a second look: base_acp_server.py for the missing cleanup in RequestIDMiddleware, and obs_ids.py for propagating sampling priority when activating the ddtrace Context.

Important Files Changed

Filename Overview
src/agentex/lib/core/tracing/obs_ids.py Adds sync_ddtrace_to_lgtm() that reads the active OTel span context and activates a matching ddtrace Context in dual mode; no-op guard logic and import fallbacks are correct, but sampling priority is not forwarded to ddtrace.
src/agentex/lib/sdk/fastacp/base/base_acp_server.py Calls _sync_ddtrace_to_lgtm() after OTel context attachment in the middleware; OTel context is properly detached in finally but the activated ddtrace context has no corresponding cleanup, risking context leakage across keep-alive requests.

Sequence Diagram

sequenceDiagram
    participant C as Client
    participant M as RequestIDMiddleware
    participant OTel as OpenTelemetry Context
    participant DD as ddtrace Context
    participant App as ASGI App

    C->>M: HTTP request (traceparent header)
    M->>OTel: _attach_incoming_trace_context(scope)
    Note over OTel: OTel context attached,<br/>token returned for cleanup
    M->>DD: _sync_ddtrace_to_lgtm()
    Note over DD: Reads active OTel span context<br/>Activates matching ddtrace Context<br/>(same trace_id, span_id)
    M->>App: await self.app(scope, receive, send)
    App-->>M: response
    M->>OTel: _detach_trace_context(token)
    Note over OTel: OTel context cleaned up
    Note over DD: ddtrace context NOT cleaned up<br/>(persists in asyncio task)
    M-->>C: HTTP response
Loading

Comments Outside Diff (1)

  1. src/agentex/lib/sdk/fastacp/base/base_acp_server.py, line 104-111 (link)

    P1 Ddtrace context not cleaned up, risks leaking across keep-alive requests

    _attach_incoming_trace_context returns a detach token that is cleaned up in finally. _sync_ddtrace_to_lgtm() activates a ddtrace Context but never restores the previous one. In uvicorn's HTTP/1.1 keep-alive loop, all requests on the same connection share a single asyncio Task (and therefore the same contextvars scope). Request A sets ddtrace context to trace_X; the finally block detaches the OTel context but leaves trace_X active in ddtrace. Request B arrives with no traceparent_sync_ddtrace_to_lgtm() becomes a no-op (no valid OTel span), and B's ddtrace spans are incorrectly attributed to trace_X.

    The fix mirrors the OTel pattern: capture tracer.context_provider.get() before activation and restore it in finally.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/agentex/lib/sdk/fastacp/base/base_acp_server.py
    Line: 104-111
    
    Comment:
    **Ddtrace context not cleaned up, risks leaking across keep-alive requests**
    
    `_attach_incoming_trace_context` returns a detach token that is cleaned up in `finally`. `_sync_ddtrace_to_lgtm()` activates a ddtrace `Context` but never restores the previous one. In uvicorn's HTTP/1.1 keep-alive loop, all requests on the same connection share a single asyncio `Task` (and therefore the same `contextvars` scope). Request A sets ddtrace context to `trace_X`; the `finally` block detaches the OTel context but leaves `trace_X` active in ddtrace. Request B arrives with no `traceparent``_sync_ddtrace_to_lgtm()` becomes a no-op (no valid OTel span), and B's ddtrace spans are incorrectly attributed to `trace_X`.
    
    The fix mirrors the OTel pattern: capture `tracer.context_provider.get()` before activation and restore it in `finally`.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Cursor Fix in Claude Code Fix in Codex

Fix All in Cursor Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/agentex/lib/sdk/fastacp/base/base_acp_server.py:104-111
**Ddtrace context not cleaned up, risks leaking across keep-alive requests**

`_attach_incoming_trace_context` returns a detach token that is cleaned up in `finally`. `_sync_ddtrace_to_lgtm()` activates a ddtrace `Context` but never restores the previous one. In uvicorn's HTTP/1.1 keep-alive loop, all requests on the same connection share a single asyncio `Task` (and therefore the same `contextvars` scope). Request A sets ddtrace context to `trace_X`; the `finally` block detaches the OTel context but leaves `trace_X` active in ddtrace. Request B arrives with no `traceparent``_sync_ddtrace_to_lgtm()` becomes a no-op (no valid OTel span), and B's ddtrace spans are incorrectly attributed to `trace_X`.

The fix mirrors the OTel pattern: capture `tracer.context_provider.get()` before activation and restore it in `finally`.

### Issue 2 of 2
src/agentex/lib/core/tracing/obs_ids.py:111
**Sampling priority not forwarded, ddtrace may make an independent sampling decision**

`Context(trace_id=sc.trace_id, span_id=sc.span_id)` leaves `sampling_priority` unset. ddtrace will then apply its own sampler for this activated context, which can differ from the sampling decision already made by the OTel SDK for the same trace. In practice this means a span that OTel keeps may be dropped by Datadog (or vice-versa), making the two backends show different coverage for the same trace id even when `DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED=true` guarantees matching ids. If the egp-api-backend reference sets `sampling_priority=USER_KEEP` (or forwards it from the W3C `tracestate`), doing the same here would ensure consistent sampling across both backends.

Reviews (1): Last reviewed commit: "feat(tracing): make ddtrace adopt the OT..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Add sync_ddtrace_to_lgtm(): in SGP_OBS_MODE=dual, read the active OpenTelemetry
span context and activate a matching ddtrace Context, so ddtrace-emitted
(best-effort Datadog) spans share the SAME trace_id as the OTel/LGTM trace --
one trace visible in both backends. Called at FastACP ingress after the W3C
context is attached.

No-op outside dual mode, or when OTel/ddtrace is unavailable or no OTel span is
active. Pairs with OTel-owns-HTTP (DD_TRACE_FASTAPI/STARLETTE_ENABLED=false).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
sc = trace.get_current_span().get_span_context()
if not (sc and sc.is_valid):
return
tracer.context_provider.activate(Context(trace_id=sc.trace_id, span_id=sc.span_id))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Sampling priority not forwarded, ddtrace may make an independent sampling decision

Context(trace_id=sc.trace_id, span_id=sc.span_id) leaves sampling_priority unset. ddtrace will then apply its own sampler for this activated context, which can differ from the sampling decision already made by the OTel SDK for the same trace. In practice this means a span that OTel keeps may be dropped by Datadog (or vice-versa), making the two backends show different coverage for the same trace id even when DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED=true guarantees matching ids. If the egp-api-backend reference sets sampling_priority=USER_KEEP (or forwards it from the W3C tracestate), doing the same here would ensure consistent sampling across both backends.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agentex/lib/core/tracing/obs_ids.py
Line: 111

Comment:
**Sampling priority not forwarded, ddtrace may make an independent sampling decision**

`Context(trace_id=sc.trace_id, span_id=sc.span_id)` leaves `sampling_priority` unset. ddtrace will then apply its own sampler for this activated context, which can differ from the sampling decision already made by the OTel SDK for the same trace. In practice this means a span that OTel keeps may be dropped by Datadog (or vice-versa), making the two backends show different coverage for the same trace id even when `DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED=true` guarantees matching ids. If the egp-api-backend reference sets `sampling_priority=USER_KEEP` (or forwards it from the W3C `tracestate`), doing the same here would ensure consistent sampling across both backends.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Cursor Fix in Claude Code Fix in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant