Skip to content

Commit 0f63017

Browse files
merge: resolve latest next tracing changes
Preserve the newer fail-open observability handling while retaining error category metadata and updated expectations. Co-authored-by: Cursor <cursoragent@cursor.com>
2 parents 6f27da0 + 72732b7 commit 0f63017

16 files changed

Lines changed: 1862 additions & 60 deletions

.stats.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
configured_endpoints: 75
2-
openapi_spec_url: https://storage.googleapis.com/stainless-sdk-openapi-specs/sgp/agentex-sdk-644a4ec06aa1f055c614cbef3379684819a4edd84eeb20d2fb29ae01663622a3.yml
3-
openapi_spec_hash: a6a4dc0c09691ac9783bf38e9653a464
2+
openapi_spec_url: https://storage.googleapis.com/stainless-sdk-openapi-specs/sgp/agentex-sdk-330ce4f0d8feed6caeb73d6b12277cfd89f6ad85535b8c8a6f509743b0b6f8cb.yml
3+
openapi_spec_hash: ed6b33682c511df6de538714c0864aa3
44
config_hash: 593e89b291976a5e84e4c3c3f8324354

src/agentex/lib/adk/_modules/tracing.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,8 +238,22 @@ async def span(
238238
try:
239239
yield span
240240
except Exception as exc:
241+
# Record the failure on the span so the obs span reflects the error
242+
# instead of a false green. Agents use THIS context manager (not
243+
# AsyncTrace.span, which is the only other place set_span_error is
244+
# called), so without this a failed step closes green. end_span (in
245+
# finally) reads it via get_span_error and propagates it to
246+
# close_obs_span. Stored on span.data, so it round-trips through the
247+
# END_SPAN activity on the Temporal path too.
248+
#
249+
# Guard set_span_error itself: it's obs work and must never replace
250+
# the app's exception on the way out. We always re-raise the ORIGINAL
251+
# exc regardless.
241252
if span:
242-
set_span_error(span, exc)
253+
try:
254+
set_span_error(span, exc)
255+
except Exception: # pragma: no cover - obs must not break app path
256+
pass
243257
raise
244258
finally:
245259
if span:

src/agentex/lib/core/clients/temporal/utils.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
from temporalio.converter import PayloadCodec, DataConverter
1010
from temporalio.contrib.pydantic import pydantic_data_converter
1111

12+
from agentex.lib.core.tracing.temporal import temporal_tracing_interceptors
13+
1214
# class DateTimeJSONEncoder(AdvancedJSONEncoder):
1315
# def default(self, o: Any) -> Any:
1416
# if isinstance(o, datetime.datetime):
@@ -136,6 +138,9 @@ async def get_temporal_client(
136138
connect_kwargs: dict[str, Any] = {
137139
"target_host": temporal_address,
138140
"plugins": plugins,
141+
# Propagate OTel trace context on outbound start_workflow / execute_activity
142+
# (enabled by default; AGENTEX_TEMPORAL_TRACE_INTERCEPTOR_ENABLED=false to disable).
143+
"interceptors": temporal_tracing_interceptors(),
139144
}
140145

141146
if data_converter is not None:

src/agentex/lib/core/temporal/services/temporal_task_service.py

Lines changed: 76 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
from __future__ import annotations
22

3+
import sys
34
from typing import Any
45
from datetime import timedelta
6+
from contextlib import contextmanager
7+
from collections.abc import Iterator
58

69
from agentex.types.task import Task
710
from agentex.types.agent import Agent
@@ -13,6 +16,55 @@
1316
from agentex.lib.core.clients.temporal.temporal_client import TemporalClient
1417

1518

19+
@contextmanager
20+
def _acp_dispatch_span(name: str, task_id: str | None = None) -> Iterator[None]:
21+
"""Wrap an ACP -> Temporal dispatch (start_workflow / signal) in an OTel span.
22+
23+
The Temporal OpenTelemetry interceptor propagates trace context by injecting
24+
the CURRENTLY ACTIVE span into the Temporal message headers on the caller
25+
side (``start_workflow`` / ``signal_workflow``); the worker then extracts it
26+
and roots the workflow / activity spans under it. But the ACP server dispatches
27+
from a bare async handler with no active span, so nothing is injected and the
28+
workflow's activities become DETACHED trace roots -- the business work shows up
29+
in Tempo as a fresh trace with no link back to the ``task/create`` /
30+
``event/send`` that triggered it.
31+
32+
Opening a span here gives the interceptor something to inject. It becomes a
33+
child of the ingress request span when one is active (front-of-request
34+
propagation), or a fresh per-turn root otherwise.
35+
36+
Fail-open across the WHOLE obs setup, not just the import: ``get_tracer`` and
37+
entering ``start_as_current_span`` run the sampler and every
38+
``SpanProcessor.on_start`` (the SDK does not guard those), so a broken
39+
provider or a custom sampler/processor that raises would otherwise fail the
40+
dispatch itself. If any of it fails we run the dispatch untraced. The dispatch
41+
body (the ``yield``) is OUTSIDE the guard so its exceptions still propagate.
42+
"""
43+
span_cm = None
44+
try:
45+
from opentelemetry import trace as _otel_trace
46+
47+
tracer = _otel_trace.get_tracer("agentex.acp")
48+
# task_id goes on an attribute, NOT in the span name: a per-task span name is
49+
# high-cardinality and breaks span-name aggregation in Tempo.
50+
attributes = {"agentex.task_id": task_id} if task_id else None
51+
span_cm = tracer.start_as_current_span(name, kind=_otel_trace.SpanKind.PRODUCER, attributes=attributes)
52+
span_cm.__enter__()
53+
except Exception: # pragma: no cover - obs must never break a dispatch
54+
span_cm = None
55+
56+
try:
57+
yield
58+
finally:
59+
if span_cm is not None:
60+
# Pass exc info so the span reflects a failed dispatch; guard __exit__
61+
# so closing the span can never mask the dispatch outcome.
62+
try:
63+
span_cm.__exit__(*sys.exc_info())
64+
except Exception: # pragma: no cover - best-effort close
65+
pass
66+
67+
1668
class TemporalTaskService:
1769
"""
1870
Submits Agent agent_tasks to the async runtime for execution.
@@ -26,7 +78,6 @@ def __init__(
2678
self._temporal_client = temporal_client
2779
self._env_vars = env_vars
2880

29-
3081
async def submit_task(self, agent: Agent, task: Task, params: dict[str, Any] | None) -> str:
3182
"""
3283
Submit a task to the async runtime for execution.
@@ -37,22 +88,19 @@ async def submit_task(self, agent: Agent, task: Task, params: dict[str, Any] | N
3788
# indefinitely, which long-lived chat/session agents rely on). A positive
3889
# value bounds the whole continue-as-new chain's wall-clock lifetime.
3990
timeout_seconds = self._env_vars.WORKFLOW_EXECUTION_TIMEOUT_SECONDS
40-
execution_timeout = (
41-
timedelta(seconds=timeout_seconds)
42-
if timeout_seconds and timeout_seconds > 0
43-
else None
44-
)
45-
return await self._temporal_client.start_workflow(
46-
workflow=self._env_vars.WORKFLOW_NAME,
47-
arg=CreateTaskParams(
48-
agent=agent,
49-
task=task,
50-
params=params,
51-
),
52-
id=task.id,
53-
task_queue=self._env_vars.WORKFLOW_TASK_QUEUE,
54-
execution_timeout=execution_timeout,
55-
)
91+
execution_timeout = timedelta(seconds=timeout_seconds) if timeout_seconds and timeout_seconds > 0 else None
92+
with _acp_dispatch_span("acp.task_create", task_id=task.id):
93+
return await self._temporal_client.start_workflow(
94+
workflow=self._env_vars.WORKFLOW_NAME,
95+
arg=CreateTaskParams(
96+
agent=agent,
97+
task=task,
98+
params=params,
99+
),
100+
id=task.id,
101+
task_queue=self._env_vars.WORKFLOW_TASK_QUEUE,
102+
execution_timeout=execution_timeout,
103+
)
56104

57105
async def get_state(self, task_id: str) -> WorkflowState:
58106
"""
@@ -63,16 +111,17 @@ async def get_state(self, task_id: str) -> WorkflowState:
63111
)
64112

65113
async def send_event(self, agent: Agent, task: Task, event: Event, request: dict | None = None) -> None:
66-
return await self._temporal_client.send_signal(
67-
workflow_id=task.id,
68-
signal=SignalName.RECEIVE_EVENT.value,
69-
payload=SendEventParams(
70-
agent=agent,
71-
task=task,
72-
event=event,
73-
request=request,
74-
).model_dump(),
75-
)
114+
with _acp_dispatch_span("acp.event_send", task_id=task.id):
115+
return await self._temporal_client.send_signal(
116+
workflow_id=task.id,
117+
signal=SignalName.RECEIVE_EVENT.value,
118+
payload=SendEventParams(
119+
agent=agent,
120+
task=task,
121+
event=event,
122+
request=request,
123+
).model_dump(),
124+
)
76125

77126
async def interrupt(self, agent: Agent, task: Task, request: dict | None = None) -> None:
78127
"""Forward a task/interrupt to the running workflow as a dedicated signal.

src/agentex/lib/core/temporal/workers/worker.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
from agentex.lib.utils.logging import make_logger
3131
from agentex.lib.utils.registration import register_agent
32+
from agentex.lib.core.tracing.temporal import temporal_tracing_interceptors
3233
from agentex.lib.environment_variables import EnvironmentVariables
3334
from agentex.lib.core.compat.version_guard import assert_backend_compatible
3435

@@ -126,6 +127,9 @@ async def get_temporal_client(
126127
connect_kwargs: dict[str, Any] = {
127128
"target_host": temporal_address,
128129
"plugins": plugins,
130+
# Propagate OTel trace context on outbound start_workflow / execute_activity
131+
# (enabled by default; AGENTEX_TEMPORAL_TRACE_INTERCEPTOR_ENABLED=false to disable).
132+
"interceptors": temporal_tracing_interceptors(),
129133
}
130134

131135
if data_converter is not None:
@@ -229,7 +233,9 @@ async def run(
229233
max_concurrent_activities=self.max_concurrent_activities,
230234
build_id=str(uuid.uuid4()),
231235
debug_mode=debug_enabled, # Disable deadlock detection in debug mode
232-
interceptors=self.interceptors, # Pass interceptors to Worker
236+
# Tracing interceptor OUTERMOST so business interceptors (and the spans
237+
# they create) nest under the propagated workflow/activity span.
238+
interceptors=[*temporal_tracing_interceptors(), *self.interceptors],
233239
)
234240

235241
logger.info(f"Starting workers for task queue: {self.task_queue}")

src/agentex/lib/core/tracing/obs_ids.py

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,20 @@
1111
persisted business span to the Tempo/Datadog trace for the turn that produced it,
1212
while the business trace still groups the entire run by task id.
1313
14-
Source selection follows SGP_OBS_MODE, matching egp-api-backend:
14+
Source selection follows SGP_OBS_MODE:
1515
- unset / "dd_only": ddtrace context (current stack)
16-
- "dual": OTel/LGTM preferred, ddtrace fallback
1716
- "lgtm": OTel/LGTM only
1817
18+
("dual" was removed: co-resident ddtrace+OTel can't be bridged in-process --
19+
you can't run ddtrace-run and the OTel operator's auto-instrumentation in the
20+
same process, and DD_TRACE_OTEL_ENABLED yields a single tracer with nothing to
21+
bridge. Two-backend export is a collector fan-out under "lgtm", not a mode here.
22+
An unrecognized SGP_OBS_MODE -- including a stale "dual" -- degrades to dd_only.)
23+
1924
This never fabricates ids -- if no observability context is active, it returns
2025
an empty dict and the span is simply not tagged.
2126
"""
27+
2228
from __future__ import annotations
2329

2430
import os
@@ -27,10 +33,9 @@
2733
__all__ = ("get_obs_mode", "obs_correlation")
2834

2935
DD_ONLY = "dd_only"
30-
DUAL = "dual"
3136
LGTM = "lgtm"
3237
_DEFAULT_MODE = DD_ONLY
33-
_VALID_MODES = (DD_ONLY, DUAL, LGTM)
38+
_VALID_MODES = (DD_ONLY, LGTM)
3439

3540

3641
def get_obs_mode() -> str:
@@ -64,20 +69,31 @@ def _ddtrace_ids() -> Optional[Tuple[str, str]]:
6469
return None
6570

6671

67-
def obs_correlation() -> Dict[str, str]:
68-
"""Return ``{"obs.trace_id": ..., "obs.span_id": ...}`` for the active
72+
def obs_correlation(prefer_otel: bool = False) -> Dict[str, str]:
73+
"""Return ``{"obs_trace_id": ..., "obs_span_id": ...}`` for the active
6974
observability context, or ``{}`` if none is active.
7075
76+
These land in the business span's ``data`` -> egp ``operation_metadata``
77+
(an existing JSONB column, GIN-indexed) -> ClickHouse ``metadata_raw``, so
78+
the correlation edge needs no schema migration. Underscored keys (not
79+
dotted) keep them addressable via Postgres JSON paths
80+
(``operation_metadata->>'obs_trace_id'``).
81+
82+
``prefer_otel``: on the Temporal path the active span is the temporalio OTel
83+
``TracingInterceptor`` span regardless of ``SGP_OBS_MODE``, so callers there
84+
read OTel first (falling back to ddtrace) -- otherwise the default ``dd_only``
85+
mode would read ids for an unrelated ddtrace trace, not the activity span.
86+
7187
Never fabricates ids -- this is a correlation tag, not the span's id.
7288
"""
73-
mode = get_obs_mode()
74-
if mode == LGTM:
75-
ids = _lgtm_ids()
76-
elif mode == DUAL:
77-
ids = _lgtm_ids() or _ddtrace_ids()
78-
else: # dd_only
79-
ids = _ddtrace_ids()
89+
try:
90+
if prefer_otel:
91+
ids = _lgtm_ids() or _ddtrace_ids()
92+
else:
93+
ids = _lgtm_ids() if get_obs_mode() == LGTM else _ddtrace_ids()
94+
except Exception: # obs must never fail an app call
95+
return {}
8096

8197
if not ids:
8298
return {}
83-
return {"obs.trace_id": ids[0], "obs.span_id": ids[1]}
99+
return {"obs_trace_id": ids[0], "obs_span_id": ids[1]}

0 commit comments

Comments
 (0)