[WIP] feat(tracing): correlate business spans with active observability trace#465
[WIP] feat(tracing): correlate business spans with active observability trace#465NiteshDhanpal wants to merge 1 commit into
Conversation
| obs = obs_correlation() | ||
| if obs: | ||
| serialized_data = {**(serialized_data or {}), **obs} |
There was a problem hiding this comment.
serialized_data can be a list when data is passed as list[dict[str, Any]] — recursive_model_dump returns a list for that case, and a non-empty list is truthy, so serialized_data or {} evaluates to the list. {**<list>, **obs} then raises TypeError: argument after ** must be a mapping, not list at span-creation time whenever an obs context is active. The same pattern is duplicated in AsyncTrace.start_span (line 243–244). A safe guard would be: if obs and isinstance(serialized_data, dict): serialized_data = {**serialized_data, **obs} (or similarly initialise to {} when None).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agentex/lib/core/tracing/trace.py
Line: 86-88
Comment:
`serialized_data` can be a list when `data` is passed as `list[dict[str, Any]]` — `recursive_model_dump` returns a list for that case, and a non-empty list is truthy, so `serialized_data or {}` evaluates to the list. `{**<list>, **obs}` then raises `TypeError: argument after ** must be a mapping, not list` at span-creation time whenever an obs context is active. The same pattern is duplicated in `AsyncTrace.start_span` (line 243–244). A safe guard would be: `if obs and isinstance(serialized_data, dict): serialized_data = {**serialized_data, **obs}` (or similarly initialise to `{}` when `None`).
How can I resolve this? If you propose a fix, please make it concise.| ctx = tracer.current_trace_context() | ||
| if ctx and ctx.trace_id: | ||
| return format(ctx.trace_id, "032x"), format(ctx.span_id or 0, "016x") | ||
| return None |
There was a problem hiding this comment.
ctx.span_id or 0 falls back to 0 when span_id is None or zero, which formats to "0000000000000000" — the OTel invalid/null sentinel. This produces a correlation pair where obs.trace_id is populated but obs.span_id is the null sentinel, which consumers may interpret as "no active span." If span_id is absent, skipping the correlation entirely is more correct.
| ctx = tracer.current_trace_context() | |
| if ctx and ctx.trace_id: | |
| return format(ctx.trace_id, "032x"), format(ctx.span_id or 0, "016x") | |
| return None | |
| ctx = tracer.current_trace_context() | |
| if ctx and ctx.trace_id and ctx.span_id: | |
| return format(ctx.trace_id, "032x"), format(ctx.span_id, "016x") | |
| return None |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agentex/lib/core/tracing/obs_ids.py
Line: 61-64
Comment:
`ctx.span_id or 0` falls back to `0` when `span_id` is `None` or zero, which formats to `"0000000000000000"` — the OTel invalid/null sentinel. This produces a correlation pair where `obs.trace_id` is populated but `obs.span_id` is the null sentinel, which consumers may interpret as "no active span." If `span_id` is absent, skipping the correlation entirely is more correct.
```suggestion
ctx = tracer.current_trace_context()
if ctx and ctx.trace_id and ctx.span_id:
return format(ctx.trace_id, "032x"), format(ctx.span_id, "016x")
return None
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Tag each adk business span at creation time with the active observability trace_id/span_id (OpenTelemetry preferred, ddtrace fallback), selected via SGP_OBS_MODE. The business trace_id stays the run-level task id; the obs ids are attached as span data (obs.trace_id/obs.span_id) using the span-link pattern so a span can be pivoted to its per-turn Tempo/Datadog trace without collapsing the agent-run grouping. Capture is emit-time (in start_span), never at ingest, so batched /v5/spans/batch flushing cannot shatter one run across N obs traces. Returns no tag when no obs context is active (safe no-op; default SGP_OBS_MODE=dd_only). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
a3a7ae1 to
cd91ba7
Compare
Stack (1/3)
What this PR does
Tags each adk business span, at creation time, with the active observability
trace_id/span_id(OpenTelemetry preferred, ddtrace fallback), so a persisted span can be pivoted to its per-turn Tempo/Datadog trace.core/tracing/obs_ids.py:get_obs_mode()(readsSGP_OBS_MODE; unset/empty/unrecognized →dd_only) andobs_correlation()returning{"obs.trace_id", "obs.span_id"}for the active obs context, or{}if none.core/tracing/trace.py: bothstart_span(sync + async) mergeobs_correlation()into the span'sdata.Design notes
trace_idis unchanged — it stays the run-level task id. We use the OTel span-link pattern (correlate across granularities) instead of overwriting the trace_id, which would shatter one agent run across N per-request obs traces./v5/spans/batchflushing cannot mis-group spans.SGP_OBS_MODEunset →dd_only; returns no tag when no obs context is active. Behavior only changes once an obs context exists (see PRs 2 & 3).Verification
py_compile+ unit smoke: default modedd_only, empty dict when no active context, correct id widths (32-hex / 16-hex).Greptile Summary
This PR introduces obs-correlation tagging: at span creation time, each business span is stamped with the active observability
trace_id/span_id(OTel preferred, ddtrace fallback) so a persisted span can be pivoted to its per-turn Tempo/Datadog trace without overwriting the run-level businesstrace_id.obs_ids.py(new):get_obs_mode()readsSGP_OBS_MODE(defaultdd_only) andobs_correlation()returns{\"obs.trace_id\", \"obs.span_id\"}or{}when no context is active.trace.py: BothTrace.start_spanandAsyncTrace.start_spannow mergeobs_correlation()intoserialized_dataat emission time, ensuring the correct per-request context is captured rather than waiting for batch flush.startsWith('stainless-sdks/')prefix check.Confidence Score: 4/5
The obs-correlation logic runs in the hot path of every span creation; an unhandled exception from ddtrace or OTel would silently crash all span creation for that agent run.
Two issues in obs_ids.py directly affect span creation reliability: the narrow except ImportError leaves runtime errors from ddtrace/OTel unhandled, and the span_id or 0 null-sentinel path (flagged in a prior thread). Both live in the critical start_span path shared by every caller of Trace and AsyncTrace.
src/agentex/lib/core/tracing/obs_ids.py and src/agentex/lib/core/tracing/trace.py need attention before merge.
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Caller participant Trace/AsyncTrace participant obs_ids participant OTel/ddtrace Caller->>Trace/AsyncTrace: "start_span(name, data=...)" Trace/AsyncTrace->>obs_ids: obs_correlation() obs_ids->>obs_ids: get_obs_mode() reads SGP_OBS_MODE alt "mode == lgtm" obs_ids->>OTel/ddtrace: _lgtm_ids() get_current_span() else "mode == dual" obs_ids->>OTel/ddtrace: _lgtm_ids() or _ddtrace_ids() else "mode == dd_only default" obs_ids->>OTel/ddtrace: _ddtrace_ids() current_trace_context() end OTel/ddtrace-->>obs_ids: (trace_id, span_id) or None obs_ids-->>Trace/AsyncTrace: obs.trace_id and obs.span_id or empty dict Trace/AsyncTrace->>Trace/AsyncTrace: merge obs into serialized_data Trace/AsyncTrace->>Trace/AsyncTrace: create Span with merged data Trace/AsyncTrace-->>Caller: Span%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Caller participant Trace/AsyncTrace participant obs_ids participant OTel/ddtrace Caller->>Trace/AsyncTrace: "start_span(name, data=...)" Trace/AsyncTrace->>obs_ids: obs_correlation() obs_ids->>obs_ids: get_obs_mode() reads SGP_OBS_MODE alt "mode == lgtm" obs_ids->>OTel/ddtrace: _lgtm_ids() get_current_span() else "mode == dual" obs_ids->>OTel/ddtrace: _lgtm_ids() or _ddtrace_ids() else "mode == dd_only default" obs_ids->>OTel/ddtrace: _ddtrace_ids() current_trace_context() end OTel/ddtrace-->>obs_ids: (trace_id, span_id) or None obs_ids-->>Trace/AsyncTrace: obs.trace_id and obs.span_id or empty dict Trace/AsyncTrace->>Trace/AsyncTrace: merge obs into serialized_data Trace/AsyncTrace->>Trace/AsyncTrace: create Span with merged data Trace/AsyncTrace-->>Caller: SpanReviews (2): Last reviewed commit: "feat(tracing): correlate business spans ..." | Re-trigger Greptile