feat: add Workflow Insight plugin - #661
Conversation
5d83fc9 to
9cb8201
Compare
GitHub Actions
Failure
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| <module>sdk-testing</module> | ||
| <module>sdk-integration-tests</module> | ||
| <module>otel-plugin</module> | ||
| <module>insight-plugin</module> |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| */ | ||
| @Deprecated | ||
| public final class Json { | ||
| static final ObjectMapper MAPPER = new ObjectMapper(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| exporter.export(shaped); | ||
| exporter.flush(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| if (operations == null) { | ||
| return out; | ||
| } | ||
| for (OperationChangeItemInfo item : operations.values()) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| private static ErrorInfo toErrorInfo(Throwable t) { | ||
| return new ErrorInfo(t.getClass().getSimpleName(), t.getMessage()); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| if (!include || value == null) { | ||
| return null; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| public List<OperationRecord> operations() { | ||
| return operations; |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| return false; | ||
| } | ||
| long unsigned = fnv1a32(executionArn) & 0xffffffffL; | ||
| return (double) unsigned / 0xffffffffL < rate; |
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.
|
Use /ai review command to trigger AI review workflows for draft PRs |
d90f91f to
bdbd88f
Compare
|
/ai review |
| if (state == null || !state.sampledIn) { | ||
| return; | ||
| } | ||
| emit(buildRecord( |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_xryomwjxrzqt3z55wozggpsrsl
[P2] Keep ON_CHANGE exports off checkpoint processing
onOperationChange runs synchronously while checkpoint coordination is still active. This call performs S3/CloudWatch I/O and flush() inline for every change, so a slow exporter stalls subsequent checkpoints and can cause invocation timeouts. Queue immutable snapshots here and drain/flush them asynchronously, awaiting completion from onInvocationEnd; add a delayed-exporter test proving this hook returns promptly.
| if (!state.sampledIn) { | ||
| return; | ||
| } | ||
| state.cachedInput = info.executionInput(); |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_cyyzil3ide53kqls24cwcz3cts
[P2] Snapshot execution input before user code can mutate it
executionInput() is the same mutable object passed to the handler. Caching its reference lets handler mutations rewrite later insight records; in ON_CHANGE mode, a mutating input transform can even alter the object before the handler runs. Create and cache a detached snapshot before applying transforms, use it for every emission including invocation end, and test both handler and transform mutations.
| if (isTerminal) { | ||
| byArn.remove(info.durableExecutionArn()); |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_qh6xoafzze3z3ccgrbppucmunr
[P2] Do not retain suspended executions indefinitely
A PENDING/RETRYING execution may resume in another Lambda environment, so this warm container may never observe its terminal event and byArn grows without bound—even when sampling is disabled. Each new invocation already supplies the stable execution start time and input. Remove state on every invocation end and recreate it at the next start, or use a bounded expiry strategy; test a paired suspend/resume lifecycle.
| * is applied (omit on throw so a failing redactor never leaks the raw value); otherwise include as-is. | ||
| */ | ||
| static Object applyDataContent(Object value, boolean include, Function<Object, Object> transform) { | ||
| if (!include || value == null) { |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_oirdg3rnpdnwvpldafyiqrnncz
[P2] Preserve included JSON null values
This conflates an enabled value that is actually null with content disabled by configuration. A successful handler returning null therefore emits no output field despite output inclusion defaulting to true. Track field presence separately or use an explicit JSON-null sentinel, omitting only unavailable/disabled content; add null input/output tests.
| // own class/message would lose the original checkpointed failure identity. When the checkpointed ErrorObject is | ||
| // present, derive name/message from its errorType/errorMessage, falling back to the throwable's own fields for | ||
| // any value the ErrorObject leaves null. | ||
| if (t instanceof DurableOperationException doe && doe.getErrorObject() != null) { |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_hp5bcyxuzbh2vgqafztcdoghc5
[P2] Unwrap unrecoverable execution errors too
DurableExecutor also supplies UnrecoverableDurableExecutionException for nondeterminism, illegal operations, and durable API failures. This branch only unwraps DurableOperationException, so those records lose ErrorObject.errorType—including service error codes—and report the generic wrapper class. Extract the ErrorObject from both wrapper types through a shared helper and add terminal and retrying error tests.
| <module>sdk-testing</module> | ||
| <module>sdk-integration-tests</module> | ||
| <module>otel-plugin</module> | ||
| <module>insight-plugin</module> |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_luk5otghmahig7iqbjsa7g67gu
[P2] Trigger the build for insight-plugin-only changes
The main build workflow's path filters do not include insight-plugin/**. This PR runs because it also changes the root POM, but subsequent changes confined to the module will skip the multi-JDK Maven build and its tests. Add insight-plugin/** to both the pull-request and push filters in .github/workflows/build.yml.
| static Object deepCopyContent(Object value) { | ||
| if (value == null | ||
| || value instanceof String | ||
| || value instanceof Number |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_rzwdoyquezpr2qgby63imtmxcn
[P3] Do not treat every Number as immutable
Number includes mutable types such as AtomicInteger, AtomicLong, and custom implementations. Returning them unchanged means a first exporter can mutate a payload value that later exporters still share, defeating the isolation guarantee. Share only known immutable numeric classes or convert arbitrary numbers into detached JSON scalars; add an AtomicInteger isolation test.
| return false; | ||
| } | ||
| long unsigned = fnv1a32(executionArn) & 0xffffffffL; | ||
| return (double) unsigned / 0xffffffffL < rate; |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_47kz274xvx4sow4mmbm7gzor7a
[P3] Normalize sampling into the documented half-open interval
Dividing by the maximum unsigned 32-bit value maps 0xffffffff to exactly 1.0, producing [0,1] rather than [0,1) and shifting deterministic sampling boundaries. Coordinate the denominator with the other SDKs, divide by 2^32, and add fixed-vector compatibility tests.
| return (double) unsigned / 0xffffffffL < rate; | |
| return (double) unsigned / 0x1_0000_0000L < rate; |
Codex AI reviewFound eight actionable issues affecting checkpoint latency, input isolation, lifecycle cleanup, record fidelity, CI coverage, exporter isolation, and sampling. Tests were not run per review constraints. Reviewed commit |
2 participants
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
N/A. Workflow Insight conformance requirements are being reviewed in aws/aws-durable-execution-conformance-tests#73.
Description
Adds the preview Workflow Insight plugin as a new
insight-pluginMaven module.The plugin emits schema-versioned execution records from the existing Java plugin hooks and matches the JavaScript Workflow Insight contract. It includes:
operationsByNamerecord shapesThe implementation uses the invocation operation snapshots, execution payloads, errors, attempts, and serialized operation results already exposed by the Java plugin API. No core hook changes are required.
Demo/Screenshots
No UI change.
insight-plugin/README.mdcontains a usage example. The draft conformance PR linked above contains Java handlers for all 18 Workflow Insight requirements.Checklist
Testing
Unit Tests
Yes. The module adds 35 tests covering record construction, sampling, emission modes, replay/suspension state, operation filtering, result transforms, aggregation, truncation, exporter requests, failure isolation, and flush behavior.
mvn -q -pl insight-plugin -am test mvn -q -pl insight-plugin -am clean verifyBoth pass.
The full repository reactor also passes:
Integration Tests
Yes.
WorkflowInsightPluginTestexercises the plugin throughLocalDurableTestRunner.The draft Workflow Insight conformance suite was also run against deployed Java 21 Lambda functions in
us-west-2:operationsByNameleg: 18 passed, 0 failedSink-specific assertions were capability-gated and exercised by the complementary leg.
Examples
The module README includes configuration and exporter usage. Customer-facing conformance handlers are in aws/aws-durable-execution-conformance-tests#73 rather than this repository, following the repository's conformance-test ownership rule.