From c8defb415689e68bb2cb087c49771fb5d4263796 Mon Sep 17 00:00:00 2001 From: silanhe Date: Fri, 31 Jul 2026 22:22:33 +0000 Subject: [PATCH 1/2] feat(otel): set StatusCode.OK on operation and attempt spans on success Port the explicit success-status behavior from the Python OTel plugin (PR #604) to the Java plugins. Operation spans (onOperationEnd terminal path and the cross-invocation continuation-span path) and attempt spans (onUserFunctionEnd) now call span.setStatus(StatusCode.OK) on success, where previously they were left UNSET. Existing ERROR + recordException on failure is unchanged. Applied to both InvocationOtelPlugin and ExecutionOtelPlugin. The Invocation-span (applyInvocationStatus) and Workflow-span OK/ERROR mappings are untouched, and operation spans force-ended attribute-less at onInvocationEnd (still-running/suspended) remain UNSET. Adds OK-on-success tests for operation and attempt spans in both plugin test classes. --- .../durable/otel/ExecutionOtelPlugin.java | 6 ++++ .../durable/otel/InvocationOtelPlugin.java | 6 ++++ .../durable/otel/ExecutionOtelPluginTest.java | 28 +++++++++++++++ .../otel/InvocationOtelPluginTest.java | 35 +++++++++++++++++++ 4 files changed, 75 insertions(+) diff --git a/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/ExecutionOtelPlugin.java b/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/ExecutionOtelPlugin.java index 6071bdbc9..8ce0d5921 100644 --- a/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/ExecutionOtelPlugin.java +++ b/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/ExecutionOtelPlugin.java @@ -308,6 +308,8 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { span.setStatus(StatusCode.ERROR, info.error().getMessage()); span.recordException(info.error()); + } else { + span.setStatus(StatusCode.OK); } endSpan(span, info.endTimestamp()); } else { @@ -350,6 +352,8 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { continuationSpan.setStatus(StatusCode.ERROR, info.error().getMessage()); continuationSpan.recordException(info.error()); + } else { + continuationSpan.setStatus(StatusCode.OK); } endSpan(continuationSpan, info.endTimestamp()); @@ -438,6 +442,8 @@ public void onUserFunctionEnd(UserFunctionEndInfo info) { if (!info.succeeded() && info.error() != null) { span.setStatus(StatusCode.ERROR, info.error().getMessage()); span.recordException(info.error()); + } else if (info.succeeded()) { + span.setStatus(StatusCode.OK); } endSpan(span, info.endTimestamp()); diff --git a/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/InvocationOtelPlugin.java b/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/InvocationOtelPlugin.java index 9111b80d2..8e39ccb49 100644 --- a/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/InvocationOtelPlugin.java +++ b/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/InvocationOtelPlugin.java @@ -370,6 +370,8 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { span.setStatus(StatusCode.ERROR, info.error().getMessage()); span.recordException(info.error()); + } else { + span.setStatus(StatusCode.OK); } span.end(); } else { @@ -413,6 +415,8 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { continuationSpan.setStatus(StatusCode.ERROR, info.error().getMessage()); continuationSpan.recordException(info.error()); + } else { + continuationSpan.setStatus(StatusCode.OK); } continuationSpan.end(); @@ -508,6 +512,8 @@ public void onUserFunctionEnd(UserFunctionEndInfo info) { if (!info.succeeded() && info.error() != null) { span.setStatus(StatusCode.ERROR, info.error().getMessage()); span.recordException(info.error()); + } else if (info.succeeded()) { + span.setStatus(StatusCode.OK); } if (info.endTimestamp() != null) { diff --git a/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/ExecutionOtelPluginTest.java b/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/ExecutionOtelPluginTest.java index df0326641..6418c9a16 100644 --- a/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/ExecutionOtelPluginTest.java +++ b/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/ExecutionOtelPluginTest.java @@ -344,6 +344,34 @@ void userFunctionFailure_setsErrorOnAttemptSpan() { assertEquals(StatusCode.ERROR, attemptSpan.getStatus().getStatusCode()); } + @Test + void userFunctionSuccess_setsOkOnAttemptSpan() { + plugin.onInvocationStart(new InvocationInfo("req-1", ARN, true, Instant.now())); + plugin.onUserFunctionStart( + new UserFunctionStartInfo("op-1", "compute", "STEP", "Step", null, Instant.now(), false, 1)); + plugin.onUserFunctionEnd(new UserFunctionEndInfo( + "op-1", "compute", "STEP", "Step", null, Instant.now(), Instant.now(), false, 1, true, null)); + plugin.onInvocationEnd(new InvocationEndInfo("req-1", ARN, true, InvocationStatus.SUCCEEDED, null)); + + var attemptSpan = spanExporter.getFinishedSpanItems().stream() + .filter(s -> s.getName().contains("attempt")) + .findFirst() + .orElseThrow(); + assertEquals(StatusCode.OK, attemptSpan.getStatus().getStatusCode()); + } + + @Test + void operationSuccess_setsOkOnOperationSpan() { + plugin.onInvocationStart(new InvocationInfo("req-1", ARN, true, Instant.now())); + plugin.onOperationStart(new OperationInfo("op-1", "step-ok", "STEP", "Step", null, Instant.now(), null, false)); + plugin.onOperationEnd(new OperationEndInfo( + "op-1", "step-ok", "STEP", "Step", null, Instant.now(), Instant.now(), "SUCCEEDED", null, false, null)); + plugin.onInvocationEnd(new InvocationEndInfo("req-1", ARN, true, InvocationStatus.SUCCEEDED, null)); + + var operationSpan = spanByName(spanExporter.getFinishedSpanItems(), "step-ok"); + assertEquals(StatusCode.OK, operationSpan.getStatus().getStatusCode()); + } + @Test void operationNotCompleted_notEndedAtInvocationEnd() { plugin.onInvocationStart(new InvocationInfo("req-1", ARN, true, Instant.now())); diff --git a/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/InvocationOtelPluginTest.java b/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/InvocationOtelPluginTest.java index 2536d0ad3..4247dd105 100644 --- a/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/InvocationOtelPluginTest.java +++ b/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/InvocationOtelPluginTest.java @@ -271,6 +271,41 @@ void userFunctionEnd_withFailure_setsErrorOnAttemptSpan() { assertEquals(StatusCode.ERROR, attemptSpan.getStatus().getStatusCode()); } + @Test + void userFunctionEnd_withSuccess_setsOkOnAttemptSpan() { + plugin.onInvocationStart(new InvocationInfo("req-1", "arn:exec1", true, Instant.now())); + + plugin.onUserFunctionStart( + new UserFunctionStartInfo("op-1", "compute", "STEP", "Step", null, Instant.now(), false, 1)); + plugin.onUserFunctionEnd(new UserFunctionEndInfo( + "op-1", "compute", "STEP", "Step", null, Instant.now(), Instant.now(), false, 1, true, null)); + + plugin.onInvocationEnd(new InvocationEndInfo("req-1", "arn:exec1", true, InvocationStatus.SUCCEEDED, null)); + + var attemptSpan = spanExporter.getFinishedSpanItems().stream() + .filter(s -> s.getName().contains("attempt")) + .findFirst() + .orElseThrow(); + assertEquals(StatusCode.OK, attemptSpan.getStatus().getStatusCode()); + } + + @Test + void operationEnd_withSuccess_setsOkOnOperationSpan() { + plugin.onInvocationStart(new InvocationInfo("req-1", "arn:exec1", true, Instant.now())); + + plugin.onOperationStart(new OperationInfo("op-1", "step-ok", "STEP", "Step", null, Instant.now(), null, false)); + plugin.onOperationEnd(new OperationEndInfo( + "op-1", "step-ok", "STEP", "Step", null, Instant.now(), Instant.now(), "SUCCEEDED", null, false, null)); + + plugin.onInvocationEnd(new InvocationEndInfo("req-1", "arn:exec1", true, InvocationStatus.SUCCEEDED, null)); + + var operationSpan = spanExporter.getFinishedSpanItems().stream() + .filter(s -> "step-ok".equals(s.getName())) + .findFirst() + .orElseThrow(); + assertEquals(StatusCode.OK, operationSpan.getStatus().getStatusCode()); + } + @Test void fullLifecycle_producesCorrectSpanHierarchy() { var arn = "arn:aws:lambda:us-east-1:123:function:test:$LATEST/durable/exec1"; From ad07ddb883ecf8f2eeabea09543debb722ec6cc1 Mon Sep 17 00:00:00 2001 From: silanhe Date: Fri, 31 Jul 2026 23:00:19 +0000 Subject: [PATCH 2/2] fix(otel): gate operation-span OK on SUCCEEDED status (addresses AI review on #583) --- .../durable/otel/ExecutionOtelPlugin.java | 10 ++- .../durable/otel/InvocationOtelPlugin.java | 10 ++- .../durable/otel/ExecutionOtelPluginTest.java | 46 ++++++++++++++ .../otel/InvocationOtelPluginTest.java | 61 +++++++++++++++++++ 4 files changed, 123 insertions(+), 4 deletions(-) diff --git a/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/ExecutionOtelPlugin.java b/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/ExecutionOtelPlugin.java index 8ce0d5921..b1e1bacd1 100644 --- a/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/ExecutionOtelPlugin.java +++ b/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/ExecutionOtelPlugin.java @@ -308,7 +308,11 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { span.setStatus(StatusCode.ERROR, info.error().getMessage()); span.recordException(info.error()); - } else { + } else if ("SUCCEEDED".equals(info.status()) || info.status() == null) { + // Only stamp OK on genuine success. onOperationEnd fires for every terminal status, and + // extractErrorFromOperation returns null for CANCELLED (always) and for FAILED/TIMED_OUT/STOPPED + // with no attached error object — those carry a non-null, non-SUCCEEDED status and must stay UNSET. + // A null status is a successful statusless virtual (FLAT CONTEXT) operation, which is OK. span.setStatus(StatusCode.OK); } endSpan(span, info.endTimestamp()); @@ -352,7 +356,9 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { continuationSpan.setStatus(StatusCode.ERROR, info.error().getMessage()); continuationSpan.recordException(info.error()); - } else { + } else if ("SUCCEEDED".equals(info.status()) || info.status() == null) { + // See onOperationEnd (this-invocation branch): only genuine success (or a successful statusless + // virtual operation) is OK; error-less non-success statuses stay UNSET. continuationSpan.setStatus(StatusCode.OK); } diff --git a/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/InvocationOtelPlugin.java b/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/InvocationOtelPlugin.java index bf92c14e8..1bf85bcd4 100644 --- a/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/InvocationOtelPlugin.java +++ b/otel-plugin/src/main/java/software/amazon/lambda/durable/otel/InvocationOtelPlugin.java @@ -430,7 +430,11 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { span.setStatus(StatusCode.ERROR, info.error().getMessage()); span.recordException(info.error()); - } else { + } else if ("SUCCEEDED".equals(info.status()) || info.status() == null) { + // Only stamp OK on genuine success. onOperationEnd fires for every terminal status, and + // extractErrorFromOperation returns null for CANCELLED (always) and for FAILED/TIMED_OUT/STOPPED + // with no attached error object — those carry a non-null, non-SUCCEEDED status and must stay UNSET. + // A null status is a successful statusless virtual (FLAT CONTEXT) operation, which is OK. span.setStatus(StatusCode.OK); } span.end(); @@ -475,7 +479,9 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { continuationSpan.setStatus(StatusCode.ERROR, info.error().getMessage()); continuationSpan.recordException(info.error()); - } else { + } else if ("SUCCEEDED".equals(info.status()) || info.status() == null) { + // See onOperationEnd (this-invocation branch): only genuine success (or a successful statusless + // virtual operation) is OK; error-less non-success statuses stay UNSET. continuationSpan.setStatus(StatusCode.OK); } diff --git a/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/ExecutionOtelPluginTest.java b/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/ExecutionOtelPluginTest.java index 6418c9a16..c1601a2ea 100644 --- a/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/ExecutionOtelPluginTest.java +++ b/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/ExecutionOtelPluginTest.java @@ -372,6 +372,52 @@ void operationSuccess_setsOkOnOperationSpan() { assertEquals(StatusCode.OK, operationSpan.getStatus().getStatusCode()); } + @Test + void operationEnd_withNonSuccessStatusAndNoError_leavesOperationSpanUnset() { + // onOperationEnd fires for every terminal status. A CANCELLED operation (or an error-less + // FAILED/TIMED_OUT/STOPPED) carries a non-null, non-SUCCEEDED status with a null error. It must NOT be + // stamped OK — the span status stays UNSET. + plugin.onInvocationStart(new InvocationInfo("req-1", ARN, true, Instant.now())); + plugin.onOperationStart( + new OperationInfo("op-cancel", "step-cancel", "STEP", "Step", null, Instant.now(), null, false)); + plugin.onOperationEnd(new OperationEndInfo( + "op-cancel", "step-cancel", "STEP", "Step", null, Instant.now(), Instant.now(), "CANCELLED", null, + false, null)); + plugin.onInvocationEnd(new InvocationEndInfo("req-1", ARN, true, InvocationStatus.SUCCEEDED, null)); + + var operationSpan = spanByName(spanExporter.getFinishedSpanItems(), "step-cancel"); + assertEquals(StatusCode.UNSET, operationSpan.getStatus().getStatusCode()); + } + + @Test + void operationEnd_withoutStart_nonSuccessStatusAndNoError_leavesContinuationSpanUnset() { + // Same guard on the continuation-span branch (operation completed between invocations): an error-less + // TIMED_OUT terminal status must NOT be stamped OK. + plugin.onInvocationStart(new InvocationInfo("req-2", ARN, false, Instant.now())); + plugin.onOperationEnd(new OperationEndInfo( + "op-cb-timeout", "my-callback", "CALLBACK", "Callback", null, Instant.now(), Instant.now(), + "TIMED_OUT", null, false, null)); + plugin.onInvocationEnd(new InvocationEndInfo("req-2", ARN, false, InvocationStatus.SUCCEEDED, null)); + + var continuationSpan = spanByName(spanExporter.getFinishedSpanItems(), "my-callback"); + assertEquals(StatusCode.UNSET, continuationSpan.getStatus().getStatusCode()); + } + + @Test + void operationEnd_withNullStatusAndNoError_setsOkOnOperationSpan() { + // A successful statusless virtual (FLAT CONTEXT) operation fires onOperationEnd with a null operation -> + // null status and null error. This is genuine success and must be stamped OK. + plugin.onInvocationStart(new InvocationInfo("req-1", ARN, true, Instant.now())); + plugin.onOperationStart( + new OperationInfo("op-ctx", "my-ctx", "CONTEXT", null, null, Instant.now(), null, false)); + plugin.onOperationEnd(new OperationEndInfo( + "op-ctx", "my-ctx", "CONTEXT", null, null, Instant.now(), Instant.now(), null, null, false, null)); + plugin.onInvocationEnd(new InvocationEndInfo("req-1", ARN, true, InvocationStatus.SUCCEEDED, null)); + + var operationSpan = spanByName(spanExporter.getFinishedSpanItems(), "my-ctx"); + assertEquals(StatusCode.OK, operationSpan.getStatus().getStatusCode()); + } + @Test void operationNotCompleted_notEndedAtInvocationEnd() { plugin.onInvocationStart(new InvocationInfo("req-1", ARN, true, Instant.now())); diff --git a/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/InvocationOtelPluginTest.java b/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/InvocationOtelPluginTest.java index bb736945b..7f371ff08 100644 --- a/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/InvocationOtelPluginTest.java +++ b/otel-plugin/src/test/java/software/amazon/lambda/durable/otel/InvocationOtelPluginTest.java @@ -491,6 +491,67 @@ void operationEnd_withSuccess_setsOkOnOperationSpan() { assertEquals(StatusCode.OK, operationSpan.getStatus().getStatusCode()); } + @Test + void operationEnd_withNonSuccessStatusAndNoError_leavesOperationSpanUnset() { + // onOperationEnd fires for every terminal status. A CANCELLED operation (or an error-less + // FAILED/TIMED_OUT/STOPPED) carries a non-null, non-SUCCEEDED status with a null error. It must NOT be + // stamped OK — the span status stays UNSET. + plugin.onInvocationStart(new InvocationInfo("req-1", "arn:exec1", true, Instant.now())); + + plugin.onOperationStart( + new OperationInfo("op-cancel", "step-cancel", "STEP", "Step", null, Instant.now(), null, false)); + plugin.onOperationEnd(new OperationEndInfo( + "op-cancel", "step-cancel", "STEP", "Step", null, Instant.now(), Instant.now(), "CANCELLED", null, + false, null)); + + plugin.onInvocationEnd(new InvocationEndInfo("req-1", "arn:exec1", true, InvocationStatus.SUCCEEDED, null)); + + var operationSpan = spanExporter.getFinishedSpanItems().stream() + .filter(s -> "step-cancel".equals(s.getName())) + .findFirst() + .orElseThrow(); + assertEquals(StatusCode.UNSET, operationSpan.getStatus().getStatusCode()); + } + + @Test + void operationEnd_withoutMatchingStart_nonSuccessStatusAndNoError_leavesContinuationSpanUnset() { + // Same guard on the continuation-span branch (operation completed between invocations): an error-less + // TIMED_OUT terminal status must NOT be stamped OK. + plugin.onInvocationStart(new InvocationInfo("req-1", "arn:exec1", true, Instant.now())); + + plugin.onOperationEnd(new OperationEndInfo( + "op-cb-timeout", "my-callback", "CALLBACK", "Callback", null, Instant.now(), Instant.now(), + "TIMED_OUT", null, false, null)); + + plugin.onInvocationEnd(new InvocationEndInfo("req-1", "arn:exec1", true, InvocationStatus.SUCCEEDED, null)); + + var continuationSpan = spanExporter.getFinishedSpanItems().stream() + .filter(s -> s.getName().contains("callback")) + .findFirst() + .orElseThrow(); + assertEquals(StatusCode.UNSET, continuationSpan.getStatus().getStatusCode()); + } + + @Test + void operationEnd_withNullStatusAndNoError_setsOkOnOperationSpan() { + // A successful statusless virtual (FLAT CONTEXT) operation fires onOperationEnd with a null operation -> + // null status and null error. This is genuine success and must be stamped OK. + plugin.onInvocationStart(new InvocationInfo("req-1", "arn:exec1", true, Instant.now())); + + plugin.onOperationStart( + new OperationInfo("op-ctx", "my-ctx", "CONTEXT", null, null, Instant.now(), null, false)); + plugin.onOperationEnd(new OperationEndInfo( + "op-ctx", "my-ctx", "CONTEXT", null, null, Instant.now(), Instant.now(), null, null, false, null)); + + plugin.onInvocationEnd(new InvocationEndInfo("req-1", "arn:exec1", true, InvocationStatus.SUCCEEDED, null)); + + var operationSpan = spanExporter.getFinishedSpanItems().stream() + .filter(s -> "my-ctx".equals(s.getName())) + .findFirst() + .orElseThrow(); + assertEquals(StatusCode.OK, operationSpan.getStatus().getStatusCode()); + } + @Test void fullLifecycle_producesCorrectSpanHierarchy() { var arn = "arn:aws:lambda:us-east-1:123:function:test:$LATEST/durable/exec1";