-
Notifications
You must be signed in to change notification settings - Fork 226
fix(nodejs): make aws-lambda and aws-sdk instrumentations respect OTEL_NODE_DISABLED_INSTRUMENTATIONS #2012
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…L_NODE_DISABLED_INSTRUMENTATIONS Extends PR open-telemetry#1653 to make AWS-specific instrumentations (aws-lambda, aws-sdk) also respect the OTEL_NODE_DISABLED_INSTRUMENTATIONS environment variable. Previously, these instrumentations were always loaded regardless of the disable flag. Now they follow the same conditional loading pattern as other instrumentations. Fixes: Users can now disable AWS Lambda auto-instrumentation in dev mode
|
Hi @DivMode, Could you please fix the lint issues. |
- Add trailing commas per Prettier rules - Fix object formatting with proper line breaks - Resolves CI lint failures in PR open-telemetry#2012
|
Hi @DivMode, in general this looks good to me. Thank you for your effort. I notice you've also made some changes to the MeterProvider config, but didn't mention the reasoning behind that change. |
wpessers
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also this is changing some fundamental behaviour documented here: https://github.com/open-telemetry/opentelemetry-lambda/blob/main/nodejs/README.md?plain=1#L10-L11
So that readme should be updated as well to reflect the change.
| const exportIntervalMillis = process.env.OTEL_METRIC_EXPORT_INTERVAL | ||
| ? parseInt(process.env.OTEL_METRIC_EXPORT_INTERVAL, 10) | ||
| : 60000; | ||
|
|
||
| const exportTimeoutMillis = process.env.OTEL_METRIC_EXPORT_TIMEOUT | ||
| ? parseInt(process.env.OTEL_METRIC_EXPORT_TIMEOUT, 10) | ||
| : 30000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the desire here is to fallback to the defaults provided in the metrics-sdk I'd prefer leaving these options undefined when the environment variables are not set. Otherwise we're just doing the same defaulting logic. I'm open to other default suggestions though.
|
@serkan-ozal we also need to consider, this is a breaking change. Because, if current nodejs layer users are already using the |
Problem
DynamoDB Streams and other AWS event sources lack metadata carriers for trace propagation. Unlike SQS (message attributes) or API Gateway (HTTP headers), DynamoDB Streams only contain the changed record data.
The Trace Propagation Issue
When using OpenTelemetry with DynamoDB Streams:
Producer writes to DynamoDB with trace context stored in record:
{ "data": "data", "_otel": { "traceparent": "00-7d7836d35ab7da657d35c2b5f889c3be-de0931d210e214b3-01" } }Consumer Lambda needs to extract and restore this context
Problem:
AwsLambdaInstrumentationcreates automatic wrapper span with NEW trace ID before handler code runs:Result: Distributed trace is broken - cannot correlate stream processing with original operation
Solution
This PR extends the functionality added in the base implementation to make
aws-lambdaandaws-sdkinstrumentations respectOTEL_NODE_DISABLED_INSTRUMENTATIONS.Previously, PR #1653 added the ability to disable instrumentations via
OTEL_NODE_DISABLED_INSTRUMENTATIONS, but the AWS Lambda and AWS SDK instrumentations were always loaded unconditionally in thecreateInstrumentations()function. This meant users couldn't disable these instrumentations even when setting the environment variable.Usage:
export OTEL_NODE_DISABLED_INSTRUMENTATIONS=aws-lambdaEffect: Disables automatic Lambda wrapper span, allowing handler to manually restore trace context from event data before creating spans.
Use Case: DynamoDB Streams Trace Propagation
Producer (stores trace context):
Consumer (restores trace context with disabled AwsLambdaInstrumentation):
Result: Full trace continuity from producer → DynamoDB → stream → consumer
Related Issues
Event Sources Affected
This enables manual trace propagation for AWS event sources without metadata carriers:
Industry Pattern
This approach aligns with industry best practices for DynamoDB Streams trace propagation:
Testing
aws-lambdaandaws-sdkadded todefaultInstrumentationListgetActiveInstumentations()OTEL_NODE_DISABLED_INSTRUMENTATIONS=aws-lambdaBenefits