Skip to content

feat(otel)!: honor configured samplers for durable spans - #875

Draft
zhongkechen wants to merge 1 commit into
aws:mainfrom
zhongkechen:feat/honor-configured-sampler-853
Draft

zhongkechen wants to merge 1 commit into
aws:mainfrom
zhongkechen:feat/honor-configured-sampler-853

Conversation

@zhongkechen

Copy link
Copy Markdown
Contributor

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Issue Link, if available

Closes #853.

Draft pending agreement on the shared sampling RFC and coordinated conformance contract. Related: JavaScript implementation #1173, Python #887, and shared docs #295. This must not ship as an unannounced patch-level behavior change.

Description

Both durable views now honor the provider's configured sampler. For example, always_off drops Workflow, Invocation, operation, attempt and synthetic-root spans even when the incoming X-Ray header says Sampled=1. parentbased_* preserves upstream inheritance; direct always_on, always_off and traceidratio use their standard OTel semantics.

Resolve from the canonical execution trace ID and original complete propagated remote parent. A valid parent without a sampled flag is unsampled; an invalid/incomplete parent supplies root context. Ambient handlers and synthesized ancestors never become sampling inputs. Carry this original parent through the existing agent bridge and preserve full sampling results in supported provider/loader arrangements. Invocation parenting falls back from a known dropped same-trace ambient handler to the execution ancestor.

Environment parsing, OTEL_TRACES_SAMPLER_ARG, defaults, validation and ownership stay with the provider. Ordinary spans retain its delegate behavior. The process cache remains an optimization, with no persisted decisions or support for nondeterministic policies. README migration guidance is explicitly unreleased and links the shared proposal.

Demo/Screenshots

No UI change. Exporter assertions cover both views; no X-Ray console qualification is claimed.

Design

Design note, following the proposed shared RFC and the implementation outline in #853. Maintainer/cross-language agreement is pending; keep this PR draft until that prerequisite is settled.

  • Cause: upstream and ambient short circuits bypassed the delegate; deferred resolution discarded the original parent and always evaluated at root.
  • Simpler options: remove those short circuits and reuse the resolved provider sampler. Separate environment parsing and policy-specific exceptions duplicate provider configuration and cannot preserve arbitrary sampling results.
  • Source change: six production files, net 55 lines removed; no public API members added or removed. Existing private bridge encoding now carries the original parent through the existing OTel W3C propagator.

Assumptions

  • Standard policy/default/validation behavior is exercised against OTel 1.67.0's real autoconfiguration SDK and production SPI discovery, with actual environment variables in fresh JVMs.
  • Both application-owned and deferred global-provider paths are tested. Separate plugin classloaders preserve parent flags/tracestate and full results. Matching application/extension plugin versions are required.
  • Deployed ADOT Lambda layer behavior, X-Ray ingestion/console rendering, and cloud conformance are unverified by this PR. Local exporter evidence is not backend qualification.
  • The shared conformance CONTRIBUTING sampling table currently specifies the old precedence. Coordinate its replacement and policy-matrix, no-flag/no-parent, resume/retry and dropped-ambient requirements before release. Per repository guidance, this PR does not add conformance requirements or hand-code an SDK handler workaround.

Decisions not covered by the issue or the design note

  • Remove the plain-replacement provider's preliminary remote-root probe too: retain normal per-span sampling with original remote-parent flags, avoiding a discarded extra sampler call.
  • Run the actual environment probe as a standalone CI harness on Java 17, using an externally resolved matching autoconfiguration artifact, without adding a production or Maven test dependency.
  • Retain recording-only ambient parents; only a known dropped ambient parent triggers fallback.

Checklist

  • I have filled out every section of the PR template
  • Local validation and its limitations are reported below
  • Changed branches have focused policy, bridge, or real-handler parent coverage
  • Baseline regression evidence is recorded below
  • Edited expectations and fixtures are listed below

Testing

Java 17 validation passed across the Maven reactor: 3992 tests, zero remaining failures/errors, 36 skipped tests (31 cloud examples and five existing SDK skips). The initial mvn -B spotless:apply install run hit one intermittent checkpoint-count assertion in the unchanged BackendOperationTimingTest (expected 1, observed 2); mvn -B spotless:apply install -rf :aws-durable-execution-sdk-java-plugin-otel reran all 1,256 OTel tests successfully and completed all remaining modules. No change was made to that timing test.

mvn -B spotless:check, git diff --check, local documentation link checks, Python/YAML/shell syntax checks, the exact new CI environment-probe commands (25/25 configurations), and the existing installed OTel API compatibility matrix (4/4) passed. Cloud example tests and deployed cross-language conformance were not run.

Unit Tests

New deferred-parent matrix covers all six policies in local and separate plugin classloaders, original flags/tracestate, full RECORD_ONLY results, cache reuse/cold recomputation, and ordinary-span isolation. Real handler-span tests distinguish DROP, RECORD_ONLY, sampled, unrelated and missing ambient parents.

Edited expectations and fixtures:

Tests Reason
DurableSamplerTest explicit sampled/unsampled conflict tests Configured direct policies now win.
OtelPluginSupportTest Replace upstream/ambient precedence assertions with stable original-parent and deferred-provider assertions.
ExecutionTraceContextTest undecided-parent and incomplete-parent tests Remote flags stay original; synthetic flags use the resolved policy.
ExecutionOtelPluginTest.xrayExtraction_undecidedSampling_parentBasedSampler_* A valid remote parent without a sampled flag drops under ParentBased.
ExecutionRootParityTest.completeRemoteParentRetainsNormalPerSpanPolicy Plain replacements no longer receive a preliminary root-policy query.
ReplacementSamplerPolicyTest, VisibleReplacementSdkSpanPolicyTest Incomplete parents use root policy; complete unset-flag parents are unsampled.
ExecutionRootTest anchor ownership and processor isolation Fixtures request sampling/drop explicitly while retaining root identity and unrelated-provider checks.
DurableSamplingDecisionClassLoaderTest, SamplingProcessorIsolationTest, RootStartProcessorIsolationTest Explicitly select durable spans while retaining all ordinary sampling/random-ID assertions; also assert carriers are empty before processor callbacks.
InvocationHeaderTest, OtelPropagationMetadataTest, InvokePropagationIntegrationTest, PluginStartupOrderTest Fixtures that require inherited flags now use ParentBased; retain propagation, identity, replay and lifecycle assertions.

Integration Tests

ConfiguredSamplerIntegrationTest adds 152 cases. Against base d71d588c, 60 fail (zero errors); all pass with this implementation. The 24 real runner resume/retry cases use fixed trace IDs on both sides of the ratio threshold, verify every durable span's selection, and assert completed user work executes once.

The standalone actual-autoconfiguration probe fails 18 of 25 configuration processes on the base and passes all 25 with this change: 864 initial/resume scenarios, plus seven invalid-configuration rejections. It is now part of the Build workflow. Instructions: SAMPLING.md.

Examples

No new executable example; the README documents the policy matrix, provider configuration, compatibility boundary and migration to parentbased_*.

Resolve sampling from the canonical trace and original remote parent in
both provider and deferred agent paths. Keep ordinary spans delegated,
qualify replay and autoconfiguration, and document the proposed migration.

Refs aws#853; shared contract aws/aws-durable-execution-docs#308.

This branch has not been deployed

No deployments
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.

[Feature]: Honor OTEL_TRACES_SAMPLER for durable spans

1 participant