From e11c3b66feaf836cb80972c82666596766f049e7 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Fri, 7 Jan 2022 10:15:37 +0100 Subject: [PATCH 1/6] fix: added local testing behavior --- packages/tracing/src/Tracer.ts | 13 ++++++-- packages/tracing/tests/unit/Tracer.test.ts | 39 +++++++++++++++++++--- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/packages/tracing/src/Tracer.ts b/packages/tracing/src/Tracer.ts index d59a95cd99..a22e73155a 100644 --- a/packages/tracing/src/Tracer.ts +++ b/packages/tracing/src/Tracer.ts @@ -128,8 +128,8 @@ class Tracer implements TracerInterface { private tracingEnabled: boolean = true; public constructor(options: TracerOptions = {}) { - this.setOptions(options); this.provider = new ProviderService(); + this.setOptions(options); } /** @@ -458,7 +458,10 @@ class Tracer implements TracerInterface { * @returns segment - The active segment or subsegment in the current scope. */ public getSegment(): Segment | Subsegment { - const segment = this.provider.getSegment(); + let segment = this.provider.getSegment(); + if (segment === undefined && this.isTracingEnabled() === false) { + segment = new Subsegment('## Dummy segment'); + } if (segment === undefined) { throw new Error('Failed to get the current sub/segment from the context.'); } @@ -567,6 +570,8 @@ class Tracer implements TracerInterface { * @param segment - Subsegment to set as the current segment */ public setSegment(segment: Segment | Subsegment): void { + if (this.isTracingEnabled() === false) return; + return this.provider.setSegment(segment); } @@ -730,6 +735,7 @@ class Tracer implements TracerInterface { private setTracingEnabled(enabled?: boolean): void { if (enabled !== undefined && enabled === false) { this.tracingEnabled = enabled; + this.provider.setContextMissingStrategy(() => undefined); return; } @@ -737,6 +743,7 @@ class Tracer implements TracerInterface { const customConfigValue = this.getCustomConfigService()?.getTracingEnabled(); if (customConfigValue !== undefined && customConfigValue.toLowerCase() === 'false') { this.tracingEnabled = false; + this.provider.setContextMissingStrategy(() => undefined); return; } @@ -744,12 +751,14 @@ class Tracer implements TracerInterface { const envVarsValue = this.getEnvVarsService()?.getTracingEnabled(); if (envVarsValue.toLowerCase() === 'false') { this.tracingEnabled = false; + this.provider.setContextMissingStrategy(() => undefined); return; } if (this.isLambdaSamCli() || this.isLambdaExecutionEnv() === false) { this.tracingEnabled = false; + this.provider.setContextMissingStrategy(() => undefined); } } diff --git a/packages/tracing/tests/unit/Tracer.test.ts b/packages/tracing/tests/unit/Tracer.test.ts index df58ce22ba..ca4a2484cf 100644 --- a/packages/tracing/tests/unit/Tracer.test.ts +++ b/packages/tracing/tests/unit/Tracer.test.ts @@ -274,7 +274,7 @@ describe('Class: Tracer', () => { describe('Method: getSegment', () => { - test('when called outside of a namespace or without parent segment, it throws an error', () => { + test('when called outside of a namespace or without parent segment, and Tracer is active, it throws an error', () => { // Prepare const tracer: Tracer = new Tracer(); @@ -286,7 +286,7 @@ describe('Class: Tracer', () => { }); - test('when called outside of a namespace or without parent segment, it throws an error', () => { + test('when called and no segment is returned, while Tracer is active, it throws an error', () => { // Prepare const tracer: Tracer = new Tracer(); @@ -298,7 +298,22 @@ describe('Class: Tracer', () => { }).toThrow('Failed to get the current sub/segment from the context.'); }); - + + test('when called outside of a namespace or without parent segment, and Tracer is NOT active, it returns a dummy subsegment', () => { + + // Prepare + delete process.env.AWS_EXECUTION_ENV; // This will disable the tracer, simulating local execution + const tracer: Tracer = new Tracer(); + + // Act + const segment = tracer.getSegment(); + + // Assess + expect(segment).toBeInstanceOf(Subsegment); + expect(segment.name).toBe('## Dummy segment'); + + }); + test('when called within a namespace, it returns the parent segment', () => { // Prepare @@ -320,7 +335,7 @@ describe('Class: Tracer', () => { }); describe('Method: setSegment', () => { - test('when called outside of a namespace or without parent segment, it throws an error', () => { + test('when called outside of a namespace or without parent segment, and Tracer is enabled, it throws an error', () => { // Prepare const tracer: Tracer = new Tracer(); @@ -332,6 +347,22 @@ describe('Class: Tracer', () => { }).toThrow('No context available. ns.run() or ns.bind() must be called first.'); }); + test('when called outside of a namespace or without parent segment, and Tracer is NOT enabled, it does nothing', () => { + + // Prepare + delete process.env.AWS_EXECUTION_ENV; // This will disable the tracer, simulating local execution + const tracer: Tracer = new Tracer(); + const setSegmentSpy = jest.spyOn(tracer.provider, 'setSegment'); + + // Act + const newSubsegment = new Subsegment('## foo.bar'); + tracer.setSegment(newSubsegment); + + // Assess + expect(setSegmentSpy).toBeCalledTimes(0); + + }); + test('when called within a namespace, it sets the segment', () => { // Prepare From 434327a33fcab0f4495775edf3fbddfb0c7872f7 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Fri, 7 Jan 2022 16:31:18 +0100 Subject: [PATCH 2/6] fix: correctly disable x-ray-sdk when tracer is disabled --- packages/tracing/src/Tracer.ts | 46 +++++++++++++++++----------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/packages/tracing/src/Tracer.ts b/packages/tracing/src/Tracer.ts index a22e73155a..26253209ea 100644 --- a/packages/tracing/src/Tracer.ts +++ b/packages/tracing/src/Tracer.ts @@ -128,8 +128,12 @@ class Tracer implements TracerInterface { private tracingEnabled: boolean = true; public constructor(options: TracerOptions = {}) { - this.provider = new ProviderService(); this.setOptions(options); + this.provider = new ProviderService(); + if (this.isTracingEnabled() === false) { + // Tell x-ray-sdk to not throw an error if context is missing but tracing is disabled + this.provider.setContextMissingStrategy(() => ({})); + } } /** @@ -140,7 +144,7 @@ class Tracer implements TracerInterface { * @param error - Error to serialize as metadata */ public addErrorAsMetadata(error: Error): void { - if (this.tracingEnabled === false) { + if (this.isTracingEnabled() === false) { return; } @@ -163,7 +167,7 @@ class Tracer implements TracerInterface { * @param methodName - Name of the method that is being traced */ public addResponseAsMetadata(data?: unknown, methodName?: string): void { - if (data === undefined || this.captureResponse === false || this.tracingEnabled === false) { + if (data === undefined || this.captureResponse === false || this.isTracingEnabled() === false) { return; } @@ -175,7 +179,7 @@ class Tracer implements TracerInterface { * */ public addServiceNameAnnotation(): void { - if (this.tracingEnabled === false || this.serviceName === undefined) { + if (this.isTracingEnabled() === false || this.serviceName === undefined) { return; } this.putAnnotation('Service', this.serviceName); @@ -191,7 +195,7 @@ class Tracer implements TracerInterface { * @see https://docs.aws.amazon.com/lambda/latest/dg/runtimes-context.html */ public annotateColdStart(): void { - if (this.tracingEnabled === true) { + if (this.isTracingEnabled() === true) { this.putAnnotation('ColdStart', Tracer.coldStart); } if (Tracer.coldStart === true) { @@ -222,7 +226,7 @@ class Tracer implements TracerInterface { * @returns AWS - Instrumented AWS SDK */ public captureAWS(aws: T): T { - if (this.tracingEnabled === false) return aws; + if (this.isTracingEnabled() === false) return aws; return this.provider.captureAWS(aws); } @@ -251,7 +255,7 @@ class Tracer implements TracerInterface { * @returns service - Instrumented AWS SDK v2 client */ public captureAWSClient(service: T): T { - if (this.tracingEnabled === false) return service; + if (this.isTracingEnabled() === false) return service; return this.provider.captureAWSClient(service); } @@ -281,7 +285,7 @@ class Tracer implements TracerInterface { * @returns service - Instrumented AWS SDK v3 client */ public captureAWSv3Client(service: T): T { - if (this.tracingEnabled === false) return service; + if (this.isTracingEnabled() === false) return service; return this.provider.captureAWSv3Client(service); } @@ -322,7 +326,7 @@ class Tracer implements TracerInterface { const originalMethod = descriptor.value; descriptor.value = ((event, context, callback) => { - if (this.tracingEnabled === false) { + if (this.isTracingEnabled() === false) { return originalMethod?.apply(target, [ event, context, callback ]); } @@ -389,7 +393,7 @@ class Tracer implements TracerInterface { const originalMethod = descriptor.value; descriptor.value = (...args: unknown[]) => { - if (this.tracingEnabled === false) { + if (this.isTracingEnabled() === false) { return originalMethod?.apply(target, [...args]); } @@ -458,14 +462,14 @@ class Tracer implements TracerInterface { * @returns segment - The active segment or subsegment in the current scope. */ public getSegment(): Segment | Subsegment { - let segment = this.provider.getSegment(); - if (segment === undefined && this.isTracingEnabled() === false) { - segment = new Subsegment('## Dummy segment'); + if (this.isTracingEnabled() === false) { + return new Subsegment('## Dummy segment'); } + const segment = this.provider.getSegment(); if (segment === undefined) { throw new Error('Failed to get the current sub/segment from the context.'); } - + return segment; } @@ -501,7 +505,7 @@ class Tracer implements TracerInterface { * @param value - Value for annotation */ public putAnnotation(key: string, value: string | number | boolean): void { - if (this.tracingEnabled === false) return; + if (this.isTracingEnabled() === false) return; const document = this.getSegment(); if (document instanceof Segment) { @@ -534,7 +538,7 @@ class Tracer implements TracerInterface { * @param timestamp - Namespace that metadata will lie under, if none is passed it will use the serviceName */ public putMetadata(key: string, value: unknown, namespace?: string | undefined): void { - if (this.tracingEnabled === false) return; + if (this.isTracingEnabled() === false) return; const document = this.getSegment(); if (document instanceof Segment) { @@ -735,30 +739,26 @@ class Tracer implements TracerInterface { private setTracingEnabled(enabled?: boolean): void { if (enabled !== undefined && enabled === false) { this.tracingEnabled = enabled; - this.provider.setContextMissingStrategy(() => undefined); - + return; } const customConfigValue = this.getCustomConfigService()?.getTracingEnabled(); if (customConfigValue !== undefined && customConfigValue.toLowerCase() === 'false') { this.tracingEnabled = false; - this.provider.setContextMissingStrategy(() => undefined); - + return; } const envVarsValue = this.getEnvVarsService()?.getTracingEnabled(); if (envVarsValue.toLowerCase() === 'false') { this.tracingEnabled = false; - this.provider.setContextMissingStrategy(() => undefined); - + return; } if (this.isLambdaSamCli() || this.isLambdaExecutionEnv() === false) { this.tracingEnabled = false; - this.provider.setContextMissingStrategy(() => undefined); } } From 013bd58fbd2cc043dca3ec4f90ffed99f5b07040 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Mon, 10 Jan 2022 11:08:29 +0100 Subject: [PATCH 3/6] Update packages/tracing/tests/unit/Tracer.test.ts Co-authored-by: Sara Gerion <47529391+saragerion@users.noreply.github.com> --- packages/tracing/tests/unit/Tracer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tracing/tests/unit/Tracer.test.ts b/packages/tracing/tests/unit/Tracer.test.ts index ca4a2484cf..07a8cfe351 100644 --- a/packages/tracing/tests/unit/Tracer.test.ts +++ b/packages/tracing/tests/unit/Tracer.test.ts @@ -286,7 +286,7 @@ describe('Class: Tracer', () => { }); - test('when called and no segment is returned, while Tracer is active, it throws an error', () => { + test('when called and no segment is returned, while tracing is enabled, it throws an error', () => { // Prepare const tracer: Tracer = new Tracer(); From e9af86454916fce8661a36bfd103284151e3109f Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Mon, 10 Jan 2022 11:08:34 +0100 Subject: [PATCH 4/6] Update packages/tracing/tests/unit/Tracer.test.ts Co-authored-by: Sara Gerion <47529391+saragerion@users.noreply.github.com> --- packages/tracing/tests/unit/Tracer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tracing/tests/unit/Tracer.test.ts b/packages/tracing/tests/unit/Tracer.test.ts index 07a8cfe351..7157bfc488 100644 --- a/packages/tracing/tests/unit/Tracer.test.ts +++ b/packages/tracing/tests/unit/Tracer.test.ts @@ -299,7 +299,7 @@ describe('Class: Tracer', () => { }); - test('when called outside of a namespace or without parent segment, and Tracer is NOT active, it returns a dummy subsegment', () => { + test('when called outside of a namespace or without parent segment, and tracing is disabled, it returns a dummy subsegment', () => { // Prepare delete process.env.AWS_EXECUTION_ENV; // This will disable the tracer, simulating local execution From e8d10ee3609deb6023374fdda6c2b1fc6a6ee271 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Mon, 10 Jan 2022 11:08:39 +0100 Subject: [PATCH 5/6] Update packages/tracing/tests/unit/Tracer.test.ts Co-authored-by: Sara Gerion <47529391+saragerion@users.noreply.github.com> --- packages/tracing/tests/unit/Tracer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tracing/tests/unit/Tracer.test.ts b/packages/tracing/tests/unit/Tracer.test.ts index 7157bfc488..55c6133302 100644 --- a/packages/tracing/tests/unit/Tracer.test.ts +++ b/packages/tracing/tests/unit/Tracer.test.ts @@ -274,7 +274,7 @@ describe('Class: Tracer', () => { describe('Method: getSegment', () => { - test('when called outside of a namespace or without parent segment, and Tracer is active, it throws an error', () => { + test('when called outside of a namespace or without parent segment, and tracing is enabled, it throws an error', () => { // Prepare const tracer: Tracer = new Tracer(); From 06d5506f6feeb46f2997833bb791660aba1abaf5 Mon Sep 17 00:00:00 2001 From: Andrea Amorosi Date: Mon, 10 Jan 2022 11:08:45 +0100 Subject: [PATCH 6/6] Update packages/tracing/tests/unit/Tracer.test.ts Co-authored-by: Sara Gerion <47529391+saragerion@users.noreply.github.com> --- packages/tracing/tests/unit/Tracer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tracing/tests/unit/Tracer.test.ts b/packages/tracing/tests/unit/Tracer.test.ts index 55c6133302..5262fa46b6 100644 --- a/packages/tracing/tests/unit/Tracer.test.ts +++ b/packages/tracing/tests/unit/Tracer.test.ts @@ -347,7 +347,7 @@ describe('Class: Tracer', () => { }).toThrow('No context available. ns.run() or ns.bind() must be called first.'); }); - test('when called outside of a namespace or without parent segment, and Tracer is NOT enabled, it does nothing', () => { + test('when called outside of a namespace or without parent segment, and tracing is disabled, it does nothing', () => { // Prepare delete process.env.AWS_EXECUTION_ENV; // This will disable the tracer, simulating local execution