feat: allow setting attributes when recording exceptions

open-telemetry · Jun 22, 2021 · 5665c42 · 5665c42
1 parent 7860344
commit 5665c42
Show file tree

Hide file tree

Showing 2 changed files with 97 additions and 14 deletions.
diff --git a/packages/opentelemetry-tracing/src/Span.ts b/packages/opentelemetry-tracing/src/Span.ts
@@ -133,19 +133,11 @@ export class Span implements api.Span, ReadableSpan {
       api.diag.warn('Dropping extra events.');
       this.events.shift();
     }
-    if (isTimeInput(attributesOrStartTime)) {
-      if (typeof startTime === 'undefined') {
-        startTime = attributesOrStartTime as api.TimeInput;
-      }
-      attributesOrStartTime = undefined;
-    }
-    if (typeof startTime === 'undefined') {
-      startTime = hrTime();
-    }
+    const [resolvedAttributes, resolvedTime] = Span._resolveEventArgs(attributesOrStartTime, startTime);
     this.events.push({
       name,
-      attributes: attributesOrStartTime as api.SpanAttributes,
-      time: timeInputToHrTime(startTime),
+      attributes: resolvedAttributes,
+      time: timeInputToHrTime(resolvedTime),
     });
     return this;
   }
@@ -186,8 +178,13 @@ export class Span implements api.Span, ReadableSpan {
     return this._ended === false;
   }
 
-  recordException(exception: api.Exception, time: api.TimeInput = hrTime()) {
-    const attributes: api.SpanAttributes = {};
+  recordException(
+    exception: api.Exception,
+    attributesOrTime?: api.SpanAttributes | api.TimeInput,
+    time?: api.TimeInput
+  ): void {
+    const [resolvedAttributes, resolvedTime] = Span._resolveEventArgs(attributesOrTime, time);
+    let attributes: api.SpanAttributes = {};
     if (typeof exception === 'string') {
       attributes[SemanticAttributes.EXCEPTION_MESSAGE] = exception;
     } else if (exception) {
@@ -205,13 +202,14 @@ export class Span implements api.Span, ReadableSpan {
         attributes[SemanticAttributes.EXCEPTION_STACKTRACE] = exception.stack;
       }
     }
+    attributes = Object.assign(attributes, resolvedAttributes);
 
     // these are minimum requirements from spec
     if (
       attributes[SemanticAttributes.EXCEPTION_TYPE] ||
       attributes[SemanticAttributes.EXCEPTION_MESSAGE]
     ) {
-      this.addEvent(ExceptionEventName, attributes as api.SpanAttributes, time);
+      this.addEvent(ExceptionEventName, attributes as api.SpanAttributes, resolvedTime);
     } else {
       api.diag.warn(`Failed to record an exception ${exception}`);
     }
@@ -235,4 +233,24 @@ export class Span implements api.Span, ReadableSpan {
     }
     return this._ended;
   }
+
+  private static _resolveEventArgs(
+    attributesOrTimeArg?: api.SpanAttributes | api.TimeInput,
+    timeArg?: api.TimeInput
+  ): [api.SpanAttributes | undefined, api.TimeInput] {
+    let attributes: api.SpanAttributes | undefined;
+    let time = timeArg;
+    if (isTimeInput(attributesOrTimeArg)) {
+      if (typeof time === 'undefined') {
+        time = attributesOrTimeArg as api.TimeInput;
+      }
+    } else {
+      attributes = attributesOrTimeArg as api.SpanAttributes;
+    }
+    if (typeof time === 'undefined') {
+      time = hrTime();
+    }
+
+    return [attributes, time];
+  }
 }
diff --git a/packages/opentelemetry-tracing/test/common/Span.test.ts b/packages/opentelemetry-tracing/test/common/Span.test.ts
@@ -719,6 +719,71 @@ describe('Span', () => {
         span.recordException('boom', [0, 123]);
         const event = span.events[0];
         assert.deepStrictEqual(event.time, [0, 123]);
+        assert.deepStrictEqual(event.attributes, { [SemanticAttributes.EXCEPTION_MESSAGE]: 'boom' });
+      });
+    });
+
+    describe('when attributes are provided', () => {
+      it('should record an exception with those attributes', () => {
+        const span = new Span(
+          tracer,
+          ROOT_CONTEXT,
+          name,
+          spanContext,
+          SpanKind.CLIENT
+        );
+        assert.strictEqual(span.events.length, 0);
+        span.recordException('boom', { meow: 123, woof: "hello" });
+        const event = span.events[0];
+        assert.deepStrictEqual(event.attributes, {
+          [SemanticAttributes.EXCEPTION_MESSAGE]: 'boom',
+          meow: 123,
+          woof: "hello"
+        });
+      });
+
+      it('provided attributes should take precedence over generated ones', () => {
+        const span = new Span(
+          tracer,
+          ROOT_CONTEXT,
+          name,
+          spanContext,
+          SpanKind.CLIENT
+        );
+        assert.strictEqual(span.events.length, 0);
+        span.recordException({ name: 'Error', message: 'boom', stack: 'bar' },
+          {
+            [SemanticAttributes.EXCEPTION_TYPE]: 'My Special Error',
+            [SemanticAttributes.EXCEPTION_MESSAGE]: 'My Special Message',
+            [SemanticAttributes.EXCEPTION_STACKTRACE]: 'My Special Stack',
+          });
+        const event = span.events[0];
+        assert.deepStrictEqual(event.attributes, {
+          [SemanticAttributes.EXCEPTION_TYPE]: 'My Special Error',
+          [SemanticAttributes.EXCEPTION_MESSAGE]: 'My Special Message',
+          [SemanticAttributes.EXCEPTION_STACKTRACE]: 'My Special Stack',
+        });
+      });
+    });
+
+    describe('when both attributes and time are provided', () => {
+      it('should record an exception with provided attributes and time', () => {
+        const span = new Span(
+          tracer,
+          ROOT_CONTEXT,
+          name,
+          spanContext,
+          SpanKind.CLIENT
+        );
+        assert.strictEqual(span.events.length, 0);
+        span.recordException('boom', { meow: 123, woof: 'hello' }, [0, 123]);
+        const event = span.events[0];
+        assert.deepStrictEqual(event.time, [0, 123]);
+        assert.deepStrictEqual(event.attributes, {
+          [SemanticAttributes.EXCEPTION_MESSAGE]: 'boom',
+          meow: 123,
+          woof: "hello"
+        })
       });
     });