feat(otel): Parent Workflow span onto shared execution trace - #647
feat(otel): Parent Workflow span onto shared execution trace#647ayushiahjolia wants to merge 1 commit into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
5c0f1a8 to
95fb7cd
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
95fb7cd to
d97ef9c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
d97ef9c to
6896e3d
Compare
This comment has been minimized.
This comment has been minimized.
338314b to
d226218
Compare
This comment has been minimized.
This comment has been minimized.
d226218 to
c0df70f
Compare
This comment has been minimized.
This comment has been minimized.
c0df70f to
0ff8de7
Compare
| private static final ContextKey<SamplingResult> KEY = | ||
| ContextKey.named("software.amazon.lambda.durable.otel.durable-sampling-decision"); |
There was a problem hiding this comment.
Codex AI review
P1: ContextKey uses reference identity, but the documented ADOT setup loads this JAR separately in the application and Java-agent extension classloaders. The application plugin stores the decision with one KEY, while the agent-loaded DurableSampler reads another, so it always delegates. For example, upstream Sampled=1 with always_off drops every span, contrary to the promised authoritative upstream decision. Bridge the decision across classloaders, analogous to DeterministicIdGenerator's thread-scoped system-property bridge, or use a bootstrap-shared carrier. Add a real dual-classloader test.
| * @param sampling the upstream sampling decision | ||
| */ | ||
| public record ExtractedContext(String traceId, String parentSpanId) {} | ||
| public record ExtractedContext(String traceId, String parentSpanId, Sampling sampling) { |
There was a problem hiding this comment.
Codex AI review
P2: The new public canonical constructor accepts a null sampling. This also occurs when Jackson reads legacy two-field serialized values. resolveSamplingResult then switches on null, throwing an exception that PluginRunner swallows and disabling telemetry for the invocation. Add a compact constructor that normalizes null to Sampling.UNDECIDED, and test legacy/missing-field deserialization.
Codex AI reviewTwo findings: the ADOT agent path can ignore authoritative sampling decisions, and null sampling values can disable telemetry. Reviewed commit |
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
#644
Description
Anchor every durable execution on one trace: the propagated remote server span is the execution ancestor when valid, else a deterministic synthetic root. Workflow and Invocation spans share the trace; sampling follows the header (explicit Sampled preserved, unset deferred to the configured sampler).
Also keep the local test runner's execution ARN stable across reinvocations to match the backend, and pin the OTel conformance workflow to the backend-parented topology assertions.
Demo/Screenshots
N/A
Checklist
Testing
Unit Tests
Have unit tests been written for these changes? Updated.
Integration Tests
Have integration tests been written for these changes? Added/Updated
Examples
Has a new example been added for the change? (if applicable) N/A