feat(tracing): delegate obs edge to sgp-obs + wire init_tracing at ACP startup - #493
Open
NiteshDhanpal wants to merge 12 commits into
Open
feat(tracing): delegate obs edge to sgp-obs + wire init_tracing at ACP startup#493NiteshDhanpal wants to merge 12 commits into
NiteshDhanpal wants to merge 12 commits into
Conversation
#485) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…r span (#484) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…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.
…P startup Replace the SDK's inline observability edge (#484/#485/#490/#491) with thin shims that delegate to the shared sgp-obs library: - obs_ids.py / obs_span.py / temporal.py / trace.py: delegate correlation, the per-step wrapper span, the begin-obs decision, and the Temporal interceptors to sgp_obs.traces backends + Correlator. - adk/pyproject.toml: add sgp-obs==0.2.0rc1; root pyproject: add the CodeArtifact `scale` index + explicit source for sgp-obs. Also wire sgp_obs.traces.init_tracing() into the BaseACPServer lifespan. Without it, get_tracer() resolves to the API-default ProxyTracerProvider: obs wrapper spans never record or export, so no span carrying the <source>.business_trace_id reverse anchor reaches the collector. init_tracing adopts an app-installed provider if present, else installs one with the OTLP exporter (fail-open). Validated end-to-end in sgp-dev (rocket-mock + audit): agent registers, 20/20 business spans correlate (obs ids in span metadata), per-step wrapper spans record with the reverse anchor, and force_flush to the collector succeeds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ctly The obs_ids/obs_span modules were pure backward-compat adapters: they preserved the SDK's legacy surface (get_obs_mode/obs_correlation/open_obs_span/ close_obs_span/tag_ambient_obs_span/ObsSpanHandle + DD_ONLY/LGTM strings) by translating to/from sgp_obs's own types on every call. That adapter layer is the bulk of the delegation and buys nothing — trace.py is the only consumer. Delete both modules and have trace.py talk to sgp_obs directly: build the SpanRequest/BusinessRef, run the Correlator, map SGP_OBS_MODE -> ObsMode, and close via the sgp_obs handle (SpanError). Also removes the now-dead _in_tracing_dispatch_activity (the wrapper-vs-ambient decision lives entirely in the Correlator via is_dispatch_boundary). Tests: delete the shim-targeted suites (test_obs_ids/test_obs_span/ test_obs_span_fallback/test_temporal_obs_backend) — that behavior is owned by sgp_obs's own tests now. Keep the registry/error/interceptor tests (they cover trace.py's own logic); the registry test uses a minimal fake handle. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_acp_dispatch_span was per-service boundary glue living in the SDK. The ACP->Temporal dispatch is a transport boundary, not service logic, so it now lives once in the library (sgp_obs.traces.dispatch_span). Replace the 45-line function + its sys/contextlib imports with two `with dispatch_span(...)` calls, passing the business id as an attribute (agentex.task_id) so the generic library stays source-agnostic. Bumps the sgp-obs pin to 0.3.0 (adds dispatch_span + the ALWAYS_ON sampler fix). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Now that init_tracing auto-instruments outbound httpx, the span-export calls get traced themselves and nest back into the trace being exported — PUT /v5/spans/batch (SGP) and POST /spans (Agentex), plus the egp auth/db subtree a propagated traceparent drags in. Wrap both processors' async exports in sgp_obs.traces.suppress_instrumentation so the export makes no span and injects no traceparent. Verified in sgp-dev: export-pollution spans went 51 -> 0 while each business step still shows its real work (e.g. run_agent_streamed -> the LLM call). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…on by default Every agentex agent makes outbound HTTP (LLM/egp), so httpx instrumentation is universal — pull the extra in the SDK so init_tracing's egress instrumentation (and the export suppression that pairs with it) actually functions without each agent opting in. DB instrumentation (sgp-obs[db]) stays per-agent opt-in. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wrap both completion_stream (sync) and acompletion_stream (async) with sgp_obs.traces.instrument_stream so every litellm streaming call emits one gen_ai.chat span with TTFT/TTAT events + the decode-window timing (tps, tpot, output tokens) — filling the "span for TTFT but nothing for the streaming" gap for non-Temporal / litellm agents. Provider extractors count content-bearing deltas; fail-open, chunks pass through unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wire SGPLLMGateway.{completion_stream,acompletion_stream} through
sgp_obs.traces.instrument_stream so scale-gp streaming gets the same gen_ai.chat
span (TTFT/TTAT + decode-window timing) as litellm. Both gateways yield the same
OpenAI-shaped Completion chunk, so the extractors + attribute builder move to a
shared _stream_obs module (litellm refactored to use it; scale-gp tags
gen_ai.system=scale-gp). Streaming instrumentation is now uniform across adapters.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
After init_tracing, register sgp_obs's openai-agents tracing bridge so a Runner turn's internal phases (generation/tool/handoff) show up in Tempo under the business step instead of a dark gap. Fail-open no-op for non-openai-agents agents. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
Moves the SDK's inline observability edge (the correlation/wrapper-span/temporal-interceptor work from #484/#485/#490/#491) out into the shared sgp-obs library, so the SDK just delegates. Net diff vs
mainis the delegated implementation — the intermediate inline code (added in the #484/#485/#491 commits, which were never merged tomain) is replaced by thin shims.obs_ids.py/obs_span.py/temporal.py/trace.py— delegate correlation, the per-step wrapper span, the begin-obs wrapper-vs-ambient decision, and the Temporal interceptors tosgp_obs.tracesbackends +Correlator.adk/pyproject.toml— addsgp-obs==0.2.0rc1; rootpyproject.toml— add the CodeArtifactscaleindex + an explicit source (published wheel just pins the version; consumers resolve from their own index/PyPI).base_acp_server.py— wiresgp_obs.traces.init_tracing()into the ACP lifespan.Why init_tracing matters
Without it,
get_tracer()resolves to the API-defaultProxyTracerProvider: obs wrapper spans never record or export. The forward edge (obs ids in business-span metadata) still fills from ambient/propagated W3C context, but no obs span carrying the<source>.business_trace_idreverse anchor ever reaches the collector, so the business→obs pivot has nothing to land on.init_tracingadopts an app-installed provider if present, else installs one with the OTLP exporter. Fail-open (never raises out of startup).Validation (sgp-dev, rocket-mock + audit)
init_tracing, per-step wrapper spans record (distinct span ids, not a reused ambient context) and carryagentex.business_trace_id/agentex.business_span_id.force_flushto the OTLP collector returnsTrue(spans ship).🤖 Generated with Claude Code
Greptile Summary
The PR delegates observability correlation, wrapper spans, streaming instrumentation, and Temporal propagation to
sgp-obs, while initializing tracing during ACP startup.sgp-obsdependency and private-index resolution configuration.Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
sgp-obs, adds bounded handle tracking, and makes processor and queue failures fail-open.sgp-obstracing dependency used by the delegated implementation.Sequence Diagram
sequenceDiagram participant C as ACP Client participant A as ACP Server participant O as sgp-obs participant T as Temporal participant W as Worker/Activity participant P as Span Processors C->>A: Request with traceparent A->>O: Continue inbound context A->>O: Initialize tracing provider A->>T: Start workflow or send signal T->>W: Propagate OTel context W->>O: Open correlated wrapper span W->>P: Persist business span O-->>O: Export observability spanReviews (7): Last reviewed commit: "feat(tracing): install the openai-agents..." | Re-trigger Greptile
Context used (5)