Repository navigation
feat(plugin): propagate chained invoke trace context in START checkpoints - #768
Conversation
|
/ai review Please review the current draft head, including the actual START wiring, replay/serialization coverage, and compatibility. The typed XAmznTraceId API intentionally assumes the pending generated Lambda model/backend update; the documented public-model compile failure is an accepted external dependency, and the real typed setter must remain. |
This comment has been minimized.
This comment has been minimized.
|
/ai review Please review current draft head b744341 including the new real-marshaller checkpoint batch-boundary regressions and encoded option sizing. The typed XAmznTraceId setter/getter intentionally assume the pending generated Lambda model and backend rollout; the public-model compiler failure remains an explicitly accepted external dependency. Do not remove or gate the real typed field to make CI green. |
This comment has been minimized.
This comment has been minimized.
Adopt current base dependency and workflow pins; explicitly retain the previous 20-case suite size. The upstream typed XAmznTraceId model prerequisite remains unresolved.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Codex AI reviewNo actionable findings in the PR changes. Review was static; tests were not executed, and deployed parent/child trace propagation was not verified. Reviewed commit |
| try { | ||
| logger.warn(message, arguments); | ||
| } catch (RuntimeException ignored) { | ||
| // A failed logging backend cannot change optional metadata collection or the handler's result. |
There was a problem hiding this comment.
Would it be useful to at least do a System.err.println() to help with local debugging should the logger throw an exception for any reason?
There was a problem hiding this comment.
It could help locally. I’ve kept this warning best effort: an ordinary logging failure must not change metadata collection or prevent the invoke START. Falling back unconditionally to System.err would bypass the application’s logging destination and filtering, including in production. The tradeoff is that this warning can be lost when the logger itself is broken; that would need diagnosis at the logging backend. I’m leaving this catch unchanged.
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
Refs #764. Coordinated PRs: JS #957 and Python #755.
Description
New invoke START checkpoints can carry the calling operation's X-Ray context in the optional typed ChainedInvokeOptions.XAmznTraceId field. The typed field uses the official AWS SDK 2.56.0 model inherited from main. Collection happens after the operation-start hook and payload serialization; batched invokes retain distinct operation parents, and request sizing includes the encoded options. Function, tenant, payload, operation identity and recorded outcomes retain their behavior.
The collector uses immutable SDK-owned input/contribution types and the first supported nonblank value. Conflicting contributions produce diagnostics without logging the header. Exception and LinkageError failures in an optional plugin are isolated so later plugins and the invoke START can proceed; other Error types retain their original propagation and identity.
The OTel sampling boundary is explicit. Invocation view uses an already-started operation span's real context. Execution view defers that span until completion, so it sends metadata only with a visible provider using DurableSampler. It abstains for replacement samplers and opaque provider/agent facades whose effective policy cannot be verified. Invocation view applies the same restriction to a planned context before an actual span is available. These cases retain the backend's omitted-header fallback; this PR does not promise new invoke-parent linkage through an unverifiable Execution-view provider. The fix does not query the sampler again, create an early span, change the configured sampling policy or inspect private provider internals.
The String-only producer overload avoids a dependency on the new metadata DTO classes in bundled plugin layers. It does not remove their existing same-thread invocation-hook capability requirement: cores lacking it, including released 2.2.1, are rejected at construction. Metadata collection additionally requires a core that implements the new hook.
Recorded START and terminal replay do not recollect metadata or resend START. A failed uncommitted START may recollect on retry. No new durable export acknowledgement, reflective model fallback, raw HTTP field injection or serializer replacement is introduced.
Validation
Current head
b97ebfc8fb5c65d77bd4757ba13cfc804cc5875cincorporates mainf82bde356b328f80373d5bd7d35fe65e6ba3f80d, including AWS SDK BOM 2.56.0, Lambda Java Core 1.4.1, Central publishing plugin 0.12.0 and the five upstream reusable-workflow reference updates. SDK source, tests and conformance workflow/catalog references remain byte-identical to preceding PR headb917d8075dfafb5f6412fce186be34979805ad8f; runtime dependencies were revalidated below.Current Java 17 validation built and installed all nine reactor modules: 3,018 tests passed, 31 conditional/cloud tests were skipped, and there were zero failures or errors. All 16 installed core/plugin/OTel-API compatibility probes passed, including the existing negative and unsupported-pair controls. Full Spotless checks passed. Dependency resolution confirmed AWS Lambda 2.56.0 and Lambda Java Core 1.4.1, and all four JAR coordinates/versions were checked. The five changed workflow callers passed YAML/input-contract checks against their pinned definitions. New-head CI, including the supported Java-version matrix and deployed conformance, is tracked separately.
Focused regression evidence retained from the preceding implementation:
Cloud coverage and scope
The requested first-START/completed-replay and actual child Root/Parent/Sampled checks are implemented in shared conformance PR #131, strengthening its existing case11. Its current composed-reference cloud run reports 108 passes and four case11 Parent/ancestry failures across the two views and two backends. This is not a qualified Java backend-topology result. Runtime observations and complete X-Ray ancestry remain separate from the client model/wire checks above. The current shared reference result is 108 passes and four Parent/ancestry failures; it does not qualify the deployed Java contract.
This PR remains ready for review; no package, tag or release is published. The #782 factory/lifetime migration and distributed-map propagation remain separate. Java's current map/parallel APIs use CONTEXT operations and do not expose a distributed-map START path here.