Skip to content

Commit 9466b3b

Browse files
NiteshDhanpalclaude
andcommitted
fix(tracing): skip obs wrapper inside Temporal activities
start_span/end_span run as SEPARATE Temporal activities (START_SPAN/END_SPAN) that Temporal can route to different worker processes. The obs wrapper handle is stored in a process-local module dict, so on a multi-replica fleet the END lands on a different worker than the START: the handle is never popped (leak / OOM risk) and the wrapper span is never ended (dangling obs_span_id in Tempo). Inside a Temporal activity, skip opening our own wrapper and instead stamp the reverse tag onto the interceptor-propagated ambient span (tag_ambient_obs_span) and read forward ids via obs_correlation(). Trace-level correlation is preserved via the Temporal OTel TracingInterceptor (#485); the per-step named wrapper and TurnTrace RETRY/ASYNC roll-up are deferred (see TODO(obs-followup)). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 68d0342 commit 9466b3b

2 files changed

Lines changed: 102 additions & 5 deletions

File tree

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

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131

3232
from agentex.lib.core.tracing.obs_ids import LGTM, get_obs_mode
3333

34-
__all__ = ("ObsSpanHandle", "open_obs_span", "close_obs_span")
34+
__all__ = ("ObsSpanHandle", "open_obs_span", "close_obs_span", "tag_ambient_obs_span")
3535

3636
# Instrumentation scope name so these wrapper spans are identifiable in Tempo/DD.
3737
_TRACER_NAME = "agentex.business"
@@ -175,6 +175,43 @@ def open_obs_span(
175175
return None
176176

177177

178+
def tag_ambient_obs_span(
179+
business_span_id: Optional[str] = None,
180+
business_trace_id: Optional[str] = None,
181+
) -> None:
182+
"""Stamp the reverse tag onto the CURRENTLY ACTIVE obs span -- without opening
183+
a new one.
184+
185+
Used on the Temporal path (see ``trace._in_temporal_activity``): there we must
186+
NOT open our own wrapper span, because start_span/end_span run as separate
187+
activities on possibly different workers and the wrapper could never be
188+
closed. Instead we lean on the span the Temporal OTel ``TracingInterceptor``
189+
already made active for this activity and just add
190+
``agentex.business_span_id`` / ``agentex.business_trace_id`` so the obs -> business
191+
pivot still works. Best-effort; never raises."""
192+
try:
193+
if get_obs_mode() == LGTM:
194+
from opentelemetry import trace
195+
196+
span = trace.get_current_span()
197+
if span is not None and span.get_span_context().is_valid:
198+
if business_span_id:
199+
span.set_attribute(_ATTR_BUSINESS_SPAN_ID, business_span_id)
200+
if business_trace_id:
201+
span.set_attribute(_ATTR_BUSINESS_TRACE_ID, business_trace_id)
202+
else:
203+
from ddtrace.trace import tracer
204+
205+
span = tracer.current_span()
206+
if span is not None:
207+
if business_span_id:
208+
span.set_tag(_ATTR_BUSINESS_SPAN_ID, business_span_id)
209+
if business_trace_id:
210+
span.set_tag(_ATTR_BUSINESS_TRACE_ID, business_trace_id)
211+
except Exception: # pragma: no cover - best-effort; obs must never break a call
212+
pass
213+
214+
178215
def close_obs_span(
179216
handle: Optional[ObsSpanHandle],
180217
error: Optional[Dict[str, str]] = None,

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

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
ObsSpanHandle,
1717
open_obs_span,
1818
close_obs_span,
19+
tag_ambient_obs_span,
1920
)
2021
from agentex.lib.core.tracing.span_error import get_span_error, set_span_error
2122
from agentex.lib.core.tracing.span_queue import (
@@ -42,6 +43,43 @@
4243
_OBS_HANDLES: dict[str, ObsSpanHandle] = {}
4344

4445

46+
def _in_temporal_activity() -> bool:
47+
"""True when executing inside a Temporal activity.
48+
49+
On the Temporal path ``start_span`` and ``end_span`` run as SEPARATE
50+
activities (START_SPAN / END_SPAN) that Temporal can route to DIFFERENT
51+
worker processes. A wrapper obs span opened in the START_SPAN activity could
52+
therefore never be closed by END_SPAN -- its handle lives in another
53+
process's ``_OBS_HANDLES`` -- so it would leak (unbounded, OOM risk) and its
54+
persisted ``obs_span_id`` would dangle (the span is never .end()ed, so never
55+
exported to Tempo).
56+
57+
So inside an activity we do NOT open our own wrapper. We lean on the span the
58+
Temporal OTel ``TracingInterceptor`` (see ``core/tracing/temporal.py`` +
59+
scale-agentex-python#485) already made active for this activity -- which is
60+
rooted under the turn's propagated trace -- and merely stamp the reverse tag
61+
onto it (``tag_ambient_obs_span``). That keeps trace-level correlation with
62+
no cross-process handle to leak.
63+
64+
Never raises; returns False when temporalio isn't importable.
65+
66+
TODO(obs-followup): this intentionally drops the *named per-step* wrapper on
67+
the Temporal path (obs_span_id becomes the ambient activity span, not a
68+
step-named span) and does NOT add TurnTrace RETRY/ASYNC roll-up -- retried
69+
turns still surface as N unlinked spans. Follow-up diff should (a) optionally
70+
materialize a self-contained named wrapper inside a single activity using the
71+
span's own start/end timestamps, and (b) build the TurnTrace roll-up.
72+
Test-later: on a multi-replica worker fleet, assert _OBS_HANDLES stays
73+
bounded (no leak / OOM) and that obs_trace_id resolves to the turn trace.
74+
"""
75+
try:
76+
from temporalio import activity
77+
78+
return activity.in_activity()
79+
except Exception:
80+
return False
81+
82+
4583
class Trace:
4684
"""
4785
Trace is a wrapper around the Agentex API for tracing.
@@ -105,9 +143,20 @@ def start_span(
105143
# so you can pivot obs -> business in Tempo/DD. Falls back to the ambient
106144
# obs context (ddtrace) when not in lgtm mode. Business trace_id stays the
107145
# run-level task id.
146+
#
147+
# Inside a Temporal activity we skip the wrapper entirely and only tag the
148+
# interceptor-propagated ambient span: opening a wrapper there would leak,
149+
# since start_span / end_span run as separate activities on possibly
150+
# different workers and the handle could never be closed. See
151+
# _in_temporal_activity().
108152
id = str(uuid.uuid4())
109-
obs_handle = open_obs_span(name, business_span_id=id, business_trace_id=self.trace_id)
110-
obs = obs_handle.correlation if obs_handle is not None else obs_correlation()
153+
if _in_temporal_activity():
154+
tag_ambient_obs_span(business_span_id=id, business_trace_id=self.trace_id)
155+
obs_handle = None
156+
obs = obs_correlation()
157+
else:
158+
obs_handle = open_obs_span(name, business_span_id=id, business_trace_id=self.trace_id)
159+
obs = obs_handle.correlation if obs_handle is not None else obs_correlation()
111160
if obs:
112161
serialized_data = {**(serialized_data or {}), **obs}
113162

@@ -274,9 +323,20 @@ async def start_span(
274323
# so you can pivot obs -> business in Tempo/DD. Falls back to the ambient
275324
# obs context (ddtrace) when not in lgtm mode. Business trace_id stays the
276325
# run-level task id.
326+
#
327+
# Inside a Temporal activity we skip the wrapper entirely and only tag the
328+
# interceptor-propagated ambient span: opening a wrapper there would leak,
329+
# since start_span / end_span run as separate activities on possibly
330+
# different workers and the handle could never be closed. See
331+
# _in_temporal_activity().
277332
id = str(uuid.uuid4())
278-
obs_handle = open_obs_span(name, business_span_id=id, business_trace_id=self.trace_id)
279-
obs = obs_handle.correlation if obs_handle is not None else obs_correlation()
333+
if _in_temporal_activity():
334+
tag_ambient_obs_span(business_span_id=id, business_trace_id=self.trace_id)
335+
obs_handle = None
336+
obs = obs_correlation()
337+
else:
338+
obs_handle = open_obs_span(name, business_span_id=id, business_trace_id=self.trace_id)
339+
obs = obs_handle.correlation if obs_handle is not None else obs_correlation()
280340
if obs:
281341
serialized_data = {**(serialized_data or {}), **obs}
282342

0 commit comments

Comments
 (0)