fix(otel): Fix X-Ray trace integration and cross-invocation retry spans#508
fix(otel): Fix X-Ray trace integration and cross-invocation retry spans#508ayushiahjolia wants to merge 1 commit into
Conversation
ayushiahjolia
force-pushed
the
fix/otel-plugin-xray-integration
branch
2 times, most recently
from
ec15f90 to
9a7cb23
Compare
ayushiahjolia
force-pushed
the
fix/otel-plugin-xray-integration
branch
from
9a7cb23 to
5c269c3
Compare
| - AdotArch: amd64 | ||
| Environment: | ||
| Variables: | ||
| AWS_LAMBDA_EXEC_WRAPPER: /opt/otel-handler |
There was a problem hiding this comment.
how come we don't need this anymore?
| Instant startTimestamp, | ||
| Instant endTimestamp) {} | ||
| Instant endTimestamp, | ||
| boolean isContinuation) {} |
There was a problem hiding this comment.
nit: we're using isReplay in JS (I think it can still be confusing but replay is a terminology that is more commonly used around durable functions). I can be convinced otherwise
There was a problem hiding this comment.
how is this new variable currently being used? I'm not seeing changes in the sdk itself
There was a problem hiding this comment.
Replay is applicable only to user functions in Context operations. I think we lack a term for the other operations.
| @@ -152,10 +161,17 @@ public void onInvocationStart(InvocationInfo info) { | |||
| } | |||
| // If no extracted context, idGenerator falls back to ARN-derived trace ID | |||
There was a problem hiding this comment.
Is idGenerator using the trace id from previous invocation?
|
|
||
| // Set service.name to "invocation" — X-Ray uses this as the display name for SERVER spans, | ||
| // creating a separate service node in the trace map labeled "invocation". | ||
| var resource = Resource.create(Attributes.of(AttributeKey.stringKey("service.name"), "invocation")); |
There was a problem hiding this comment.
nit: use SERVICE_NAME constant from opentelemetry package.
| // Set service.name to "invocation" — X-Ray uses this as the display name for SERVER spans, | ||
| // creating a separate service node in the trace map labeled "invocation". | ||
| var resource = Resource.create(Attributes.of(AttributeKey.stringKey("service.name"), "invocation")); | ||
| tracerProviderBuilder.setResource(resource); |
There was a problem hiding this comment.
This overrides what users have configured for the resource in tracerProviderBuilder. Consider merge the resources and override only service name.
3 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
#507
Description
Fixes OTel plugin X-Ray integration: cross-invocation retry span grouping, trace context extraction, and trace map display to match the JS SDK behavior.
System.getenv("_X_AMZN_TRACE_ID")returned stale/null values due to JVM env caching, so the plugin fell back to an ARN-derived trace ID disconnected from Lambda's X-Ray segments.service.nameresource andSpanKind.SERVERprevented X-Ray from creating a distinct service node for durable execution spans.Demo/Screenshots
Checklist
Testing
Unit Tests
Have unit tests been written for these changes? Yes
Integration Tests
Have integration tests been written for these changes? N/A
Examples
Has a new example been added for the change? (if applicable) Updated existing ones.