Repository navigation
test: cover OTel replay and user callback context in Java - #769
zhongkechen wants to merge 75 commits into
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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| || (!usesDurableSamplingBridge(sdkTracerProvider) | ||
| && extracted != null | ||
| && extracted.hasCompleteRemoteParent()))) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
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.
| || (errorObject.errorType() == null | ||
| && errorObject.errorMessage() == null | ||
| && errorObject.errorData() == null | ||
| && !errorObject.hasStackTrace())) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
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.
This comment has been minimized.
This comment has been minimized.
| span.setAttribute(DURABLE_ATTEMPT_NUMBER, info.attempt().longValue()); | ||
| } | ||
| if (info.error() != null) { | ||
| if (OtelPluginSupport.hasOperationErrorDetails(info.error())) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
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.
| && (usesLocalDurableSampler(sdkTracerProvider) | ||
| || (!usesDurableSamplingBridge(sdkTracerProvider) | ||
| && extracted != null | ||
| && extracted.hasCompleteRemoteParent()))) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
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.
This comment has been minimized.
This comment has been minimized.
| && (usesLocalDurableSampler(sdkTracerProvider) | ||
| || (!usesDurableSamplingBridge(sdkTracerProvider) | ||
| && extracted != null | ||
| && extracted.hasCompleteRemoteParent()))) { |
There was a problem hiding this comment.
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.
| return details.errorType() != null | ||
| || details.errorMessage() != null | ||
| || details.errorData() != null | ||
| || details.hasStackTrace(); |
There was a problem hiding this comment.
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.
Codex AI reviewTwo medium-severity OTel edge cases remain in sampling and exception-detail handling; the added tests do not cover them. Reviewed commit |
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 to75987d46a915bc37409eed3ea9c3617a924c9756from 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
ef905e89passed 2,990 tests with 31 conditional skips; SDK/core/testing/conformance sources are unchanged by this four-file OTel followup.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.