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..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,6 +308,12 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { span.setStatus(StatusCode.ERROR, info.error().getMessage()); span.recordException(info.error()); + } 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()); } else { @@ -350,6 +356,10 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { continuationSpan.setStatus(StatusCode.ERROR, info.error().getMessage()); continuationSpan.recordException(info.error()); + } 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); } endSpan(continuationSpan, info.endTimestamp()); @@ -438,6 +448,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 e8224e77b..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,6 +430,12 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { span.setStatus(StatusCode.ERROR, info.error().getMessage()); span.recordException(info.error()); + } 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(); } else { @@ -473,6 +479,10 @@ public void onOperationEnd(OperationEndInfo info) { if (info.error() != null) { continuationSpan.setStatus(StatusCode.ERROR, info.error().getMessage()); continuationSpan.recordException(info.error()); + } 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); } continuationSpan.end(); @@ -568,6 +578,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..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 @@ -344,6 +344,80 @@ 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 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 39971f667..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 @@ -456,6 +456,102 @@ 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 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";