Skip to content
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.

Sign up for GitHub

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

fix(tracer): avoid throwing errors in manual instrumentation when running outside of AWS Lambda #442

Merged
merged 6 commits into from
Jan 10, 2022
Merged

fix(tracer): avoid throwing errors in manual instrumentation when running outside of AWS Lambda #442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e11c3b6
fix: added local testing behavior
dreamorosi Jan 7, 2022
434327a
fix: correctly disable x-ray-sdk when tracer is disabled
dreamorosi Jan 7, 2022
013bd58
Update packages/tracing/tests/unit/Tracer.test.ts
dreamorosi Jan 10, 2022
e9af864
Update packages/tracing/tests/unit/Tracer.test.ts
dreamorosi Jan 10, 2022
e8d10ee
Update packages/tracing/tests/unit/Tracer.test.ts
dreamorosi Jan 10, 2022
06d5506
Update packages/tracing/tests/unit/Tracer.test.ts
dreamorosi Jan 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 25 additions & 16 deletions packages/tracing/src/Tracer.ts
View file
Open in desktop
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
Open in desktop
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