fix(otel): scope deterministic IDs to plugin tracers - #646
Conversation
9952f89 to
2807a76
Compare
2807a76 to
77f6ba9
Compare
77f6ba9 to
669c3f5
Compare
669c3f5 to
aa4a594
Compare
aa4a594 to
1114a2e
Compare
1114a2e to
798cb0c
Compare
798cb0c to
b7bfe80
Compare
| ``trace.get_tracer_provider()``). ``EXPLICIT`` uses | ||
| ``tracer_provider`` as-is and skips instrumentation registration. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
| *, | ||
| id_generator: IdGenerator | None = None, | ||
| ) -> ProviderResult: | ||
| def create_tracer_provider(config: OtelPluginConfig) -> ProviderResult: |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| current_generator = tracer.id_generator | ||
| if isinstance(current_generator, cls): | ||
| return current_generator | ||
|
|
||
| generator = cls(fallback_id_generator=current_generator) | ||
| provider.id_generator = generator | ||
| tracer.id_generator = generator |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
| omitted, the globally configured provider is used (for example, the | ||
| provider installed by the ADOT Lambda layer). Standalone | ||
| instrumentation registration is skipped for an application-owned | ||
| provider. | ||
| context_extractor: Upstream trace-context extractor. Defaults to the |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| if ambient_span_context.is_valid: | ||
| return ambient_context | ||
| return self._extracted_context or ambient_context |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| ## Features | ||
|
|
||
| - **Deterministic Trace IDs**: All invocations of the same durable execution share a single trace, derived from the X-Ray trace header or execution ARN | ||
| - **Deterministic Workflow Traces**: Durable operations use an execution-derived trace that is independent of the ambient Lambda/X-Ray trace |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
| tracer_provider: An application-owned provider to use as-is. When | ||
| omitted, the globally configured provider is used (for example, the | ||
| provider installed by the ADOT Lambda layer). Standalone | ||
| instrumentation registration is skipped for an application-owned | ||
| provider. |
There was a problem hiding this comment.
Codex AI review
[P1] Preserve the released configuration API
Removing ProviderSource, ExporterConfig, and the associated OtelPluginConfig fields makes documented existing configurations fail during import or construction after an upgrade. AUTO_OTLP users also lose provider setup entirely. Retain these exported names and arguments as deprecated compatibility shims, preserving their behavior until a major release, and add upgrade-compatibility tests.
| def _invocation_parent_context(self) -> Context: | ||
| """Return the active ambient context, then extracted upstream context.""" | ||
| ambient_context = otel_context.get_current() | ||
| ambient_span_context = trace.get_current_span( | ||
| ambient_context | ||
| ).get_span_context() | ||
| if ambient_span_context.is_valid: | ||
| return ambient_context |
There was a problem hiding this comment.
Codex AI review
[P1] Do not treat the previous Workflow span as ambient context
ExecutionOtelPlugin attaches its Workflow span but never detaches it. Without an outer OTel wrapper, the next warm invocation therefore sees the previous Workflow as current here and parents its Invocation span to the prior execution, defeating the trace separation this change introduces. Store the attachment token and detach it at invocation end and before defensive reset/start; add a test covering two sequential invocations on one plugin instance.
Codex AI reviewTwo blocking regressions found. Warm reuse and upgrade compatibility remain uncovered by the in-process tests. Reviewed commit |
Summary
This PR fixes two independent defects that could place multiple root spans in
the same OpenTelemetry trace.
Issue 1: deterministic IDs leaked through the shared provider
DeterministicIdGeneratorwas installed on the resolvedTracerProvider.A provider is shared by every instrumentation scope that uses it, so unrelated
instrumentation could receive the active durable execution trace ID when it
created a root span. The unrelated span had no durable parent, but appeared as
another root of the durable workflow trace.
The generator also held the current execution trace ID as shared mutable state.
Concurrent plugin instances or executions could therefore replace or consume
one another's deterministic IDs.
Issue 2: the Workflow root reused the ambient Lambda/X-Ray trace ID
The parentless
Workflowspan derived its trace ID from_X_AMZN_TRACE_ID.The ambient Lambda span and the plugin's
Invocationspan used that same trace,but the
Workflowspan was deliberately created without a parent. The resultwas two disconnected root trees sharing one trace ID:
The same collision occurred without ambient instrumentation when
Workflowand
Invocationwere independently created as roots with the deterministicexecution trace ID.
Why multiple roots are harmful
OpenTelemetry backends expect a trace to describe one causal tree. Multiple
roots turn it into a disconnected forest. Backend behavior varies, but the
potential consequences include:
or measures across unrelated root branches;
they only share a leaked trace ID;
discard another, leaving a partial trace.
Changes
Scope deterministic generation to plugin spans
DeterministicIdGeneratoron the SDKTracerused by each durableplugin instead of mutating the shared
TracerProvider.ContextVar-scoped overrides only around the plugin-ownedstart_span()call that needs a deterministic ID.one-shot safeguard for re-entrant generation.
generation to the provider's original generator.
is_trace_id_random()so newer OpenTelemetry SDKs set trace flagscorrectly for deterministic and fallback IDs.
Separate workflow and invocation trace identities
time. It no longer reuses the ambient Lambda/X-Ray trace ID.
Invocationspans to the active ambient context in bothGLOBALandEXPLICITmodes, using extracted upstream context as a fallback.trace ID for the root
Invocationspan.The resulting trace models are:
Each durable Workflow trace has one root. Each invocation joins its ambient
trace, or creates a provider-generated trace in which
Invocationis the soleroot.
Do not fabricate continuation links
InvocationOtelPluginpreviously reconstructed a deterministic trace ID andspan ID to link a continuation segment to a prior span for the same logical
operation. The prior
SpanContextis not checkpointed, so the plugin cannotknow that the target span was actually created or exported, nor preserve its
real trace flags or trace state.
Continuation and retry segments now receive fresh span IDs and retain the real
link to the durable
Workflowspan, but do not emit a synthetic link to anunobserved prior operation span.
Keep provider ownership explicit
before ADOT setup can bind after the
ProxyTracerresolves.unavailable, preventing partial traces, and retry on the next invocation.
AUTO_OTLP. Provider/exporter construction, sampling, propagation,and HTTP instrumentation remain application or ADOT responsibilities.
GLOBALandEXPLICITprovider modes.Result
enter the durable workflow trace accidentally.
form disconnected roots in one trace.
replay, while continuation segments use fresh span IDs.
Testing
hatch run dev-otel:test(115 passed)23 source files)OpenTelemetry SDK
1.20.0(39 passed)Closes #644