feat(tracing): per-step obs wrappers inside business Temporal activities - #491
Open
NiteshDhanpal wants to merge 1 commit into
Open
feat(tracing): per-step obs wrappers inside business Temporal activities#491NiteshDhanpal wants to merge 1 commit into
NiteshDhanpal wants to merge 1 commit into
Conversation
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
On the Temporal / async path,
_begin_obsskipped the per-step obs wrapper for any Temporal activity (Option A) and only stamped the ambientRunActivityspan. 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_activityfollow-up called out in #484.The fix
Option A is only actually required for the SDK's own dispatched
start-span/end-spanactivities (thein_temporal_workflow()path), where start and end run as separate activities that Temporal can route to different workers — so a wrapper opened instart-spancould never be closed byend-span.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 ambientRunActivityspan and closes in-process._begin_obsnow discriminates on activity type via_in_tracing_dispatch_activity()(true only forstart-span/end-span):The now-dead
_in_temporal_activity()is removed (its TODO is exactly what this implements). The bounded_OBS_HANDLESregistry backstops any mis-discrimination.Verified on infra-staging (rocket-mock-async-agent), one async turn
agentex.businesswrapper spansobs_span_idEach 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 underRunActivity, matching the sync path.Tests
Updated
test_temporal_obs_backend.pyfor 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 narrows the Temporal observability special case to the SDK’s dispatched start-span and end-span activities, allowing business activities to create per-step wrappers.
Confidence Score: 4/5
The PR appears safe to merge, with one non-blocking maintainability issue in the activity-name discriminator.
The new branching behavior is consistent with the registered activity names and intended wrapper lifecycle, but duplicating those names as literals creates an avoidable drift point.
Files Needing Attention: src/agentex/lib/core/tracing/trace.py
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[_begin_obs] --> B{Tracing dispatch activity?} B -->|start-span or end-span| C[Tag ambient RunActivity span] C --> D[Return ambient correlation] B -->|Business activity or sync path| E[Open per-step obs wrapper] E --> F[Return wrapper correlation]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(tracing): per-step obs wrappers ins..." | Re-trigger Greptile
Context used:
Learned From
scaleapi/scaleapi#126557