Repository navigation
Tier 1 of the plan in #259 - #305
Merged
Merged
Conversation
Seven defect tests (D1-D7 from the audited Tier 1 map) written before any fix: unavailability must warn with an install hint instead of silent no-op; two endpoints must route independently; spans must be exported before _save_otel returns; env-style string and mapping headers must both work while garbage raises a clear error; the otel_endpoint field must be honored; the dead otel_metrics kwarg must emit a DeprecationWarning; and the call must return a structured result and log its outcome. All eight fail on the unfixed tree, each for its documented reason. Two labeled characterization pins (env-var endpoint resolution and param-beats-env precedence) pass before and after, pinning behavior the repair must preserve.
_save_otel now builds a per-export-call TracerProvider with shutdown_on_exit disabled, flushes and shuts down in try/finally, and never touches the global tracer provider, so sequential exports to different endpoints route independently and spans are delivered before the call returns (batch thread joined in-call). Configuration resolves through _resolve_otlp_config with documented precedence: explicit parameter, then the otel_endpoint field (default now None instead of a dead localhost:5000 string), then the SDK's own OTEL_EXPORTER_OTLP_* environment variables. Headers accept a mapping or an env-style k=v string via the SDK parser; anything else raises a clear ValueError. Unavailability of the otel extra logs a warning with an install hint instead of silently no-opping, success and flush failure are logged on ursa.observability.otel, spans carry a Resource with service.name, agent, and run id, and the call returns a structured result the render path reports truthfully.
The argument has never been read; explicit use now emits a DeprecationWarning pointing at issue 259, where Tier 2 supersedes it.
Gate actions from the T1.1 review: an in-process D1 variant pins the structured otel-unavailable result and its install-hint warning, and the success log now says flushed rather than exported, since force_flush confirms handoff to the exporter, not delivery (the SDK exposes no per-batch delivery status).
Accept any collections.abc.Mapping for otel_headers (not just dict), widen the render annotations to match, and align the render line with the flush-not-delivery semantics of the success log.
Four red tests pin the T1.2 contract: the root span carries the run's real started_at/ended_at, each llm_events entry becomes a child span inside the root interval with GenAI semconv attributes, and ok=False events get ERROR status. Two green guards pin root-only export for empty llm_events and robustness to missing context timestamps.
The exported trace now mirrors the run it describes: the root span takes its start and end from the payload context's started_at and ended_at, and every llm_events entry becomes a child span at its real wall-clock interval. Children carry GenAI semconv attributes (string literals, Development status: gen_ai.operation.name, gen_ai.request.model, gen_ai.usage.input_tokens/output_tokens) plus ursa.langgraph.node/step, and events recorded with ok=False get ERROR span status. Events without timestamps are skipped; a context without timestamps falls back to the SDK clock.
The T1.2 child spans read only a "model" metadata key, which live langchain callbacks never set; real runs exported bare "chat" spans with no model. Children now fall back to ls_model_name, carry the Required gen_ai.provider.name from ls_provider, and are SpanKind.CLIENT per the GenAI conventions. Span assembly is guarded so a malformed event exports a partial trace with a warning instead of raising inside the agent's finally and leaving orphaned children, and events skipped for missing timestamps are logged at debug level.
Add a reference page covering the otel extra, the save_otel invoke flag, endpoint and header resolution order, MLflow and collector examples, the exported span shape, and the flush and re-export semantics. Point the child-span docstring at the GenAI conventions' new repository.
URSA imports only opentelemetry.exporter.otlp.proto.http, which the opentelemetry-exporter-otlp-proto-http distribution provides; the umbrella package additionally pulled in the unused grpc exporter and grpcio. Same version floor.
The mutation matrix showed the top precedence pair (explicit parameter over the otel_endpoint field) had no direct pin, and A12 asks for an assertion that export leaves no atexit-registered providers behind. Both pins are green on the current code and were verified to bite: flipping the precedence order fails the first, and re-enabling shutdown_on_exit while dropping the explicit shutdown fails the second.
Regenerated with a current uv (lockfile revision 3), which drops the otlp umbrella package and normalizes some marker representation; the platform exclusions are unchanged, checked via uv export.
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.
Summary
This is Tier 1 of the plan laid out in #259: repair the existing OTLP trace export so it is correct, observable, and documented, and make the exported trace carry the run's real timeline. There is no new user surface beyond the existing
save_otelflag and theotelextra, and the default path (export off) behaves exactly as before. Every defect from the audit was reproduced first, pinned by a pre-registered failing test, and the tests flipped green with no test edits after each repair.What was broken and what changed
save_otel=Truedid nothing, invisiblyset_tracer_provider; the SDK honors only the first, so later exports landed on the first endpoint ([2, 0] repro in #259) and each call leaked a provider and threadforce_flush(10 s timeout) and shutdown inside the call, in try/finallystr-annotatedotel_headersraisedValueErrorat exporter construction"k=v,k2=v2"string (parsed with the SDK's own liberal parser); anything else raises aValueErrorthat namesotel_headersotel_metricsaccepted and never read;Telemetry.otel_endpointnever readotel_metricsnow warnsDeprecationWarning(Tier 2 supersedes it); the field joined the endpoint resolution orderOTEL_*variables, then the SDK default; characterization tests pin that the pre-existing env behavior survived_save_otelclaimed-> strand returned its argument; success and failure were both invisible{ok, endpoint, span_count, reason}result, with success and failure logged onursa.observability.otelThe exported trace now
One trace per export: a root span named with the run id spanning the run's real
started_attoended_atand carrying the time-breakdown and token-total attributes, plus oneSpanKind.CLIENTchild span per recorded LLM call at the call's real wall-clock interval. Children are namedchat <model>and carrygen_ai.operation.name,gen_ai.request.model,gen_ai.provider.name,gen_ai.usage.input_tokens,gen_ai.usage.output_tokens,ursa.langgraph.node, andursa.langgraph.stepwhen the recorder captured them; the model and provider come from thels_model_nameandls_providerkeys that LangChain callbacks attach. Failed calls export with ERROR status and the error text as the status description. Malformed events cannot raise out of the export: assembly is guarded, the root always ends, and a partial trace with a warning beats a crashedfinally.Attribute names are string literals following the OpenTelemetry GenAI semantic conventions. Those conventions are Development status and recently moved to the semantic-conventions-genai repository, so names may still evolve upstream; the docs page and the docstring cite the new home.
The six questions from #259, answered with reversible defaults
Rather than blocking Tier 1 on the thread, each open question got a documented default chosen to be cheap to reverse:
llm_eventsalready carries real timestamps, and MLflow ingestion is HTTP-only. StandardOTEL_*env vars are honoredset_tracer_provider, per the OTel library guidelines cited in #259render().okmeans the spans were handed to the exporter and flushed; the SDK logs delivery failures. Export never raises into the agent, and an unreachable collector is bounded by the flush and shutdown timeouts and reported in the result and logmetricscommand from #290 once Tier 2 landsEcosystem alignment
Exporting a completed run after the fact is explicitly supported by the OTel Trace API, which defines span creation and end with explicit timestamps from the past. The resulting shape matches how the ecosystem structures LLM traces: MLflow's opt-in GenAI semconv export mode translates its spans into the same
gen_ai.*attributes per LLM call, and OpenLLMetry migrated its attribute names to exactly the names used here (gen_ai.request.model,gen_ai.usage.input_tokens,gen_ai.usage.output_tokens). OpenInference uses its own namespace but deliberately leaves span naming unregulated, so nothing here conflicts with it. The one place URSA diverges from ecosystem practice is the root span name (the run id), which is high-cardinality; the GenAI agent-spans convention (invoke_agent <agent name>withgen_ai.agent.nameand conversation id attributes) is the natural Tier 2 upgrade and pairs with metrics on the same Resource.Packaging change that needs your sign-off
The
otelextra now installsopentelemetry-exporter-otlp-proto-http>=1.39.0instead of theopentelemetry-exporter-otlpumbrella. URSA imports onlyopentelemetry.exporter.otlp.proto.http, and the umbrella additionally pulled in the unused gRPC exporter and grpcio wheels. It is a one-line revert if you prefer the umbrella. The accompanyinguv.lockrefresh was generated with a current uv (lockfile revision 3), which also normalizes some unrelated marker representation; I verified withuv exportthat the platform exclusions (for example the nvidia wheels staying linux-only) are unchanged. Related and worth weighing together: when the endpoint resolves through the SDK's own env or default path, the reported endpoint string is read from the exporter's private_endpointattribute, which is stable across the pinned floor (verified at 1.44.0) but is private API.Tests and CI
25 tests: 24 in
tests/observability/test_otel_export.pyplus aDeprecationWarningtest intests/agents/test_otel_kwargs.py. Method throughout: tests were written first and observed red before each fix. The two core repair waves committed their red tests separately beforehand (4960641 before 36b8af8, 596a2bf before 8005f55) and flipped green with zero test edits; the follow-up fixes carry their new red tests in the same commit, and no earlier test's body was ever edited. The load-bearing behaviors were mutation-checked (flushing, per-call provider routing, endpoint precedence including parameter over field, real timestamps, child parenting, live-metadata reads, the assembly guard, atexit cleanliness). The two subprocess tests total about 0.6 s.Full suite locally: 362 passed; the only non-passes are pre-existing environment ones (the OPENAI_API_KEY smoke failure and three dashboard import errors without fastapi). CI installs
--all-extras, so the battery runs on all three OS images; the new code and tests use nothing beyond Python 3.11, create no files or directories, and hold no handles that would trouble Windows.Docs
New reference page, Reference then OpenTelemetry export: install and opt-in, the endpoint resolution table, header rules, collector and MLflow examples, the exported span shape, and a semantics-and-limits section that states plainly what
okdoes and does not promise, the re-export behavior, the unclamped-timestamps caveat, and the currenterror.typeomission. The mkdocs build was validated locally; the site workflow publishes from main after merge.Non-goals here (Tier 2 and beyond)
Metrics instruments; live node and tool spans (their records carry no timestamps today, so faithful spans need live instrumentation via the existing
parent_run_idbookkeeping); theinvoke_agentroot rename; a CLI toggle; any content capture;error.typeon failed children. The per-run console rendering gap that hid the old confirmation line predates this work, still hides the repaired one, and is filed separately as #304.Changelog bullet for the next release notes
Following the #291 convention of compiling the changelog at release time, here is a paste-ready bullet: