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

dreamorosi · saragerion · web-flow · commit fd02acbbe7de · 2022-01-10T12:25:01.000+01:00
* 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 &lt;47529391+saragerion@users.noreply.github.com&gt;

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

Co-authored-by: Sara Gerion &lt;47529391+saragerion@users.noreply.github.com&gt;

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

Co-authored-by: Sara Gerion &lt;47529391+saragerion@users.noreply.github.com&gt;

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

Co-authored-by: Sara Gerion &lt;47529391+saragerion@users.noreply.github.com&gt;

Co-authored-by: Sara Gerion &lt;47529391+saragerion@users.noreply.github.com&gt;
diff --git a/packages/tracing/src/Tracer.ts b/packages/tracing/src/Tracer.ts
@@ -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(() => ({}));
+    }
   }
 
   /**
@@ -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<T>(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<T>(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<T>(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,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;
   }
 
@@ -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) {
@@ -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) {
@@ -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);
   }
 
@@ -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;
     }
 
diff --git 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 tracing is enabled, 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 tracing is enabled, 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 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
@@ -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 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
