| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
…r span Model the business<->observability correlation edge on the emit side, with no schema migration (rides the existing operation_metadata JSONB). - obs_ids: standardize the correlation keys to obs_trace_id/obs_span_id (underscored, JSON-path friendly); remove the non-working `dual` mode (it required an in-process ddtrace<->OTel bridge that can't exist -- you can't run ddtrace-run and the OTel operator together, and DD_TRACE_OTEL_ENABLED is a single tracer). `dual` now safely degrades to dd_only. Harden obs_correlation to never raise. - obs_span (new): when the SDK creates a business span it opens a dedicated obs span named for that step and makes it active, so obs_span_id is stable and meaningful (a named span with its httpx call nested underneath) instead of an arbitrary innermost instrumentation span. Backends: OTel in lgtm; ddtrace in dd_only but only when a request trace is already active (avoids orphan root traces in un-instrumented agents). Reverse tag: stamps agentex.business_span_id / agentex.business_trace_id onto the obs span so the pivot is bidirectional. - trace: wire the wrapper into start_span/end_span (sync + async). Observability can never fail an app call -- every path is guarded and is a no-op when the tracer isn't configured. - tests: obs_ids (mode degrade + keys), obs_span (both backends, non-interference, never-fails, reverse tag), and the 3-turn mortgage Turn-2 example pinned as an executable contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Observability review (P1): the dedicated wrapper obs span was ended/finished without recording failure, so a failed business step (e.g. chat_completion) showed green in Tempo/DD -- violating "observe both success and failure" and undercutting the meaningful-obs_span_id goal. close_obs_span now takes the business span's error (from get_span_error) and marks the obs span before closing: - OTel: span.set_status(Status(ERROR, msg)) + error.type attribute - ddtrace: span.error = 1 + error.type / error.message tags end_span passes error=get_span_error(span) on both sync and async paths. Success path is unchanged (no status set). Guarded so error-marking can never break the close. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ddtrace start_span does not auto-parent (unlike OTel): start_span(name) mints a new ROOT trace every call, so a turn's business spans scattered across N Datadog traces (verified live: 52 spans -> 52 distinct obs_trace_ids). Pass child_of=current_trace_context() so wrappers nest under the request/turn trace and roll up into one trace; obs_span_id stays distinct per step. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The obs wrapper span (opened in start_span, ended in end_span) was tracked in an INSTANCE dict (self._obs_handles). But TracingService creates a fresh Trace object for every call -- self._tracer.trace(trace_id) in BOTH start_span and end_span -- so end_span ran on a different instance with an empty dict: the handle was never found, close_obs_span(None) was a no-op, and the OTel/ddtrace wrapper span was never .end()ed. Consequence in lgtm mode: the wrapper span records and its ids are written to Postgres (read at start), but since Simple/Batch span processors only export on span end, the span never reaches Tempo -- the turn trace was silently missing while everything looked correct (provider ours, sampler ALWAYS_ON, recording=True, ids stored). Fix: move the handle registry to module level, keyed by the uuid4 span id, so it survives across Trace instances. Adds a regression test that starts a span on one Trace instance and ends it on another. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fixes the failing lint job on this PR (ruff I001 import ordering + format) on the obs_ids / obs_span / trace correlation-edge files and their tests. No logic change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Annotate fake ModuleType stubs as Any (pyright rejects attribute assignment on ModuleType), widen the mock 'record' dicts to dict[str, Any], assert the Optional resolver returns before unpacking, and narrow span.data with isinstance before subscripting. Clears the pyright errors failing the lint job. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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>
…races The Temporal OTel interceptor propagates trace context by injecting the active span into the Temporal message headers on start_workflow/signal, so the worker roots the workflow+activity under it. But the ACP server dispatches from a bare async handler with no active span -> nothing injected -> the workflow's activity becomes a detached trace root, disconnected from the task/create / event/send that triggered it. Wrap submit_task and send_event in an OTel span (agentex.acp) so the interceptor has a context to inject. Child of the ingress request span when one is active (front-of-request propagation), else a per-turn root. Fail-open. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
acp.task_create:{task.id} / acp.event_send:{task.id} put the task id in the span
NAME, which is high-cardinality and breaks span-name aggregation in Tempo. Use
static names (acp.task_create / acp.event_send) and carry the id as the
agentex.task_id span attribute instead.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ndle leak, dd_only Temporal backend Four review-comment fixes for the business<->obs correlation edge, each with unit coverage: - ADK span() CM recorded no error on a failing step, so a failed agent step closed a green obs span. Now sets span error (guarded so obs can't shadow the app exception) and re-raises. (tests/test_adk_tracing_span_error.py) - In lgtm mode with no TracerProvider, open_obs_span returned a handle with empty correlation, suppressing the ambient obs_correlation() fallback -> business span got no obs_* ids. Now bails to None so the caller falls back. (tests/test_obs_span_fallback.py) - Obs-handle registry could leak (registration-order + start-without-end via public API). Processor hooks now swallow (obs must never crash the app path) and the registry is a bounded OrderedDict that evicts+closes the oldest. (tests/test_obs_handle_registry.py) - On the Temporal path the ambient span is the OTel interceptor span regardless of SGP_OBS_MODE, but tag/read branched on mode -> in the default dd_only they tagged/read an unrelated ddtrace span. Added prefer_otel (OTel-first, ddtrace fallback) via a shared _begin_obs helper used by both start_span paths. (tests/test_temporal_obs_backend.py) Also: ObsSpanHandle.close() instead of reaching into _close; Iterator from collections.abc. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ack contract The comment-#2 fix changed open_obs_span so an invalid wrapper context returns None (caller falls back to ambient obs_correlation()) instead of a handle with empty correlation. The pre-existing tests/lib/core/tracing/test_obs_span.py still asserted the old contract and failed CI. Update it to assert None + that the attached context is detached and the no-op span ended (no leak). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_acp_dispatch_span only guarded the opentelemetry import; get_tracer() and entering start_as_current_span (which runs the sampler and every SpanProcessor.on_start — the SDK guards neither) ran unprotected, so a broken provider or a custom sampler/processor that raises would fail the task/create / event/send dispatch itself, against the fail-open principle used elsewhere. Guard the full setup (import + get_tracer + span __enter__); on any failure run the dispatch untraced. Keep the dispatch body (yield) outside the guard so its exceptions still propagate, and guard __exit__ (passing exc info so the span reflects a failed dispatch) so closing can't mask the outcome. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| Back | FazBrowse Home | New Git URL |
What & why
Model the business ↔ observability correlation edge on the emit side, so a persisted business span can pivot to its Tempo/Datadog trace and back. No schema migration — the ids ride the existing operation_metadata JSONB (→ ClickHouse metadata_raw).
Two worlds today: business spans (trace_id = task id, stored in application_trace_span + CH) and obs traces (OTel/ddtrace in Tempo/Datadog) share no id. This adds the link.
Changes
Behavior (Turn 2 of the 3-turn example)
Distinct, named obs_span_id per step; all under the one turn obs trace B.
✅ Live validation — infra-staging, lgtm/Tempo, end-to-end (sync + async paths)
Deployed to sgp-infra-staging in lgtm mode (Tempo) and validated end-to-end across both execution paths — a real agent turn renders as one connected obs trace with the business↔obs correlation edge populated in both directions.
Supersedes the earlier dd_only 52-span check — the child_of/wrapper behavior from that run still holds; this extends it to full lgtm/Tempo end-to-end coverage on both the sync and async paths.
Tests
Unit coverage for mode degrade + keys, both backends, non-interference, never-fails, reverse tag, error status, child_of nesting, and the Turn-2 example. Review-round additions (all green):
Not in this PR (follow-ups)
🤖 Generated with Claude Code
Greptile Summary
The PR adds bidirectional correlation between business spans and observability traces.
Confidence Score: 5/5
The PR appears safe to merge.
The previously reported processor-failure leak is resolved because synchronous processor exceptions and asynchronous enqueue failures are contained after handle registration, while the normal end path removes and closes the wrapper handle; no blocking failure remains.
Important Files Changed
Sequence Diagram
sequenceDiagram participant App participant Business as Business Span participant Wrapper as Obs Wrapper participant Backend as Tempo/Datadog App->>Business: start_span(step) Business->>Wrapper: open named active span Wrapper-->>Business: obs_trace_id + obs_span_id Business->>Business: persist IDs in metadata Wrapper->>Wrapper: tag business_trace_id + business_span_id App->>Business: end_span(error?) Business->>Wrapper: close and propagate error status Wrapper->>Backend: export correlated spanReviews (7): Last reviewed commit: "test(tracing): update obs_span invalid-c..." | Re-trigger Greptile