Skip to content

Commit

Permalink
fix(tracer): avoid throwing errors in manual instrumentation when run…
Browse files Browse the repository at this point in the history 
…ning outside of AWS Lambda (#442)

* fix: added local testing behavior

* fix: correctly disable x-ray-sdk when tracer is disabled

* Update packages/tracing/tests/unit/Tracer.test.ts

Co-authored-by: Sara Gerion <[email protected]>

* Update packages/tracing/tests/unit/Tracer.test.ts

Co-authored-by: Sara Gerion <[email protected]>

* Update packages/tracing/tests/unit/Tracer.test.ts

Co-authored-by: Sara Gerion <[email protected]>

* Update packages/tracing/tests/unit/Tracer.test.ts

Co-authored-by: Sara Gerion <[email protected]>

Co-authored-by: Sara Gerion <[email protected]>
  • Loading branch information
@dreamorosi @saragerion
dreamorosi and saragerion authored Jan 10, 2022
1 parent b0b1cec commit fd02acb
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 20 deletions.
41 changes: 25 additions & 16 deletions packages/tracing/src/Tracer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ class Tracer implements TracerInterface {
public constructor(options: TracerOptions = {}) {
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(() => ({}));
}
}

/**
Expand All @@ -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;
}

Expand All @@ -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;
}

Expand All @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -222,7 +226,7 @@ class Tracer implements TracerInterface {
* @returns AWS - Instrumented AWS SDK
*/
public captureAWS<T>(aws: T): T {
if (this.tracingEnabled === false) return aws;
if (this.isTracingEnabled() === false) return aws;

return this.provider.captureAWS(aws);
}
Expand Down Expand Up @@ -251,7 +255,7 @@ class Tracer implements TracerInterface {
* @returns service - Instrumented AWS SDK v2 client
*/
public captureAWSClient<T>(service: T): T {
if (this.tracingEnabled === false) return service;
if (this.isTracingEnabled() === false) return service;

return this.provider.captureAWSClient(service);
}
Expand Down Expand Up @@ -281,7 +285,7 @@ class Tracer implements TracerInterface {
* @returns service - Instrumented AWS SDK v3 client
*/
public captureAWSv3Client<T>(service: T): T {
if (this.tracingEnabled === false) return service;
if (this.isTracingEnabled() === false) return service;

return this.provider.captureAWSv3Client(service);
}
Expand Down Expand Up @@ -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 ]);
}

Expand Down Expand Up @@ -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]);
}

Expand Down Expand Up @@ -458,11 +462,14 @@ class Tracer implements TracerInterface {
* @returns segment - The active segment or subsegment in the current scope.
*/
public getSegment(): Segment | Subsegment {
const segment = this.provider.getSegment();
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;
}

Expand Down Expand Up @@ -498,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) {
Expand Down Expand Up @@ -531,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) {
Expand Down Expand Up @@ -567,6 +574,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);
}

Expand Down Expand Up @@ -730,21 +739,21 @@ class Tracer implements TracerInterface {
private setTracingEnabled(enabled?: boolean): void {
if (enabled !== undefined && enabled === false) {
this.tracingEnabled = enabled;

return;
}

const customConfigValue = this.getCustomConfigService()?.getTracingEnabled();
if (customConfigValue !== undefined && customConfigValue.toLowerCase() === 'false') {
this.tracingEnabled = false;

return;
}

const envVarsValue = this.getEnvVarsService()?.getTracingEnabled();
if (envVarsValue.toLowerCase() === 'false') {
this.tracingEnabled = false;

return;
}

Expand Down
39 changes: 35 additions & 4 deletions packages/tracing/tests/unit/Tracer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 tracing is enabled, it throws an error', () => {

// Prepare
const tracer: Tracer = new Tracer();
Expand All @@ -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 tracing is enabled, it throws an error', () => {

// Prepare
const tracer: Tracer = new Tracer();
Expand All @@ -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 tracing is disabled, 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
Expand All @@ -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();
Expand All @@ -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 tracing is disabled, 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
Expand Down

0 comments on commit fd02acb

Please sign in to comment.