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> |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_v6wv3rjugx56mymsuekqwwjamc
[P1] Publish the new plugin artifact
Adding this module to the reactor builds it, but the release pipeline explicitly deploys and uploads only sdk, sdk-testing, and otel-plugin. Releases will therefore omit aws-durable-execution-sdk-java-plugin-insight, making the documented dependency unavailable from Maven Central. Add insight-plugin to .github/scripts/maven_publish.sh and the GitHub release artifact list, with a release-workflow check.
| */ | ||
| @Deprecated | ||
| public final class Json { | ||
| static final ObjectMapper MAPPER = new ObjectMapper(); |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_blkdgaotf7rzua2ga3uyouqm5e
[P1] Support the SDK's default payload types when exporting
This bare mapper cannot serialize Java-time values such as Instant, although the default JacksonSerDes supports them. If an included input or output contains one, size calculation fails and final serialization throws; emit catches that exception, silently losing every insight record. Configure the mapper with JavaTimeModule and ISO-8601 date serialization, declare the module dependency directly, and test an input/output containing an Instant.
| exporter.export(shaped); | ||
| exporter.flush(); |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_xryomwjxrzqt3z55wozggpsrsl
[P2] Keep remote exports off the checkpoint callback
emit is called synchronously from onOperationChange, which the SDK invokes before checkpoint futures and pollers are completed. Synchronous S3/CloudWatch calls and immediate flushing can therefore stall durable checkpoint coordination on every ON_CHANGE event, potentially causing invocation timeouts despite exception isolation. Queue these records on non-terminal hooks and perform/await serial export and flushing from onInvocationEnd; add a delayed-exporter test proving operation-change hooks return promptly.
| if (operations == null) { | ||
| return out; | ||
| } | ||
| for (OperationChangeItemInfo item : operations.values()) { |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_ig2yofnonmfn7zknekoxwd6xmg
[P2] Establish a deterministic chronological operation order
The hook contract supplies a map with no iteration-order guarantee, and the core snapshot originates from a concurrent map. Iterating values() produces unstable operation arrays; moreover, OperationsIndex treats the last encountered repeated name as the latest status/type, so an older occurrence may overwrite the actual latest one. Sort by startTimestamp with a stable ID tie-breaker before constructing records, and test unordered repeated-name snapshots with differing statuses.
| private static ErrorInfo toErrorInfo(Throwable t) { | ||
| return new ErrorInfo(t.getClass().getSimpleName(), t.getMessage()); |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_lndcqjp4vnrm6vjmmfzrqrfdin
[P2] Preserve the checkpointed operation error type
Operation snapshot errors are exposed as DurableOperationException wrappers by PluginInfoConverter, so this emits the generic wrapper name instead of the original checkpointed exception type. Insight records consequently lose the actionable failure identity. When the throwable is a DurableOperationException, derive the name and message from its ErrorObject, then fall back to the throwable fields; assert the exact original type and message in the failure test.
| if (!include || value == null) { | ||
| return null; |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_oirdg3rnpdnwvpldafyiqrnncz
[P2] Distinguish included JSON null from an omitted field
This conflates an enabled value that is actually null with content excluded by configuration, and the wire map subsequently omits both. A successful handler returning null therefore has no output field even though output inclusion defaults to true. Track field presence separately or use an explicit JSON-null sentinel, emitting null for included terminal values while omitting only unavailable or disabled content; add null input/output tests.
| public List<OperationRecord> operations() { | ||
| return operations; |
There was a problem hiding this comment.
Codex AI review · Finding arf_v1_rzwdoyquezpr2qgby63imtmxcn
[P2] Prevent one exporter from mutating later exporters' records
This exposes the live operations list, whose elements are also mutable. Because truncation returns the original record when it is already within the limit, all exporters then receive the same mutable object; a custom exporter that clears or redacts operations can corrupt every subsequent export. Make records immutable or provide each exporter a deep copy, and return an immutable snapshot here. Add an isolation test with a mutating first exporter.
| 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 the hash into the promised half-open interval
Dividing by the maximum unsigned 32-bit value maps 0xffffffff to exactly 1.0, producing [0,1] rather than the documented [0,1) and potentially differing from cross-SDK deterministic sampling decisions. Divide by 2^32 and add a fixed-vector compatibility test.
| return (double) unsigned / 0xffffffffL < rate; | |
| return (double) unsigned / 0x1_0000_0000L < rate; |
Codex AI reviewFound eight actionable issues, including release publication and record-loss blockers. Static review only; tests were not executed per request. Reviewed commit |
|
Use /ai review command to trigger AI review workflows for draft PRs |
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.