Skip to content

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

Open
zhongkechen wants to merge 14 commits into
mainfrom
fix/otel-fallback-root
Open

zhongkechen wants to merge 14 commits into
mainfrom
fix/otel-fallback-root

Conversation

@zhongkechen

@zhongkechen zhongkechen commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #748, including the first-invocation timing requirement in aws/aws-durable-execution-sdk-js#938.

Both OTel views now export DurableExecutionRoot when a sampled invocation resolves an SDK-owned fallback ancestor. The anchor is materialized at invocation start and flushed through normal invocation end, including PENDING and RETRY, so an execution that times out while suspended already has its ancestor. Complete remote parents are untouched, and unsampled invocations emit no synthetic root.

The span uses the existing ARN-scoped synthetic-parent ID, durable.execution.synthetic_root=true, and the stable execution-start timestamp for both start and end. Each sampled fallback invocation can re-export it to recover lost telemetry; first-invocation retries retain the same SDK-owned identity/timing/attributes. Workflow continues to carry duration and terminal outcome. The synthetic-root path uses a private one-span ID scope, consumed before processors run, so reentrant roots on the same cached tracer get independent IDs. The exported generator's existing use_ids signature and scope-wide trace override semantics remain unchanged. The normal configured tracer, provider resource, processors, and resolved sampler metadata are preserved. Provider resources/custom enrichment may vary across environments; this does not replace configured service identity or promise exactly-once export.

Validation: after integrating main's view-registration guard, all 1,765 core tests (+5 subtests, 98.28% coverage) and 449 OTel tests pass. Fourteen valid registration/wait-resume cases pass against installed core 2.0.1 with site-packages imports verified. Repository type checks, core/OTel Hatch lint/format checks, workflow wiring and diff checks pass. The complete OTel suite exercises the workspace core; the installed legacy lane covers valid registrations because exclusivity validation requires core 2.1+. The runtime dependency floor remains core 2.0.0.

Regression evidence includes both views across absent/invalid/partial remote context, explicit and local sampling (including RECORD_ONLY), shared trace IDs, retry/resume, resource changes and buffered flushes. Real decorated handlers cover suspension/resume success and failure, skipped completed steps and timeout without a terminal hook. Ten lifecycle regressions failed without anchor materialization; the reviewer's 16-case guarded-processor probe and six resource/custom-processor/noncopyable-tracer probes passed after the fixes. Sampler-supplied attributes cannot replace the SDK's root identity before span processors run. No cloud deployment was run locally.

@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
concurrency:
group: cloud-tests-${{ matrix.python-prefix }}
cancel-in-progress: false
queue: max

This comment was marked as outdated.

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.

Checked the current official Actions workflow schema: concurrency-mapping supports queue, whose allowed values are single and max. Both workflow-level and job-level concurrency use this mapping.

The reviewed head 1a57d1d has also completed Cloud tests and Conformance Tests successfully with these declarations, so the workflows are accepted and running. Keeping queue: max together with cancel-in-progress: false retains pending runs while preserving the active tests.

Comment on lines +56 to +59
attributes={
"durable.execution.arn": self.execution_arn,
"durable.execution.synthetic_root": True,
},

This comment was marked as outdated.

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.

Confirmed and fixed in dc429fd. The synthetic root now uses its own sampling intent with the SDK-owned execution ARN and synthetic-root marker taking precedence. Other sampler attributes, the resolved decision, and tracestate are preserved; the invocation's original sampling result is not mutated or resampled.

The regression reproduces both collisions before the fix for both plugin views, covering the first invocation and resume. It checks attributes as observed by a processor's on_start and by the exporter, stable root IDs, retained sampler metadata/tracestate, and one sampler call per invocation. All 387 OTel tests, package type checks, and lint/format checks pass.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 7, 2026 03:09 — with GitHub Actions Active
concurrency:
group: cloud-tests-${{ matrix.python-prefix }}
cancel-in-progress: false
queue: max

This comment was marked as outdated.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 7, 2026 19:13 — with GitHub Actions Active
sampling_intent: DurableSamplingIntent,
) -> None:
"""Export a sampled local ancestor; never replace an external parent."""
if self.ancestor.is_remote or not self.ancestor.trace_flags.sampled:

This comment was marked as outdated.

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.

Checked the published 1.20.0 release: TraceFlags.sampled is already defined as return bool(self & TraceFlags.SAMPLED). The official API 1.20.0 wheel contains the same property, and this code constructs the local ancestor's flags as TraceFlags instances.

I also installed both opentelemetry-api==1.20.0 and opentelemetry-sdk==1.20.0 and ran the complete OTel suite on 15caff4: all 449 tests pass, including fallback-root export in both views and decorated-handler suspend/resume cases. The existing accessor works at the declared dependency floor.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 7, 2026 23:06 — with GitHub Actions Active
sampling_intent: DurableSamplingIntent,
) -> None:
"""Export a sampled local ancestor; never replace an external parent."""
if self.ancestor.is_remote or not self.ancestor.trace_flags.sampled:

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.

Codex AI review · Finding arf_v1_bejdvhjdvhzsvsmin6hgyhxopm

P1: The standalone extra still supports OpenTelemetry 1.20, whose TraceFlags lacks .sampled. Every fallback invocation on that supported version raises AttributeError here, leaving the plugin with missing or fragmented telemetry. Use the existing version-compatible bitmask check with TraceFlags.SAMPLED, and add minimum-version coverage.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Codex AI review

One P1 compatibility regression remains in the fallback-root path; minimum-version OpenTelemetry coverage is missing.

Reviewed commit 2492e7197226921012199091ea0ebe3080de882f. Workflow run

This branch was successfully deployed

1 active deployment
ai-pr-review-runtime — 2492e719 Deployed Oct 7, 2026 by zhongkechen via ai-pr-review / Codex review / Generate Codex review #1413
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

1 participant