Repository navigation
fix(telemetry)!: align error names and log lines with JS - #814
ParidelPooya wants to merge 9 commits into
Conversation
- error.name is the simple type name of the user's error, the same live
and after replay (last segment of the checkpointed errorType).
- includeErrors(false) drops only per-operation errors and keeps the
execution-level error, as in the JS exporter.
- LambdaLogExporter prints the Lambda JSON log envelope
{timestamp, level, message} so lambda-log-exporter readers that filter
on level and parse message find Java records.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @Override | ||
| public void export(DurableExecutionTelemetryRecord record) { | ||
| System.out.println(Json.stringify(render(record))); | ||
| System.out.println(logLine(Json.stringify(render(record)), Instant.now())); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| String simple = type.getSimpleName(); | ||
| if (simple.isEmpty() || type.isAnonymousClass() || type.isLocalClass()) { | ||
| return simpleTypeName(type.getName()); | ||
| } | ||
| return new ErrorInfo(t.getClass().getSimpleName(), t.getMessage()); | ||
| return simple; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
- Reduce type strings exactly as Table A of the shared rule: last segment after '.' or '$', kept as is when that segment is empty, no trimming, and "Error" for a missing, empty or blank type (no wrapper-class fallback). - Derive live names from Class.getName() through the same reduction, so a class named Payment$Error is "Error" live and replayed, and anonymous and local classes follow Table A (pkg.Foo$1Local -> "1Local"). The anonymous/local special case is removed. - Treat a throwable as an SDK wrapper only when its class is declared in software.amazon.lambda.durable.exception. A user subclass of an SDK exception reports its own name. CallbackTimeoutException is never unwrapped. - When an operation's checkpoint has no error type, name it after the SDK exception the core throws for it (for example CallbackFailedException), so the operation and the execution agree. A timed-out callback with no checkpointed error now gets an operation error named CallbackTimeoutException with a null message. - Load error-name-vectors.json (shared with the JS and Python exporters) into a parameterized test, and run every Table B scenario that applies to Java through LocalDurableTestRunner, live and replayed. BREAKING CHANGE: error.name values change. A blank or missing checkpointed error type reports "Error" instead of the wrapper class name, names are no longer trimmed, a user subclass of an SDK exception reports its own name instead of its cause's, and a callback failed without an error type reports CallbackFailedException on the operation as well as the execution.
… error includeErrors(false) drops only per-operation errors, as in the JS exporter; the execution-level error and its message are always emitted. Document this in the ContentConfig javadoc, say that no setting hides the execution error message, and tell users to keep secrets out of exception messages. Rename the ErrorPrivacyGateTest constants and tests so they describe this intended behaviour instead of a "secret" that leaks. BREAKING CHANGE: in 2.3.0, includeErrors(false) also suppressed the execution-level error. It no longer does; FAILED records carry the execution error name and message whatever the setting.
…ine size - README: add the shared error.name rule in the same words as the JS and Python exporters, with the Java specifics; document the includeErrors change. - LambdaLogExporter: the printed line is the record plus 68 bytes plus one byte per quote or backslash, so up to 2 x maxRecordSizeBytes + 68; this is within the 1 MB CloudWatch Logs event limit. Replace the stale 256 KB comment and the "about 70 bytes" claim. Note that the INFO lines are dropped at an application log level of WARN or above. - LambdaLogExporterTest: assert the exact line size instead of the always-true "line > record", and check an escape-heavy record near the default limit.
ParidelPooya
changed the title
| @Override | ||
| public void export(DurableExecutionTelemetryRecord record) { | ||
| System.out.println(Json.stringify(render(record))); | ||
| System.out.println(logLine(Json.stringify(render(record)), Instant.now())); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
- Rule 6: blank means only space, \t, \n, \r, \f and \u000B, instead of String.isBlank(). U+001C to U+001F and Unicode spaces are kept. - Rule 3: the depth counts unwrap steps. At most 16 unwraps happen; if the error reached after them is still a wrapper, that wrapper's name (its checkpointed errorType) is reported. 16 wrappers give the user error, 17 and 18 give wrapper 17. A cycle stops at the first repeat and reports the last error visited before it, for two-cycles and self-cycles alike. - Wrappers are an explicit list matched on the exact runtime class, not "anything in the SDK exception package". StepInterruptedException, IllegalDurableOperationException, NonDeterministicExecutionException, InvokeTimedOutException and InvokeStoppedException are SDK errors. A timed-out or stopped invoke reports InvokeTimedOutException or InvokeStoppedException on the operation and the execution, whatever error type the checkpoint carries. - A hidden class name is cut at the first '/' before the reduction, live and from the checkpoint, so the JVM address suffix never reaches error.name. - Tests: 1/15/16/17/18/40 wrappers, two-cycle, self-cycle and a cycle after a prefix with exact names; a test that lists every class in the SDK exception package from the SDK jar and fails on an unclassified one; a hidden-class test; the blank characters. The test-only CyclicWrapperForTest in the SDK package is removed.
Replace error-name-vectors.json with the spec version 2 file, byte for byte. It adds A13 (the six ASCII whitespace characters give "Error") and A14 to A17 (U+00A0, U+FEFF, U+001C and U+2003 are not blank and are kept). The test now asserts version 2, and its javadoc states that A12 and A13 hold on the record path only for operation types without an SDK failure class.
- Add B13b (callback failed with errorType "Error" reports "Error" on the operation and the execution), B20 (two map items) and B21 (two parallel branches) with each item's and branch's own type, and B19 as not applicable (JS only). - Mark the execution cells of B5, B6, B20 and B21 as not applicable, with the reason in the test: Java map and parallel return failures as data, so no operation error propagates and the SDK never chooses between failures. The item, branch and step names are still asserted. - B17 asserts that the operation message equals the execution message. - Assert operation names on the final record, not only on earlier ON_CHANGE records. A replayed final record may omit an operation only when an ancestor context is in it as completed. - Invoke timed out and stopped report InvokeTimedOutException and InvokeStoppedException, live and replayed; a timed-out invoke has a null operation message.
- Rules 1 to 9 in the words of spec version 2, including the explicit callback ErrorType "Error" case. - Narrow the Timeout sentence to callback timeouts: InvokeTimedOutException does not contain "Timeout". - List the wrapper classes and the SDK errors; say that invoke timeout and stop are SDK errors and how they are named; describe the hidden-class cut. - Document the null operation message for timed-out callbacks and timed-out or stopped invokes without a checkpointed error, and the execution message "Callback timed out: <callbackId>".
The envelope change came in an earlier commit of this branch without a
footer. Say in the README that the line shape changed and what a reader
of the old shape must do, and add the footer so release notes list it.
BREAKING CHANGE: LambdaLogExporter prints each record as a Lambda JSON log
line {"timestamp","level":"INFO","message":"<record JSON>"} instead of the
bare record JSON printed by 2.3.0. Readers that parsed each stdout line as
a record must parse the message field.
| @Override | ||
| public void export(DurableExecutionTelemetryRecord record) { | ||
| System.out.println(Json.stringify(render(record))); | ||
| System.out.println(logLine(Json.stringify(render(record)), Instant.now())); |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_jzgzj7tw37mr7hpltjtnwnlltj
[P2] Include envelope escaping in the size safeguard
Supported nondefault budgets still permit oversized log events. With new LambdaLogExporter(1_000_000), input containing 300,000 backslashes produces roughly 600 KB of rendered JSON and passes truncation, but this second serialization expands the line to roughly 1.2 MB. A previously fitting record now exceeds the documented 1 MB event limit. Measure the escaped envelope during truncation or enforce a separate final-line cap, and add a regression test covering a nondefault budget.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
| if (op.type() == OperationType.CHAINED_INVOKE && op.status() == OperationStatus.STOPPED) { | ||
| return InvokeStoppedException.class.getSimpleName(); |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_v6ltodna4ihizso4qep2gdhqxv
[P2] Preserve terminal failure identity through enclosing contexts
When an invoke inside runInChildContext stops with errorType = "States.Stopped", this branch names the invoke InvokeStoppedException. However, ChildContextOperation.handleChildContextFailure copies the original error object into its context checkpoint and reconstructs a ChildContextFailedException; this exporter therefore names the enclosing context and execution Stopped. The same propagated failure now has different names, whereas previously all three retained States.Stopped. Preserve the SDK terminal-error identity through context checkpointing and replay, and add nested stop/timeout scenarios to the integration coverage.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
| int slash = className.indexOf('/'); | ||
| return simpleTypeName(slash < 0 ? className : className.substring(0, slash)); |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_rqfthioukz3pqjlxt2tfrb3hwt
[P3] Restrict slash stripping to hidden-class suffixes
wrapperName also passes externally supplied callback and invoke error types through this helper. An external type such as payments/v1.PaymentError consequently becomes payments, although the shared reduction rule yields PaymentError. This loses the actual failure identity and breaks cross-SDK grouping. Strip only recognized Java hidden-class address suffixes; otherwise apply simpleTypeName to the complete string. Add callback/invoke coverage for external types containing /.
To suppress repeats, reply /ai verdict deferred, /ai verdict false-positive, or /ai verdict out-of-scope on the first nonblank line. Add rationale on later lines. A new /ai verdict accepted allows future repeats again.
Codex AI reviewThree findings:
Static review only; tests and Lambda log ingestion were not exercised. Reviewed commit |
This branch was successfully deployed
1 participant
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
None. Found by comparing the Java telemetry exporter with the JS exporter, whose readers consume records from every SDK. This PR is the Java part of a shared
error.namerule that the JS (aws/aws-durable-execution-sdk-js#1052) and Python (aws/aws-durable-execution-sdk-python#803) exporters are adopting at the same time.Description
Summary
Three changes in
telemetry-exporter, so Java records match what the cross-SDK readers expect:error.namefollows the shared cross-SDK rule (spec version 2).includeErrors(false)drops only per-operation errors, as in JS. The execution-level error is always emitted.LambdaLogExporterprints the Lambda JSON log envelope{timestamp, level, message}.RUNNING
endTime/durationMs, record timestamp precision, operation ordering, map iteration names and the exporter interface are unchanged. There are no public or protected API changes; the new helpers are package-private.The exporter is
@Experimental, but 2.3.0 shipped with the old behaviour, so these must be listed in the release notes:includeErrors(false)no longer hides the execution-level error. In 2.3.0 it suppressed both the executionerrorand the operation errors. Now FAILED records always carry the executionerror.nameanderror.message; onlyoperations[].errorandoperationsByName.<name>.errorare dropped. In Java the execution error is usually the failed operation's exception rethrown, so its message is usually the operation's message too. No setting suppresses it, and the input/output transforms do not apply to errors. Users must keep secrets and personal data out of exception messages that reach the handler's return path (catch and rethrow with a safe message), or redacterrorin a custom exporter'srender. This is documented in the README and theContentConfigjavadoc.error.namevalues change (details below): simple names instead of binary names for operation errors and checkpointed types;Errorinstead of the wrapper class name for a blank type; no trimming; user subclasses of SDK exceptions report their own name; callback failures without a type reportCallbackFailedExceptionon the operation too; a timed-out or stopped invoke reportsInvokeTimedOutExceptionorInvokeStoppedExceptionwhatever error type it carries (a stop withStates.StoppedreportedStopped); timed-out callbacks and timed-out invokes without a checkpointed error now have an operationerrorwith"message": null(they had noerror).LambdaLogExporterstdout lines are the{timestamp, level, message}envelope instead of the bare record. Consumers that parsed the bare record from the line must parsemessage.Each of the three has a
BREAKING CHANGEfooter:5b0ce870(error.name),5f281eef(includeErrors) and0d12f6c7(envelope line).1.
error.nameThe README states the shared rule in the same words as the other SDKs (spec version 2):
.or$. If that segment is empty, the string is kept as it is. A missing type, or a type made only of blank characters, isError. The blank characters are the six ASCII whitespace characters: space,\t,\n,\r,\fand\v. No other character counts as blank, and names are never trimmed.error.nameequals that operation'serror.name.A callback that is failed externally with an explicit
ErrorTypeis named after that type, after the reduction of rule 6. This includes the type"Error", which reportsError. Only a missing or blank type reports the SDK callback-failure class.How Java applies it:
Class.getName()(live) or the checkpointederrorType(replay). No trim (" PaymentError "stays as is). Missing, empty, or only space/\t/\n/\r/\f/\u000BgivesError; this is an explicit check, notString.isBlank(), so U+00A0, U+FEFF, U+001C and U+2003 are kept (A13–A17).trailing.,Outer$,.and$are kept as they are.pkg.Foo$1Localgives1Local./0x…, which changes on every JVM start. The name is cut at the first/before the reduction, live and from the checkpoint, sopkg.Gen$HiddenErr/0x…givesHiddenErron every cold start.DurableOperationException,UnrecoverableDurableExecutionException,StepException,CallbackException,InvokeException,StepFailedException,CallbackFailedException,CallbackSubmitterException,ChildContextFailedException,InvokeFailedException,MapIterationFailedException,ParallelBranchFailedException,WaitForConditionFailedException. Every other class insoftware.amazon.lambda.durable.exceptionis an SDK error that reports its simple class name:CallbackTimeoutException,InvokeTimedOutException,InvokeStoppedException,StepInterruptedException,IllegalDurableOperationException,NonDeterministicExecutionException,SerDesException,DurableExecutionException. A test reads every class of that package from the SDK jar and fails if one is in neither list, so a new SDK exception cannot become a wrapper without a decision. A user subclass of a wrapper is not a wrapper and reports its own name.CallbackTimeoutException,InvokeTimedOutExceptionorInvokeStoppedExceptionon the operation and the execution, whatever error type the checkpoint carries. A stopped invoke is not named after its stop error.InvokeTimedOutExceptiondoes not containTimeout; the README's/Timeout/sentence now covers callback timeouts only.errorType, reduced, never its class name when a type is available; with a blank type it is the SDK failure class of its operation.CallbackFailedException,StepFailedException,InvokeFailedException, …), so the operation and the execution agree. An explicit"Error"is a type and reportsError(B13b).nullmessage, because there is no original message. The execution message of a timed-out callback isCallback timed out: <callbackId>, built byCallbackTimeoutException; JS and Python reportCallback timed outwithout the id. Both are in the README.Depth and cycle vectors (rule 3)
The same inputs for JS and Python to compare (
ErrorNameTest). Wrapperk(1 is the outermost) is an SDK wrapper whose checkpointederrorTypeistest.W<k>; the innermost cause isPaymentError("card declined").error.name(Java, asserted)PaymentErrorPaymentErrorPaymentErrorW17W17W17test.A→test.B→test.A(start at A)BAtest.Self→test.SelfSelftest.P→test.A→test.B→test.ABEach SDK reports its own wrapper name in the
W17/B/Selfcells: Java uses the wrapper's checkpointederrorType, so the cells are comparable only in which wrapper is reported. The JDK never returns a throwable as its own cause and the SDK wrappers' causes are fixed at construction, so the cycle tests pass the chain to the walk through its cause lookup.Table B
Run through
LocalDurableTestRunner(ErrorNameScenarioTest). "Replayed" means the handler catches the error, waits so a new invocation starts, and rethrows the error rebuilt from the checkpoint. Every cell is the actual value and equals the expected value; every row also asserts the message, live == replayed, andoperationsByName==operations[]. Operation names are asserted on the final record as well as the latest record that carries the operation: a live final record must carry every listed operation; a replayed final record may omit one only when an ancestor context is in it as completed, because the core replays a completed context from its checkpoint without running its body (B4charge, B5/B6/B20 items, B10wfc-submitter).error.name(live / replayed)error.name(live / replayed)PaymentErrorPaymentError/PaymentErrorPaymentError/PaymentErrorTypeError, then on retry withPaymentErrorPaymentError/PaymentErrorPaymentError/PaymentErrorPaymentError/PaymentErrorPaymentError/PaymentErrorPaymentError/PaymentErrorPaymentError/PaymentErrorPaymentError/PaymentErrorPaymentError/PaymentErrorPaymentError/PaymentErrorPaymentError/PaymentErrorwaitForConditioncheck throwsPaymentError/PaymentErrorPaymentError/PaymentErrorPaymentError³PaymentError/PaymentErrorPaymentError/PaymentErrorwaitForCallbacksubmitter throwsPaymentError/PaymentErrorPaymentError/PaymentErrorCallbackTimeoutException/CallbackTimeoutExceptionCallbackTimeoutException/CallbackTimeoutExceptioncom.acme.PaymentErrorPaymentError/PaymentErrorPaymentError/PaymentErrornull,""or blank typeCallbackFailedException/CallbackFailedExceptionCallbackFailedException/CallbackFailedException"Error"Error/ErrorError/ErrorPaymentErrorwith causeTypeErrorPaymentError/PaymentErrorDurableOperationExceptionwith a causeUserOperationError/UserOperationErrorSerDesExceptionSerDesException/SerDesExceptionSerDesException/SerDesExceptionSerDesException/SerDesExceptionSerDesException/SerDesExceptionPayment$Errorthrown from a stepError/ErrorError/Errorpromise.allover a failing stepPaymentError, item 1FraudErrorPaymentError/PaymentError; item 1:FraudError/FraudErrorPaymentErrorandFraudErrorpay:PaymentError/PaymentError;fraud:FraudError/FraudError¹ Java
mapreturns item failures as data (MapResult.MapError) and never throws them. No operation error propagates; the test handler throws a newPaymentError, so the execution cell would test the handler, not rule 8. For B20 the SDK therefore never has to choose between the two items (rule 9). The item and step cells are real and asserted.² Java
parallelreturns aParallelResultof branch statuses and does not throw. Only the branch future the handler callsget()on throws, and it throws that branch's own error, so the SDK never has to choose between branches (rule 9). The branch cells are asserted.³ The invoke failure is simulated with
failChainedInvoke(errorType = PaymentError.class.getName()); it tests the checkpoint path, not the callee's own serialization.⁴ B16 targets Python's message-based cause matching. Java has no such matching, so the row checks the closest Java case: a handled user error and an SDK error with the same message.
⁵
promise.allis a JS combinator with no Java equivalent. The test case is reported as skipped with this reason.The N/A reasons are in the test (
EXECUTION_NOT_APPLICABLE, and the B19 skip message).Also asserted (
invokeTimeoutAndStopAreSdkErrors, live and replayed): invoke timed out →InvokeTimedOutExceptionon the operation (messagenull) and the execution; invoke stopped withStates.Stoppedorcom.acme.PaymentError→InvokeStoppedExceptionon both, operation message kept.Before/after for the edge cases (previous head
30fff3b3→ this head):30fff3b3PaymentErrorPaymentError(17 unwraps)W17(16 unwraps)UnrecoverableDurableExecutionException(class name)W17(checkpointed type)A↔BCyclicWrapperForTest(class name)BSelfSelfErrorError…$HiddenErr/0x…HiddenErr/0x…(changes per JVM)HiddenErrStates.StoppedStoppedInvokeStoppedException"Error"ErrorError(now asserted, B13b)2.
includeErrors(false)keeps the execution-level errorJS drops only per-operation errors (
buildOperationRecords:if (!opts.includeErrors) record.error = undefined) and always sets the executionerror. Java now does the same. This is intentional and is the first breaking change above; there is no new flag.ErrorPrivacyGateTestis renamed so it describes the intended behaviour (includeErrorsFalseDropsOnlyOperationErrorsAndStillEmitsTheExecutionErrorMessage).Record with
includeErrors(false), a step throwingPaymentError("card 4111-1111-1111-1111 declined"):3.
LambdaLogExporterwrites the Lambda JSON log envelopeThe JS readers for the
lambda-log-exporterdestination start withfilter level = "INFO" and message like /DurableExecutionTelemetry/and then parsemessage. The Node runtime adds that envelope aroundconsole.log. JavaSystem.outlines are not wrapped: per the Lambda log format docs, onlyLambdaLoggerand Log4j2 output gets the JSON envelope for Java, and "Lambda doesn't double-encode any logs that are already JSON encoded". So Java lines had nolevelormessage, and those queries returned nothing.timestamp,level,message(the same as the Python exporter).timestampis ISO-8601 UTC with 3 fraction digits andZ.requestIdis left out; the readers don't use it.render), not on the line. The record is embedded as a JSON string, so the line is the record + 68 bytes + one byte per"or\in the record: up to2 × maxRecordSizeBytes + 68, about 512 KB with the default 256,000. That is within the CloudWatch Logs limit of 1 MB per log event.INFO. With the JSON log format and an application log level ofWARNor above, Lambda drops them. The README says so.0d12f6c7carries the footer.Overlap with #809
#809 removes
@Experimentalfrom the same files.git merge-treereported two import-only conflicts on30fff3b3:DurableExecutionTelemetry.javaandLambdaLogExporter.java, where the new imports sit next to the removedimport software.amazon.lambda.durable.annotations.Experimental;. Resolution: keep this PR's imports and delete that one line in each file. README andContentConfigmerge cleanly. Re-checked on this head against #809640a0162: the same two files, nothing else.Demo/Screenshots
N/A. The before/after outputs are above.
Checklist
Testing
Unit Tests
Have unit tests been written for these changes? Yes.
ErrorNameVectorsTest: loadserror-name-vectors.jsonversion 2 (byte-identical in the JS, Python and Java repositories, sha256e7c54124…), asserts"version": 2, and checks every Table A row (A1–A17) through the reduction function and through a checkpointed operation error, with the message unchanged. A12/A13 hold on the record path only for operation types without an SDK failure class (see B13); the test javadoc says so.ErrorNameScenarioTest: every Table B row above, live and replayed, with N/A cells and their reasons; the final-record checks; the callback-timeout messages; invoke timed out and stopped.ErrorNameTest: the depth and cycle vectors above; every SDK exception class classified (read from the SDK jar); hidden class; the blank characters; live == checkpointed names for nested, anonymous, local and$-named classes; user subclass of an SDK exception; user error with a cause; blank types mapped to the SDK failure class; the SDK failure-class table.ErrorPrivacyGateTest,LambdaLogExporterTest: unchanged in this round.Commands and results on this head:
Integration Tests
Have integration tests been written for these changes? No new ones; the Table B scenarios run the real SDK through
LocalDurableTestRunner.Examples
Has a new example been added for the change? N/A.
Not verified
CallbackTimeoutException/InvokeTimedOutExceptioneither way (unit-tested), but only the local runner was exercised end to end.