feat(otel): add no-arg ExecutionOtelPlugin constructor with ADOT Java agent SPI

[ayushiahjolia]

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

#541

Description

• Added no-arg ExecutionOtelPlugin() constructor (same ADOT Java agent SPI pattern as InvocationOtelPlugin)
• Moved common default-constructor logic (SPI validation, GlobalOpenTelemetry lookup, flush handling, diagnostics) into shared OtelPluginSupport
• Fixed a bug where the Workflow span's deterministic ID wasn't bridged across classloaders via system properties (setNextSpanId now writes a per-thread property like setNextSpanOperationId already did)
• Added MDC trace_id injection at invocation start for log correlation
• Added two examples: OtelXRayExecutionStepExample and OtelXRayExecutionWaitExample with tests
• Removed redundant OtelXRayDefaultConstructorExample (covered by existing OtelXRayStepExample)
• Hardened CloudBasedOtelIntegrationTest with retry logic for X-Ray span ingestion delays (fixes [Bug]: Flaky e2e Otel test #475)
• Fixed pre-existing spotless violations in conformance-tests module

Demo/Screenshots

N/A

Checklist

• I have filled out every section of the PR template
• I have thoroughly tested this change

Testing

Unit Tests

Have unit tests been written for these changes? Yes

Integration Tests

Have integration tests been written for these changes? Yes

Examples

Has a new example been added for the change? (if applicable) Yes

[github-actions]

Workflow root span ID is non-deterministic with the no-arg constructor, breaking the "single logical span" contract.

This private constructor wires this.idGenerator to a fresh DeterministicIdGenerator (from createDefaultIdGenerator()), but the spans are actually produced by the global TracerProvider, whose ID generator is the separate static instance installed by OtelPluginAutoConfigurationCustomizerProvider.ID_GENERATOR. The two instances coordinate only through System properties.

onInvocationStart sets the Workflow span ID via idGenerator.setNextSpanId(idGenerator.generateWorkflowSpanId()), but DeterministicIdGenerator.setNextSpanId writes only the pendingRawSpanId ThreadLocal (no System property). When the global provider's generator runs generateSpanId(), its own pendingRawSpanId is null and there is no property fallback, so it falls through to RANDOM.generateSpanId().

Failure scenario: a suspended (multi-invocation) execution using new ExecutionOtelPlugin(). Each invocation's Workflow span gets a different, random span ID. Operations completed between invocations are re-materialized in onOperationEnd and parented (via resolveParentContext → workflowSpan) to that invocation's random Workflow span ID, which is dropped un-exported for non-terminal invocations. The terminal Workflow span is exported with yet another random ID. Result: orphaned operation spans and a Workflow that no longer stitches into one logical span across invocations — the central design goal documented on this class.

Note: operation span IDs survive because setNextSpanOperationId does write a thread-keyed System property that the global generator reads; only the raw-span-ID path (setNextSpanId) lacks that channel.

Fix: have the no-arg path share the same generator instance the agent installed (expose OtelPluginAutoConfigurationCustomizerProvider.ID_GENERATOR) rather than constructing a new one, or add a thread-keyed System-property channel to setNextSpanId/generateSpanId mirroring the operation-ID mechanism.

[github-actions]

Workflow root span gets a non-deterministic ID in the no-arg (ADOT agent) path, orphaning operations across invocations.

The no-arg constructor delegates to createDefaultIdGenerator(), which (per the mirrored InvocationOtelPlugin comment) is intentionally a separate instance from the SPI-installed ID_GENERATOR that the GlobalOpenTelemetry provider actually uses to mint span IDs. State is bridged between the two instances only through system properties: setDurableExecutionArn, setExtractedTraceId, and setNextSpanOperationId all call setOrClearProperty.

But onInvocationStart sets the Workflow root span ID via idGenerator.setNextSpanId(idGenerator.generateWorkflowSpanId()), and DeterministicIdGenerator.setNextSpanId (DeterministicIdGenerator.java:84-86) writes only to the pendingRawSpanId ThreadLocal — it does not bridge via a system property. In the no-arg path the SPI generator therefore never sees this value and falls through to RANDOM.generateSpanId(), so the Workflow root span receives a fresh random ID on every invocation.

Because this plugin is workflow-rooted (operations are parented to the Workflow span, unlike InvocationOtelPlugin where it is only a link anchor), this breaks cross-invocation stitching for suspend/resume executions — the durable core use case:

• Invocation 1 (PENDING): Workflow span = random R1 (never exported, non-terminal). An operation that starts and ends in inv 1 is exported with parent R1.
• Invocation 2 (SUCCEEDED, terminal): Workflow span = random R2, exported with R2. The inv-1 operation's parent R1 ≠ R2 is dangling — it references a root span that is never exported.

Fix: bridge the raw span ID the same way operation IDs are bridged — add a setOrClearProperty for a raw-span-ID property in setNextSpanId, and have generateSpanId() fall back to reading it (mirroring the pendingSpanOperationId handling). Builder-based constructors are unaffected because they pass this same instance to tracerProviderBuilder.setIdGenerator(idGenerator).

Note the added defaultConstructor_usesGlobalSdkTracerProviderDirectly test cannot catch this: it registers a global provider without the SPI DeterministicIdGenerator and asserts only span count/names, never that the Workflow span ID is deterministic/stable across invocations.

[github-actions]

Workflow root span loses its deterministic ID in the no-arg (ADOT) path, breaking multi-invocation traces.

This new no-arg constructor delegates to the private ExecutionOtelPlugin(TracerProvider, DeterministicIdGenerator) overload, which creates spans from the global/SPI-installed tracer provider — whose DeterministicIdGenerator is a different instance than this plugin's idGenerator (see OtelPluginAutoConfigurationCustomizerProvider.ID_GENERATOR vs. createDefaultIdGenerator()). State is bridged between the two instances only through system properties.

onInvocationStart sets the Workflow root span's ID via idGenerator.setNextSpanId(idGenerator.generateWorkflowSpanId()), but DeterministicIdGenerator.setNextSpanId(...) writes only the pendingRawSpanId ThreadLocal — unlike setNextSpanOperationId(...), it sets no system property. So the SPI generator that actually creates the span never sees the raw ID and falls through to RANDOM.generateSpanId(). The Workflow span therefore gets a random ID on every invocation under new ExecutionOtelPlugin().

Because ExecutionOtelPlugin makes the Workflow span the structural trace root (operations with no parent hang off workflowSpan), this orphans operations across invocations. Failure scenario (exactly the new OtelXRayExecutionWaitExample): invocation 1 runs exec-before-wait, exports its operation span parented to invocation‑1's random Workflow ID, then suspends (Workflow span never exported this invocation); the terminal invocation exports the Workflow span with a different random ID. The before-wait operation's parent pointer references a span ID that is never emitted → broken/orphaned hierarchy. The builder constructors are unaffected because they install this plugin's own idGenerator on the provider, so the raw-ID ThreadLocal is honored.

Fix: bridge the raw span ID the same way operation IDs are bridged — have setNextSpanId(...) also write a per-thread system property, and add a corresponding read in generateSpanId() before the operation-ID fallback (mirroring setNextSpanOperationId/pendingSpanOperationIdProperty()). The existing unit test doesn't catch this because it asserts span names, not the Workflow span ID's stability across invocations; a replay/multi-invocation test asserting a stable Workflow span ID under the SPI path would guard it.

[github-actions]

Test coverage gap: the new cross-instance setNextSpanId bridge is untested.

This added setOrClearProperty(pendingRawSpanIdProperty(), spanId) is the crux of the PR: it lets the Workflow root span's deterministic ARN-derived ID cross from the plugin's own DeterministicIdGenerator to the separate SPI-installed generator on the global provider (default-constructor / ADOT agent path), via a per-thread system property that generateSpanId() now reads.

No test exercises this cross-instance path for a raw span ID:

• ExecutionOtelPluginTest.defaultConstructor_usesGlobalSdkTracerProviderDirectly builds its global provider with SdkTracerProvider.builder()...build() — it never installs a DeterministicIdGenerator, so the global provider uses random IDs and the property bridge is never triggered. It asserts only span names and count (3), not that the Workflow span carries the deterministic generateWorkflowSpanId() value.
• DeterministicIdGeneratorTest.generatedIds_areSharedAcrossGeneratorInstances covers setNextSpanOperationId/trace-ID sharing across two instances, but not setNextSpanId.

As a result, a regression that dropped this system-property write (reverting to ThreadLocal-only) would silently ship: the Workflow span would get a random ID from the SPI generator and no longer stitch into one logical span across invocations, yet every test would still pass.

Suggested fix: add a DeterministicIdGeneratorTest case that sets setNextSpanId(...) on one instance and asserts a second instance's generateSpanId() returns that exact value (then clears the property), mirroring the existing cross-instance test.

[SilanHe]

hmm this is intended right? (I don't really know how we monkey patched the idGenerator in Java). Can you double check? all the comments below seem to relate to this.

[ayushiahjolia]

yes pushed new changes after this comment, will check why its not auto resolved.

[github-actions]

Span-level MDC (span_id/traceSampled) is no longer cleared after a user function, leaving stale values in inter-step handler logs.

This PR moves MDC lifecycle to the invocation boundary (MDC.put(trace_id) in onInvocationStart, MdcSpanEnricher.clear() in onInvocationEnd) and deletes the MdcSpanEnricher.clear() that previously lived here in onUserFunctionEnd. But onUserFunctionStart still calls MdcSpanEnricher.inject(), which sets trace_id, span_id, and traceSampled from the attempt span. With the clear removed and nothing put in its place, span_id/traceSampled from the just-completed attempt persist until onInvocationEnd.

The sibling InvocationOtelPlugin.onUserFunctionEnd handles this precisely — it does MDC.remove(MdcSpanEnricher.MDC_SPAN_ID) (keeping trace_id for handler-level logs between steps). Any handler-thread log emitted between operations (e.g. context.getLogger().info(...) between steps) will therefore be tagged with the previous step's span_id, mis-correlating those logs to a completed child span rather than the invocation/workflow.

Fix: mirror InvocationOtelPlugin here — remove the span-level keys while retaining trace_id:

if (enableMdc) {
    MDC.remove(MdcSpanEnricher.MDC_SPAN_ID);
    MDC.remove(MdcSpanEnricher.MDC_TRACE_SAMPLED);
}

[github-actions]

Codex AI review

[P2] Clear attempt-level MDC when user functions end — otel-plugin/src/main/java/software/amazon/lambda/durable/otel/ExecutionOtelPlugin.java:239

Cleanup now occurs only at invocation end, often on a different thread. span_id therefore remains associated with an ended attempt during later plugin hooks and checkpoint logging. Mirror InvocationOtelPlugin: remove MDC_SPAN_ID in onUserFunctionEnd while retaining invocation-level trace_id, and add an MDC lifecycle test for ExecutionOtelPlugin.

Reviewed commit 7e0e0cc4550d72f6f527186927f50a1cff659e77. Workflow run

[github-actions]

MDC span_id/traceSampled now leak past a step attempt (log-correlation regression).

This PR moves MDC lifecycle to invocation scope: trace_id is put here at onInvocationStart and cleared in onInvocationEnd. But it also removed the MdcSpanEnricher.clear() call from onUserFunctionEnd (diff hunk @@ -420,10 +464,6 @@). onUserFunctionStart still calls MdcSpanEnricher.inject(), which puts trace_id, span_id, and traceSampled onto the worker thread's MDC. Nothing now removes span_id/traceSampled when the attempt ends.

Failure scenario: a step attempt runs on worker thread T, inject() sets span_id=A; the attempt ends but MDC on T is left as {trace_id, span_id=A, traceSampled}. onInvocationEnd only clears the handler thread's MDC, so T keeps the stale values. Any subsequent log emitted on T before the next onUserFunctionStart re-injects (SDK-internal logging, or a reused pooled thread across a later invocation) is attributed to the already-closed attempt span A — the very log/trace-correlation mismatch this feature is meant to prevent.

InvocationOtelPlugin.onUserFunctionEnd deliberately does MDC.remove(MdcSpanEnricher.MDC_SPAN_ID) (keeping trace_id) for exactly this reason. Restore parity here — in onUserFunctionEnd, after the scope is closed, remove the span-scoped keys:

if (enableMdc) {
    MDC.remove(MdcSpanEnricher.MDC_SPAN_ID);
    MDC.remove(MdcSpanEnricher.MDC_TRACE_SAMPLED);
}

[github-actions]

Claude AI review

Review

One confirmed finding (medium severity).

MDC span_id/traceSampled leak past step attempts — log-correlation regression

otel-plugin/src/main/java/software/amazon/lambda/durable/otel/ExecutionOtelPlugin.java:233 (diff hunk @@ -420,10 +464,6 @@ removing the onUserFunctionEnd clear; MDC put added at onInvocationStart ~line 233)

This PR shifts MDC handling in ExecutionOtelPlugin to invocation scope: trace_id is put in onInvocationStart and MdcSpanEnricher.clear() runs in onInvocationEnd. At the same time it removes the MdcSpanEnricher.clear() that used to run in onUserFunctionEnd. But onUserFunctionStart still calls MdcSpanEnricher.inject(), which puts trace_id, span_id, and traceSampled on the worker thread's MDC. After that, nothing removes span_id/traceSampled when the attempt finishes.

Failure scenario: a step attempt runs on worker thread T and injects span_id=A; the attempt ends leaving T's MDC as {trace_id, span_id=A, traceSampled}. onInvocationEnd only clears the handler thread's MDC, so T retains the stale values. Any log emitted on T before the next inject() (SDK-internal logging, or a pooled worker thread reused in a later invocation) is attributed to the already-closed attempt span A — precisely the log↔trace mismatch this feature aims to prevent. The sibling InvocationOtelPlugin.onUserFunctionEnd deliberately keeps trace_id but does MDC.remove(MDC_SPAN_ID) for this reason; parity was lost here.

Fix: in onUserFunctionEnd, after closing the scope, remove the span-scoped keys (MDC.remove(MdcSpanEnricher.MDC_SPAN_ID) and MDC_TRACE_SAMPLED), keeping trace_id for between-step handler logs.

Residual test risk

• MDC behavior is untested. ExecutionOtelPluginTest constructs the plugin with enableMdc=false, and the new default-constructor test (enableMdc=true) asserts only span counts/names, never MDC contents or cleanup. The leak above would not be caught by any existing test — a unit test asserting MDC state after onUserFunctionEnd/onInvocationEnd would close the gap.
• The rest of the change set (shared OtelPluginSupport extraction, DeterministicIdGenerator.setNextSpanId system-property bridge with matching test, null-guarded forceFlush, extractCurrentSpanContext/setExtractedTraceId(null) fallbacks matching InvocationOtelPlugin, X-Ray integration-test retry loop, new examples/tests, and the spotless/Javadoc reflow in conformance-tests) looks correct and consistent with the existing sibling plugin. No further actionable issues found.

Reviewed commit 7e0e0cc4550d72f6f527186927f50a1cff659e77. Workflow run