Skip to content

test: cover OTel replay and user callback context in Java - #769

Open
zhongkechen wants to merge 75 commits into
mainfrom
test/otel-conformance-21-24
Open

zhongkechen wants to merge 75 commits into
mainfrom
test/otel-conformance-21-24

Conversation

@zhongkechen

@zhongkechen zhongkechen commented Oct 2, 2026 •

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.

Description

Add Java public-API handlers and both-view Lambda resources for OTel requirements 21–26: completed-step replay, active user-function contexts, callback/retry contexts, invocation retry status, terminal callback failure without error details, and external callback completion across later replays. Existing cases, performance thresholds and coverage gates remain intact.

Case 24 retains RETRYING/UNSET to SUCCEEDED/OK. Case 25 sends callback failure without Error after the creating invocation completes, requiring empty error payload and UNSET. Case 26 creates its target callback on the root context, checkpoints its result and crosses two later callback barriers. Its completion span belongs to the second invocation; raw S3 counts and quiescence verify that later replays add no duplicate export. The handlers use actual public SDK operations.

The case-25 coverage separates SDK failure notification from OTel error details. A present ErrorObject, including {}, remains a DurableOperationException for ordinary plugins and invocation snapshots. Only the OTel operation-end mapping suppresses ERROR status and exception events when a container has no details. Virtual contexts can report a null status on failure; they are marked OK only when no error is present, so a detail-less failure remains UNSET. Present empty strings or an explicit empty stack list remain details. Persisted ErrorObject, caller failure, user-function error handling and existing absent-container behavior remain intact. Actual cloud history exposes CallbackFailedDetails.Error.Payload={}, while exact-wire and public callback controls cover omitted/null/empty inputs and replay.

Both #771 and #767 have merged. This branch includes main 45e4ef47, its handler-owned invocation lifecycle and immediate synthetic-root export. Existing core/plugin capability requirements and the factory migration boundary remain unchanged; #782 is separate. The remaining already-admitted-worker and large-result acknowledgment limitations are tracked in #787 and are not repaired here.

For a visible plain replacement sampler with undecided sampling and a fallback root, use the actual root's sampling result instead of consuming an extra preflight decision before any span exists. Descendants still follow the provider's own per-span policy. Explicit upstream and same-trace ambient decisions retain their behavior. A complete remote parent with undecided sampling retains its legacy ancestor-policy query; this does not promise zero extra queries for every replacement-provider path or uniform decisions from a stateful sampler. Managed, foreign-loader and opaque DurableSampler carriers and full descendant metadata remain covered.

The reusable workflow stays pinned to bdb4f1cd0f9252c1aaa978bb8b341b71f2b9d9dc; the 26-case catalog stays pinned to 75987d46a915bc37409eed3ea9c3617a924c9756 from aws/aws-durable-execution-conformance-tests#131. All conformance handlers, deployment resources, workflow locks, package versions and API floors are unchanged by the latest base/sampling update.

Validation

  • Current Java 17 OTel component verification: 832 passed, zero failures/errors/skips. The earlier full repository verification on ef905e89 passed 2,990 tests with 31 conditional skips; SDK/core/testing/conformance sources are unchanged by this four-file OTel followup.
  • All 138 focused virtual, callback and both-view operation controls pass on Java 25, with zero failures/errors/skips.
  • The sampler regression fails in both views on the combined base before the fix: 2 failures and 10 passing controls. All 42 related controls now pass, including real public handler/replay/side-effect checks and two explicit complete-remote-parent compatibility controls.
  • The virtual-status regression fails three of 11 controls before the fix (both public views plus the missing-start invocation segment); eight success/ordinary-error controls pass. All 11 now preserve status, exception events, caller error translation, hook identity and completed-step replay.
  • A non-OTel listener regression failed the empty-container case before the notification fix, while four controls passed. All five now preserve snapshots, operation-end notification, raw failure and replay.
  • Four public complete-remote-parent controls retain the configured parent-based root policy. A separate non-published removal experiment fails both always-off controls (2 failures, 14 controls pass); the adopted behavior passes all 16. The remaining policy query is a documented compatibility tradeoff.
  • Existing public handlers cover one completed side effect, callback error metadata, actual first completion and later replay without duplicate export. The 26-case mappings and scenario assets are unchanged.
  • Historical matched cloud proof on 14db8ae9: 112/112 passed (four view/backend suites ×26 and two short suites ×4), zero uncovered/unimplemented cases: https://github.com/aws/aws-durable-execution-sdk-java/actions/runs/37715017463. That result is not a claim that the new head's cloud checks have completed.
  • Formatting and diff checks pass. Current-head cloud and artifact-compatibility results are tracked separately in CI; no new local compatibility matrix was run for this update.

@zhongkechen
zhongkechen marked this pull request as ready for review October 3, 2026 00:03
@zhongkechen
zhongkechen requested a review from a team October 3, 2026 00:03
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 3, 2026 00:03 — with GitHub Actions Active
@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 3, 2026 00:03 — with GitHub Actions Active
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 3, 2026 01:55 — with GitHub Actions Active
@github-actions

This comment has been minimized.

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

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 3, 2026 02:18 — with GitHub Actions Active
Comment thread sdk/src/main/java/software/amazon/lambda/durable/execution/DurableExecutor.java Outdated
Comment thread sdk/src/main/java/software/amazon/lambda/durable/execution/DurableExecutor.java Outdated
@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 3, 2026 03:08 — with GitHub Actions Active
Comment thread sdk/src/main/java/software/amazon/lambda/durable/execution/DurableExecutor.java Outdated
@github-actions

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 13, 2026 20:31 — with GitHub Actions Active
Comment on lines +179 to +181
|| (!usesDurableSamplingBridge(sdkTracerProvider)
&& extracted != null
&& extracted.hasCompleteRemoteParent()))) {

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.

The remaining query is a compatibility tradeoff for a complete remote Root/Parent with no explicit Sampled value or matching ambient decision. ExecutionTraceContext's existing contract derives that missing sampled bit from the configured root policy; OpenTelemetry TraceFlags has no undecided value.

I tested removing only this remaining plain-provider branch on 30783fa0, using real LocalDurableTestRunner invocations, suspension and replay. With ParentBased(alwaysOff), application spans incorrectly became sampled in all three invocations in both views. Those two controls fail; the other 14 controls pass. The retained implementation passes all 16. The cause is the current unresolved-decision default (isSampled(null) is true), not a cloud/tester artifact.

I am retaining the policy lookup and documenting/testing this boundary. A visible plain replacement does not receive the SDK decision carrier; making a precomputed result authoritative would require changing the provider setup or sampling contract. Retain DurableSampler for authoritative reuse, or supply an explicit upstream Sampled decision to bypass this query. The redundant fallback-root evaluation was fixed in 30783fa0; this complete-remote-parent tradeoff remains and is not claimed fully resolved.

The explicit public regression controls and README clarification are included in ef905e89; the sampler implementation is unchanged from 30783fa0.

Comment on lines +584 to +587
|| (errorObject.errorType() == null
&& errorObject.errorMessage() == null
&& errorObject.errorData() == null
&& !errorObject.hasStackTrace())) {

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: suppressing a present empty ErrorObject in the core also removed the failure notification seen by non-OTel plugins. ef905e89 restores the original DurableOperationException for every present error container, including {}. Existing absent-container behavior and status filtering remain unchanged.

Only the OTel operation-end mapping now checks whether that container actually supplies error details before setting ERROR or recording an exception. Explicit empty strings and an explicit stack-trace list remain present details. Ordinary/custom Throwable handling and user-function failure hooks retain their existing behavior.

A public callback/resume/replay regression using only a non-OTel listener failed for the empty container before the fix, while four controls passed. The fix preserves the error in invocation snapshots and operation-end events; mixed-plugin tests independently keep the OTel span UNSET with no exception event when details are absent. Persisted error data, caller failure, completed-work replay and single completion delivery remain unchanged.

Complete Java 17 verification passes 2,990 tests with 31 conditional skips; 184 focused Java 25 controls pass. The existing 26-case handlers, workflow pins and API/version contracts are unchanged.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 13, 2026 21:21 — with GitHub Actions Active
span.setAttribute(DURABLE_ATTEMPT_NUMBER, info.attempt().longValue());
}
if (info.error() != null) {
if (OtelPluginSupport.hasOperationErrorDetails(info.error())) {

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 66f755d0. All three operation-end success branches now require info.error() == null; a virtual context's null status alone cannot turn a detail-less failure into OK. The span remains UNSET with no exception event when its error container has no details. Genuine virtual success stays OK, and ordinary errors—including explicit failures without a backend container—retain ERROR.

The regression uses real public virtual-child calls in both views, then a durable step and wait/resume/replay. It preserves caller exception translation, the original operation-hook error, null virtual status and a single completed step side effect. The missing-start invocation segment has its own controls. Before the fix, three of 11 controls reported OK instead of UNSET; the other eight passed.

Complete OTel component verification passes 832 tests on Java 17, and 138 related controls pass on Java 25. SDK/core/conformance sources, API/version requirements and the 26-case workflow remain unchanged.

Comment on lines +195 to +198
&& (usesLocalDurableSampler(sdkTracerProvider)
|| (!usesDurableSamplingBridge(sdkTracerProvider)
&& extracted != null
&& extracted.hasCompleteRemoteParent()))) {

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.

This repeats the documented complete-remote-parent tradeoff explained and tested in #769 (comment). Removing this branch makes ParentBased(alwaysOff) record application spans in both public handler/replay paths; the existing configured-root-policy contract is retained. The fallback-root redundant evaluation is already removed, while explicit upstream sampling or retaining DurableSampler provides the supported decision-reuse path. The latest virtual-status fix does not change the sampler implementation.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime October 13, 2026 21:55 — with GitHub Actions Active
&& (usesLocalDurableSampler(sdkTracerProvider)
|| (!usesDurableSamplingBridge(sdkTracerProvider)
&& extracted != null
&& extracted.hasCompleteRemoteParent()))) {

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_jal7r4ndxw7pa3ttq6qmhvup65

P2 — Avoid consuming a sampler decision for the remote-parent probe. With an undecided complete remote parent, a stateful plain sampler is queried for Workflow before any span exists; the real Invocation and Workflow spans then receive later decisions. A one-shot or rate-limited sampler can consequently export no SDK span despite initially returning RECORD_AND_SAMPLE. Remove this preflight for plain samplers or make its result authoritative for the real SDK spans, and assert that remote-parent tests observe only real-span evaluations.

Comment on lines +96 to +99
return details.errorType() != null
|| details.errorMessage() != null
|| details.errorData() != null
|| details.hasStackTrace();

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_bzbshynsy7frwlj2koqbkxsva7

P2 — Preserve explicit exception details when the ErrorObject is empty. This returns false even when a public DurableOperationException has an explicit message, cause, or suppressed failure. Virtual child contexts pass that original exception to onOperationEnd, so both plugins incorrectly leave the failed span UNSET and omit its exception event. Suppress telemetry only when both the container and the exception’s own diagnostic fields are empty, and add virtual-operation tests for an empty container with an explicit message and cause.

@github-actions

Copy link
Copy Markdown
Contributor

Codex AI review

Two medium-severity OTel edge cases remain in sampling and exception-detail handling; the added tests do not cover them.

Reviewed commit 66f755d0caf11782a864dd6b7a0eb271e8152b3a. Workflow run

This branch was successfully deployed

1 active deployment
ai-pr-review-runtime — 66f755d0 Deployed Oct 13, 2026 by zhongkechen via ai-pr-review / Codex review / Generate Codex review #2164
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.

2 participants