Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions src/agentex/lib/core/tracing/obs_ids.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
"""Correlate adk business spans with the active observability trace.

The adk business ``trace_id`` is the agent **task id** (run-level: it spans the
whole agent run across many requests -- task/create, then each message/send turn),
so we must NOT overwrite it with a per-request observability trace_id. Doing so
would collapse the run-level grouping.

Instead, each business span is *tagged* with the active observability
trace_id/span_id (this is the OpenTelemetry "span link" pattern -- correlate
across trace granularities rather than merging them). You can then pivot from a
persisted business span to the Tempo/Datadog trace for the turn that produced it,
while the business trace still groups the entire run by task id.

Source selection follows SGP_OBS_MODE, matching egp-api-backend:
- unset / "dd_only": ddtrace context (current stack)
- "dual": OTel/LGTM preferred, ddtrace fallback
- "lgtm": OTel/LGTM only

This never fabricates ids -- if no observability context is active, it returns
an empty dict and the span is simply not tagged.
"""
from __future__ import annotations

import os
from typing import Dict, Optional, Tuple

__all__ = ("get_obs_mode", "obs_correlation")

DD_ONLY = "dd_only"
DUAL = "dual"
LGTM = "lgtm"
_DEFAULT_MODE = DD_ONLY
_VALID_MODES = (DD_ONLY, DUAL, LGTM)


def get_obs_mode() -> str:
"""Unset/empty/unrecognized -> ``dd_only`` (current behavior)."""
raw = os.getenv("SGP_OBS_MODE")
if not raw:
return _DEFAULT_MODE
mode = raw.strip().lower()
return mode if mode in _VALID_MODES else _DEFAULT_MODE


def _lgtm_ids() -> Optional[Tuple[str, str]]:
try:
from opentelemetry import trace
except ImportError:
return None
ctx = trace.get_current_span().get_span_context()
if ctx and ctx.is_valid:
return format(ctx.trace_id, "032x"), format(ctx.span_id, "016x")
return None


def _ddtrace_ids() -> Optional[Tuple[str, str]]:
try:
from ddtrace import tracer
except ImportError:
return None
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
Comment on lines +61 to +64

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 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.

Suggested change
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!

Fix in Cursor Fix in Claude Code Fix in Codex



def obs_correlation() -> Dict[str, str]:
"""Return ``{"obs.trace_id": ..., "obs.span_id": ...}`` for the active
observability context, or ``{}`` if none is active.

Never fabricates ids -- this is a correlation tag, not the span's id.
"""
mode = get_obs_mode()
if mode == LGTM:
ids = _lgtm_ids()
elif mode == DUAL:
ids = _lgtm_ids() or _ddtrace_ids()
else: # dd_only
ids = _ddtrace_ids()

if not ids:
return {}
return {"obs.trace_id": ids[0], "obs.span_id": ids[1]}
13 changes: 13 additions & 0 deletions src/agentex/lib/core/tracing/trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from agentex.types.span import Span
from agentex.lib.utils.logging import make_logger
from agentex.lib.utils.model_utils import recursive_model_dump
from agentex.lib.core.tracing.obs_ids import obs_correlation
from agentex.lib.core.tracing.span_error import set_span_error
from agentex.lib.core.tracing.span_queue import (
SpanEventType,
Expand Down Expand Up @@ -79,6 +80,12 @@ def start_span(

serialized_input = recursive_model_dump(input) if input else None
serialized_data = recursive_model_dump(data) if data else None
# Tag the business span with the active observability trace_id/span_id
# (OTel/ddtrace) so it can be correlated to the per-turn obs trace. The
# business trace_id stays the run-level task id -- see obs_ids.py.
obs = obs_correlation()
if obs:
serialized_data = {**(serialized_data or {}), **obs}
Comment on lines +86 to +88

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 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.

Fix in Cursor Fix in Claude Code Fix in Codex

id = str(uuid.uuid4())

span = Span(
Expand Down Expand Up @@ -229,6 +236,12 @@ async def start_span(

serialized_input = recursive_model_dump(input) if input else None
serialized_data = recursive_model_dump(data) if data else None
# Tag the business span with the active observability trace_id/span_id
# (OTel/ddtrace) so it can be correlated to the per-turn obs trace. The
# business trace_id stays the run-level task id -- see obs_ids.py.
obs = obs_correlation()
if obs:
serialized_data = {**(serialized_data or {}), **obs}
id = str(uuid.uuid4())

span = Span(
Expand Down
Loading