Skip to content

feat(tracing): per-step obs wrappers inside business Temporal activities - #491

Merged
NiteshDhanpal merged 9 commits into
nextfrom
feat/obs-perstep-async-wrapper
Aug 11, 2026
Merged

feat(tracing): per-step obs wrappers inside business Temporal activities#491
NiteshDhanpal merged 9 commits into
nextfrom
feat/obs-perstep-async-wrapper

Conversation

@NiteshDhanpal

@NiteshDhanpal NiteshDhanpal commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What & why

Two ways to give a business step an obs presence on the Temporal path:

  • Option A (tag the ambient span) — don't open an obs span for the step; just stamp the business ids (agentex.business_trace_id / agentex.business_span_id) onto the span the OTel TracingInterceptor already made active (the ambient RunActivity span). Correlation still works, but the step gets no span of its own.
  • Per-step wrapper — open a dedicated, step-named obs span, nest that step's downstream hops (httpx / DB) under it, and close it when the step ends. This is the 1:1 mapping the sync path already does.

Before this change, _begin_obs used Option A for any Temporal activity, so every business span in an async turn collapsed onto one obs span, whereas the sync path gives each step its own (1:1). This implements the _in_temporal_activity follow-up called out in #484.

The fix

Option A is only actually required for the SDK's own dispatched start-span / end-span activities (the in_temporal_workflow() path), where start and end run as separate activities that Temporal can route to different workers — so a wrapper opened in start-span could never be closed by end-span (it would leak and its obs_span_id would dangle).

Inside a business activity (an agent turn's own adk.tracing.span, e.g. process_mortgage_turn), start and end run in the same process, so a wrapper is safe there: it nests under the interceptor's ambient RunActivity span and closes in-process.

_begin_obs now discriminates on activity type via _in_tracing_dispatch_activity() (true only for start-span/end-span):

  • dispatched tracing activity → tag the ambient span (Option A, unchanged),
  • everything else (sync or a business activity) → open a real per-step wrapper.

The now-dead _in_temporal_activity() is removed (its TODO is exactly what this implements). The bounded _OBS_HANDLES registry backstops any mis-discrimination.

Why it matters

With per-step wrappers on the async path you can tell which hop belongs to which step (hops nest under their step, not one giant activity span), each step's persisted obs_span_id points at its own step-named span instead of a shared generic one, and async finally produces the same trace shape as sync.

Span volume on retries

A retried business activity now emits a full per-step wrapper set per attempt, each nested under that attempt's RunActivity (previously they collapsed onto the ambient activity span). This is the intended 1:1 shape, but until the TurnTrace roll-up lands a retried turn surfaces as N per-attempt span sets rather than one PRIMARY + N RETRY view. That roll-up and a multi-replica bounded-_OBS_HANDLES check are tracked as TODO(obs-followup) in trace.py.

Verified on infra-staging (rocket-mock-async-agent), one async turn

before after
Tempo agentex.business wrapper spans 0 52
Tempo spans carrying the reverse tag 1 52
Postgres business spans 52 52
Postgres distinct obs_span_id 1 52 (1:1)

Each step (mortgage.advisor.turn, classify_intent, retrieve_docs.kb_query, authz.check.*, tool.*, telemetry.shard.*, synthesis.draft_reply) now gets its own obs span nested under RunActivity, matching the sync path.

Tests

Updated test_temporal_obs_backend.py for the new discriminator, plus two tests asserting: inside a dispatch activity → tag ambient (no wrapper); otherwise → open a wrapper.

🤖 Generated with Claude Code

Greptile Summary

The PR gives business steps inside Temporal activities dedicated observability wrapper spans while retaining ambient-span tagging for the SDK’s separately dispatched tracing activities.

  • Adds activity-type discrimination between tracing-dispatch and business activities.
  • Prefers the Temporal OTel context when opening wrappers and reading correlation IDs.
  • Adds backend-drift diagnostics and focused tests for dispatch and business-activity behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/agentex/lib/core/tracing/trace.py Distinguishes dispatched tracing activities from business activities and opens per-step wrappers only where start and end execute in-process.
src/agentex/lib/core/tracing/obs_span.py Adds Temporal-aware OTel-first wrapper selection with a best-effort ddtrace fallback.
src/agentex/lib/core/tracing/obs_ids.py Renames the OTel preference signal and adds deduplicated backend-drift warnings.
tests/test_temporal_obs_backend.py Covers dispatch discrimination, OTel-first business-activity correlation, and ambient tagging behavior.
tests/lib/core/tracing/test_obs_span.py Updates tracing integration tests for the keyword-based correlation interface.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Begin business span] --> B{Tracing dispatch activity?}
    B -->|Yes| C[Tag ambient Temporal OTel span]
    C --> D[Persist ambient correlation IDs]
    B -->|No| E{Inside any Temporal activity?}
    E -->|Yes| F[Open per-step OTel wrapper]
    E -->|No| G[Open wrapper selected by configured mode]
    F --> H[Persist wrapper correlation IDs]
    G --> H
    H --> I[Register handle]
    I --> J[End business span]
    J --> K[Close and remove wrapper handle]
Loading

Reviews (8): Last reviewed commit: "refactor(tracing): probe expected backen..." | Re-trigger Greptile

…ies (1:1)

Previously _begin_obs skipped the obs wrapper for ANY Temporal activity (Option
A) and only stamped the ambient RunActivity span, so all business spans in a turn
collapsed onto ONE obs span (52:1). But inside a *business* activity, start_span
and end_span run in the SAME process, so a wrapper is safe there. Option A is
only required for the SDK's own dispatched START_SPAN/END_SPAN activities (the
in_temporal_workflow path), where start and end are separate activities on
possibly different workers.

Discriminate on activity type: _in_tracing_dispatch_activity() is true only for
the "start-span"/"end-span" activities. For everything else (sync, or a business
activity) open a real per-step wrapper — it nests under the interceptor's ambient
RunActivity span and closes in-process, giving each business span its own obs
span (1:1), matching the sync path. The bounded _OBS_HANDLES registry backstops
any mis-discrimination.
@NiteshDhanpal
NiteshDhanpal force-pushed the feat/obs-perstep-async-wrapper branch from 732d739 to 5decbec Compare August 10, 2026 22:18
NiteshDhanpal and others added 7 commits August 10, 2026 15:50
…tivities

In dd_only (the default), the wrapper branch of _begin_obs read ddtrace for
both open_obs_span and the obs_correlation fallback. Inside a Temporal worker
there is no ddtrace request context, so open_obs_span returned None and the
fallback obs_correlation() returned {} -- every business span in an async turn
persisted with no obs_trace_id/obs_span_id. The ambient span in an activity is
the temporalio OTel TracingInterceptor span regardless of SGP_OBS_MODE (same
reasoning already applied to the dispatch/tag branch).

Fix: thread prefer_otel through the wrapper branch, keyed on "in any Temporal
activity" (restored _in_temporal_activity). open_obs_span gains a prefer_otel
param that opens an OTel wrapper first (falling back to ddtrace). This also
restores real per-step OTel wrapper spans in dd_only business activities, not
just the ids.

Adds a dd_only-inside-a-business-activity regression test (asserts OTel ids win
over ddtrace, not empty); the earlier verification ran in lgtm so it missed this.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The prior tests stubbed _in_tracing_dispatch_activity wholesale, so the actual
`activity.info().activity_type in ("start-span", "end-span")` comparison -- the
one line preventing a cross-worker handle leak inside START_SPAN -- had no
coverage. Add tests that fake activity.info():
- start-span / end-span -> True (asserted against TracingActivityName.value, so
  this fails if the enum ever drifts from the strings hardcoded in trace.py),
- a business activity type (process_mortgage_turn) -> False,
- not in an activity -> False (in_activity guard short-circuits before info()),
plus a small _in_temporal_activity() truth-table test.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Compare activity.info().activity_type against TracingActivityName.START_SPAN /
END_SPAN instead of the string literals "start-span" / "end-span", so the check
can't silently drift from the enum that actually names the activities
(@activity.defn(name=TracingActivityName.START_SPAN)).

Uses a lazy import inside the function to avoid the activities -> TracingService
-> AsyncTracer -> trace import cycle (the reason literals were used originally);
at call time, inside an activity, the module graph is fully loaded so the import
is safe. activity_type round-trips as the enum's str value, which a str-Enum
member compares equal to.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…emporal_activity

Deleting _in_temporal_activity earlier dropped the TODO it carried. This PR does
item (a) of it (the named per-step wrapper); the other two are still open, so
re-home them on _begin_obs where the per-step wrapper decision now lives:
  (1) TurnTrace RETRY/ASYNC roll-up -- retried turns currently surface as N
      per-attempt span sets, not one PRIMARY + N RETRY view.
  (2) multi-replica bounded-_OBS_HANDLES + obs_trace_id-resolves-to-turn check.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… drift

prefer_otel read like a per-call override of SGP_OBS_MODE, which undercut the
mode as the source of truth. Rename to expect_otel: it is not a bypass but a
context-derived expectation -- "the expected backend here is OTel" -- true on
the Temporal path, where the interceptor span is OTel regardless of mode.

Keep the mode authoritative and make config-vs-reality drift observable instead
of silently absorbing it: warn_on_backend_drift() logs once (deduped per
direction) when the expected backend has no active span but the other one does
(e.g. dd_only configured but the live span is OTel). Fail-open -- it only warns;
the caller still reads and falls back, so no correlation is lost. Wired into
_begin_obs on both the dispatch and business-activity paths.

Tests: the rename, plus two drift tests (warns once on mismatch; silent when the
expected backend is the live one).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… guard

The enum import sat above the in_activity() check, so it ran on every start_span
-- including the pure-sync ACP path that never touches Temporal. That pulled the
whole temporal activities module graph into workflow-less processes, made the
docstring's "inside an activity" safety justification untrue for the path
actually taken, and meant a broken import would silently return False (disabling
dispatch discrimination) even on the sync path. Move it below the guard: sync
never runs it, and inside an activity the graph is loaded so the lazy import
stays safe.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment thread src/agentex/lib/core/tracing/obs_ids.py Outdated
Comment thread src/agentex/lib/core/tracing/obs_span.py Outdated
…ift; rewrap docstring

- warn_on_backend_drift probes the expected backend first and returns early when
  it is live, so the healthy common path skips the second probe -- and avoids
  re-attempting the import of a backend that isn't installed (failed imports
  aren't cached in sys.modules, so the finder cost otherwise recurs every span).
  Behavior is unchanged.
- Rewrap the tag_ambient_obs_span docstring paragraph a prior edit left with one
  overlong line.

Addresses review nits.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@NiteshDhanpal
NiteshDhanpal merged commit 5c7ee10 into next Aug 11, 2026
48 checks passed
@NiteshDhanpal
NiteshDhanpal deleted the feat/obs-perstep-async-wrapper branch August 11, 2026 05:20
@stainless-app stainless-app Bot mentioned this pull request Aug 11, 2026
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.

3 participants