Repository navigation
fix(otel): export sampled fallback roots early - #754
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…/python-754-20261006
…/python-754-20261006
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Thanks for this — the "resolve the root sampling result before descendants" wiring, the ARN-scoped canonical trace ID, and the override of SDK-owned attributes against sampler attributes (test_root_identity_wins_over_sampler_attributes_before_processors) all look correct and well-tested.
Two parity-blockers against JS #966 as merged, one test-shape suggestion, and a few nits/README tweaks. The parity concern is specifically about the shape of the anchor and the sampling intent reaching processors, where JS deliberately picked a narrower contract than the current PR.
I may have mistraced some of this, happy to be corrected on any point! :-)
| provider.shutdown() | ||
|
|
||
|
|
||
| def test_shared_trace_retains_execution_scoped_root_identity( |
There was a problem hiding this comment.
suggestion — shared-trace test uses one provider and one start time
The shared-trace test (two executions with the same propagated trace ID) uses one TracerProvider and the same execution_start_time for both. JS and Java test the same scenario with distinct providers and distinct start times, which is where the ARN-scoped canonical-trace-id logic actually earns its keep (recovery across regions, cold-start resource drift, start-time skew).
Could we add a variant that gives each execution its own provider and start_time?
There was a problem hiding this comment.
Added the variant in e3c2d41. Each execution has its own TracerProvider/resource and backend start time while sharing the propagated trace ID. Both views assert the shared trace ID, distinct ARN-scoped anchor IDs, each execution's zero-duration timestamp, and its own provider resource. The existing single-provider and recovery/resource-change cases remain.
Codex AI reviewFound one compatibility blocker affecting the documented OpenTelemetry 1.20 minimum; the added tests do not exercise that lower bound. Reviewed commit |
Fixes #748.
Related timing requirement: aws/aws-durable-execution-sdk-js#938.
Create and end DurableExecutionRoot at invocation Start whenever a sampled invocation resolves an SDK-owned fallback ancestor. Complete remote parents are untouched; DROP and RECORD_ONLY invocations emit no anchor. The anchor uses its existing ARN-scoped ID and backend start timestamp for both start and end. It is a zero-duration identity span whose descendants can end later; Workflow carries execution duration and terminal outcome.
Only the resolved sampling decision is reused for the anchor. Its SDK attributes are durable.execution.arn and durable.execution.synthetic_root; variable sampler attributes and tracestate are omitted from this repeated identity. Normal durable spans retain the full resolved sampling result. Configured provider resources and processors are preserved. Re-export on later sampled invocations can recover a lost copy, but export/flush failure or termination before flushing does not provide a delivery guarantee.
Each SDK span receives a single-use sampling-intent delivery, consumed before processors run. A processor forwarding parent_context therefore uses its configured sampling policy and normal ancestry instead of replaying private SDK sampling metadata. Implicit-current behavior and public ID-generator scope semantics are unchanged. Repeated post-bind tracer checks are removed while the existing SDK/deferred-provider gate remains. Shared non-null timestamp conversion preserves each plugin's existing None policy.
The branch includes main's approved worker lifecycle and MicroVM worker package. OTel 1.1 requires core 2.1; compatibility validation uses built immutable artifacts. The removed handler_context API and marker are not reintroduced.
Validation of the review follow-up:
Current follow-up head: e3c2d41. All current-head whole workflows have succeeded, including package, cloud and OpenTelemetry Conformance Tests run 38430650225. The human Changes Requested review remains for re-review.