Skip to content

fix(otel): export sampled fallback roots early - #754

Merged
zhongkechen merged 17 commits into
mainfrom
fix/otel-fallback-root
Oct 13, 2026
Merged

zhongkechen merged 17 commits into
mainfrom
fix/otel-fallback-root

Conversation

@zhongkechen

@zhongkechen zhongkechen commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

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:

  • Five focused regressions fail on the original runtime while two implicit-current controls pass; the same seven pass after the minimal fix.
  • All 668 workspace OTel tests pass. Installed wheels pass 668 tests plus 12 public version-pair controls on API/SDK 1.45.1 and again on the actual supported minimum API/SDK 1.20.0. The validation environment is restored afterward.
  • New coverage includes changing sampler metadata across retry/resume, real processor re-entry, atomic consumption across forwarded contexts, distinct providers and start times, and decorated-handler wait/resume with completed-step suppression. Original provider initialization, status, replay and SDK-key collision controls remain.
  • All 49 core/OTel Python source files in the tested wheels match the committed sources. Types (37 OTel files), Hatch lint/format, diff and commit checks pass. The unmodified main MicroVM package passes 154 tests and its 11-file type check.

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.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 2, 2026 21:02 — with GitHub Actions Active
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 2, 2026 21:03 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 2, 2026 21:24 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 2, 2026 21:51 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 2, 2026 23:24 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 5, 2026 23:38 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen had a problem deploying to ai-pr-review-runtime October 7, 2026 02:06 — with GitHub Actions Failure
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 7, 2026 02:40 — with GitHub Actions Active
Comment thread .github/workflows/cloud-tests.yml
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 7, 2026 03:09 — with GitHub Actions Active
Comment thread .github/workflows/cloud-tests.yml
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 7, 2026 19:13 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 7, 2026 23:06 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen requested a review from a team October 8, 2026 16:59
@zhongkechen zhongkechen mentioned this pull request Oct 8, 2026
1 of 6 tasks
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 9, 2026 18:36 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@yaythomas yaythomas left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/aws-durable-execution-sdk-python-otel/README.md
Comment thread packages/aws-durable-execution-sdk-python-otel/README.md Outdated
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 13, 2026 16:27 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

Found one compatibility blocker affecting the documented OpenTelemetry 1.20 minimum; the added tests do not exercise that lower bound.

Reviewed commit e3c2d419814b35e26c9ff7575b9db236a1c762aa. Workflow run

@yaythomas yaythomas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

beaut, ty!

@zhongkechen
zhongkechen merged commit 29b8db0 into main Oct 13, 2026
93 checks passed
@zhongkechen
zhongkechen deleted the fix/otel-fallback-root branch October 13, 2026 18:18

This branch was successfully deployed

1 active deployment
ai-pr-review-runtime — e3c2d419 Deployed Oct 13, 2026 by zhongkechen via ai-pr-review / Codex review / Generate Codex review #1564
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: export stable fallback synthetic roots, including before suspension

2 participants