From 46df98d499b15b984e06719649145e8e894559ee Mon Sep 17 00:00:00 2001 From: nirsky Date: Tue, 22 Jun 2021 18:41:01 +0300 Subject: [PATCH 1/5] feat(aws-lambda): disableAwsPropagation config option --- .../README.md | 1 + .../src/instrumentation.ts | 48 +++++++++-------- .../src/types.ts | 1 + .../test/integrations/lambda-handler.test.ts | 51 +++++++++++++++++++ 4 files changed, 81 insertions(+), 20 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md b/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md index 40c73b8c88..200f9f653c 100644 --- a/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md +++ b/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md @@ -46,6 +46,7 @@ In your Lambda function configuration, add or update the `NODE_OPTIONS` environm | --- | --- | --- | | `requestHook` | `RequestHook` (function) | Hook for adding custom attributes before lambda starts handling the request. Receives params: `span, { event, context }` | | `responseHook` | `ResponseHook` (function) | Hook for adding custom attributes before lambda returns the response. Receives params: `span, { err?, res? } ` | +| `disableAwsPropagation` | `boolean` | By default, this instrumentation will try to get the context using `propagator-aws-xray`, set this to `true` to disable this behavior | ### Hooks Usage Example diff --git a/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts index 53bf84c90d..898676d555 100644 --- a/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts @@ -151,7 +151,10 @@ export class AwsLambdaInstrumentation extends InstrumentationBase { ) { const httpHeaders = typeof event.headers === 'object' ? event.headers : {}; - const parent = AwsLambdaInstrumentation._determineParent(httpHeaders); + const parent = AwsLambdaInstrumentation._determineParent( + httpHeaders, + Boolean(plugin._config.disableAwsPropagation) + ); const name = context.functionName; const span = plugin.tracer.startSpan( @@ -298,29 +301,34 @@ export class AwsLambdaInstrumentation extends InstrumentationBase { } private static _determineParent( - httpHeaders: APIGatewayProxyEventHeaders + httpHeaders: APIGatewayProxyEventHeaders, + disableAwsPropagation: boolean ): OtelContext { let parent: OtelContext | undefined = undefined; - const lambdaTraceHeader = process.env[traceContextEnvironmentKey]; - if (lambdaTraceHeader) { - parent = awsPropagator.extract( - otelContext.active(), - { [AWSXRAY_TRACE_ID_HEADER]: lambdaTraceHeader }, - headerGetter - ); - } - if (parent) { - const spanContext = trace.getSpan(parent)?.spanContext(); - if ( - spanContext && - (spanContext.traceFlags & TraceFlags.SAMPLED) === TraceFlags.SAMPLED - ) { - // Trace header provided by Lambda only sampled if a sampled context was propagated from - // an upstream cloud service such as S3, or the user is using X-Ray. In these cases, we - // need to use it as the parent. - return parent; + + if (!disableAwsPropagation) { + const lambdaTraceHeader = process.env[traceContextEnvironmentKey]; + if (lambdaTraceHeader) { + parent = awsPropagator.extract( + otelContext.active(), + { [AWSXRAY_TRACE_ID_HEADER]: lambdaTraceHeader }, + headerGetter + ); + } + if (parent) { + const spanContext = trace.getSpan(parent)?.spanContext(); + if ( + spanContext && + (spanContext.traceFlags & TraceFlags.SAMPLED) === TraceFlags.SAMPLED + ) { + // Trace header provided by Lambda only sampled if a sampled context was propagated from + // an upstream cloud service such as S3, or the user is using X-Ray. In these cases, we + // need to use it as the parent. + return parent; + } } } + // There was not a sampled trace header from Lambda so try from HTTP headers. const httpContext = propagation.extract( otelContext.active(), diff --git a/plugins/node/opentelemetry-instrumentation-aws-lambda/src/types.ts b/plugins/node/opentelemetry-instrumentation-aws-lambda/src/types.ts index a761a25fbb..3bd68595a9 100644 --- a/plugins/node/opentelemetry-instrumentation-aws-lambda/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-aws-lambda/src/types.ts @@ -36,4 +36,5 @@ export type ResponseHook = ( export interface AwsLambdaInstrumentationConfig extends InstrumentationConfig { requestHook?: RequestHook; responseHook?: ResponseHook; + disableAwsPropagation?: boolean; } diff --git a/plugins/node/opentelemetry-instrumentation-aws-lambda/test/integrations/lambda-handler.test.ts b/plugins/node/opentelemetry-instrumentation-aws-lambda/test/integrations/lambda-handler.test.ts index 5b089210a7..38a9330c47 100644 --- a/plugins/node/opentelemetry-instrumentation-aws-lambda/test/integrations/lambda-handler.test.ts +++ b/plugins/node/opentelemetry-instrumentation-aws-lambda/test/integrations/lambda-handler.test.ts @@ -526,6 +526,57 @@ describe('lambda handler', () => { // Parent unsampled so no spans exported. assert.strictEqual(spans.length, 0); }); + + it('ignores sampled lambda context if "disableAwsPropagation" config option is true', async () => { + process.env[traceContextEnvironmentKey] = sampledAwsHeader; + initializeHandler('lambda-test/async.handler', { + disableAwsPropagation: true, + }); + + const result = await lambdaRequire('lambda-test/async').handler( + 'arg', + ctx + ); + assert.strictEqual(result, 'ok'); + const spans = memoryExporter.getFinishedSpans(); + const [span] = spans; + assert.strictEqual(spans.length, 1); + assertSpanSuccess(span); + assert.notDeepStrictEqual( + span.spanContext().traceId, + sampledAwsSpanContext.traceId + ); + assert.strictEqual(span.parentSpanId, undefined); + }); + + it('takes sampled http context over sampled lambda context if "disableAwsPropagation" config option is true', async () => { + process.env[traceContextEnvironmentKey] = sampledAwsHeader; + initializeHandler('lambda-test/async.handler', { + disableAwsPropagation: true, + }); + + const proxyEvent = { + headers: { + traceparent: sampledHttpHeader, + }, + }; + + const result = await lambdaRequire('lambda-test/async').handler( + proxyEvent, + ctx + ); + + assert.strictEqual(result, 'ok'); + const spans = memoryExporter.getFinishedSpans(); + const [span] = spans; + assert.strictEqual(spans.length, 1); + assertSpanSuccess(span); + assert.strictEqual( + span.spanContext().traceId, + sampledHttpSpanContext.traceId + ); + assert.strictEqual(span.parentSpanId, sampledHttpSpanContext.spanId); + }); }); describe('hooks', () => { From abc4f0b9537d8724ae395dd3ca952dc31713f098 Mon Sep 17 00:00:00 2001 From: nirsky Date: Sun, 27 Jun 2021 09:59:00 +0300 Subject: [PATCH 2/5] code review fixes --- plugins/node/opentelemetry-instrumentation-aws-lambda/README.md | 2 +- .../src/instrumentation.ts | 2 +- .../node/opentelemetry-instrumentation-aws-lambda/src/types.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md b/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md index 200f9f653c..249f202e20 100644 --- a/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md +++ b/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md @@ -46,7 +46,7 @@ In your Lambda function configuration, add or update the `NODE_OPTIONS` environm | --- | --- | --- | | `requestHook` | `RequestHook` (function) | Hook for adding custom attributes before lambda starts handling the request. Receives params: `span, { event, context }` | | `responseHook` | `ResponseHook` (function) | Hook for adding custom attributes before lambda returns the response. Receives params: `span, { err?, res? } ` | -| `disableAwsPropagation` | `boolean` | By default, this instrumentation will try to get the context using `propagator-aws-xray`, set this to `true` to disable this behavior | +| `disableAwsContextPropagation` | `boolean` | default, this instrumentation will try to read the context from the `_X_AMZN_TRACE_ID` environment variable set by Lambda, set this to `true` to disable this behavior | ### Hooks Usage Example diff --git a/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts index 898676d555..4aeda2eb09 100644 --- a/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts @@ -153,7 +153,7 @@ export class AwsLambdaInstrumentation extends InstrumentationBase { typeof event.headers === 'object' ? event.headers : {}; const parent = AwsLambdaInstrumentation._determineParent( httpHeaders, - Boolean(plugin._config.disableAwsPropagation) + plugin._config.disableAwsContextPropagation === true ); const name = context.functionName; diff --git a/plugins/node/opentelemetry-instrumentation-aws-lambda/src/types.ts b/plugins/node/opentelemetry-instrumentation-aws-lambda/src/types.ts index 3bd68595a9..d5bbffe3d0 100644 --- a/plugins/node/opentelemetry-instrumentation-aws-lambda/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-aws-lambda/src/types.ts @@ -36,5 +36,5 @@ export type ResponseHook = ( export interface AwsLambdaInstrumentationConfig extends InstrumentationConfig { requestHook?: RequestHook; responseHook?: ResponseHook; - disableAwsPropagation?: boolean; + disableAwsContextPropagation?: boolean; } From 9832d3a1f17c4a97a516cd2d6eaa6f66bed5ab70 Mon Sep 17 00:00:00 2001 From: nirsky Date: Sun, 27 Jun 2021 10:10:03 +0300 Subject: [PATCH 3/5] tests: fixed wrong config name --- .../test/integrations/lambda-handler.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-aws-lambda/test/integrations/lambda-handler.test.ts b/plugins/node/opentelemetry-instrumentation-aws-lambda/test/integrations/lambda-handler.test.ts index 38a9330c47..2abd2fbd5c 100644 --- a/plugins/node/opentelemetry-instrumentation-aws-lambda/test/integrations/lambda-handler.test.ts +++ b/plugins/node/opentelemetry-instrumentation-aws-lambda/test/integrations/lambda-handler.test.ts @@ -527,10 +527,10 @@ describe('lambda handler', () => { assert.strictEqual(spans.length, 0); }); - it('ignores sampled lambda context if "disableAwsPropagation" config option is true', async () => { + it('ignores sampled lambda context if "disableAwsContextPropagation" config option is true', async () => { process.env[traceContextEnvironmentKey] = sampledAwsHeader; initializeHandler('lambda-test/async.handler', { - disableAwsPropagation: true, + disableAwsContextPropagation: true, }); const result = await lambdaRequire('lambda-test/async').handler( @@ -549,10 +549,10 @@ describe('lambda handler', () => { assert.strictEqual(span.parentSpanId, undefined); }); - it('takes sampled http context over sampled lambda context if "disableAwsPropagation" config option is true', async () => { + it('takes sampled http context over sampled lambda context if "disableAwsContextPropagation" config option is true', async () => { process.env[traceContextEnvironmentKey] = sampledAwsHeader; initializeHandler('lambda-test/async.handler', { - disableAwsPropagation: true, + disableAwsContextPropagation: true, }); const proxyEvent = { From 4f586201873f8aa29ae3545bb313a73ae2f5b78b Mon Sep 17 00:00:00 2001 From: nirsky Date: Sun, 27 Jun 2021 10:52:38 +0300 Subject: [PATCH 4/5] cosmetic: renamed var --- .../src/instrumentation.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts index 4aeda2eb09..c65f55b4c2 100644 --- a/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-aws-lambda/src/instrumentation.ts @@ -302,11 +302,11 @@ export class AwsLambdaInstrumentation extends InstrumentationBase { private static _determineParent( httpHeaders: APIGatewayProxyEventHeaders, - disableAwsPropagation: boolean + disableAwsContextPropagation: boolean ): OtelContext { let parent: OtelContext | undefined = undefined; - if (!disableAwsPropagation) { + if (!disableAwsContextPropagation) { const lambdaTraceHeader = process.env[traceContextEnvironmentKey]; if (lambdaTraceHeader) { parent = awsPropagator.extract( From d8a499562530564405047901206f74043c62cadb Mon Sep 17 00:00:00 2001 From: nirsky Date: Sun, 27 Jun 2021 13:25:04 +0300 Subject: [PATCH 5/5] docs: fixed type --- plugins/node/opentelemetry-instrumentation-aws-lambda/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md b/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md index 249f202e20..8c70998629 100644 --- a/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md +++ b/plugins/node/opentelemetry-instrumentation-aws-lambda/README.md @@ -46,7 +46,7 @@ In your Lambda function configuration, add or update the `NODE_OPTIONS` environm | --- | --- | --- | | `requestHook` | `RequestHook` (function) | Hook for adding custom attributes before lambda starts handling the request. Receives params: `span, { event, context }` | | `responseHook` | `ResponseHook` (function) | Hook for adding custom attributes before lambda returns the response. Receives params: `span, { err?, res? } ` | -| `disableAwsContextPropagation` | `boolean` | default, this instrumentation will try to read the context from the `_X_AMZN_TRACE_ID` environment variable set by Lambda, set this to `true` to disable this behavior | +| `disableAwsContextPropagation` | `boolean` | By default, this instrumentation will try to read the context from the `_X_AMZN_TRACE_ID` environment variable set by Lambda, set this to `true` to disable this behavior | ### Hooks Usage Example