feat(api): add attributes argument to recordException API

Originally introduced in #4071 but reverted in #4137 Co-authored-by: Marc Pichler <marc.pichler@dynatrace.com>
open-telemetry · Jan 11, 2025 · bab99f3 · bab99f3
1 parent 6864c2f
commit bab99f3
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -344,6 +344,7 @@ For semantic convention package changes, see the [semconv CHANGELOG](packages/se
 
 ### :rocket: (Enhancement)
 
+* feat(api): add attributes argument to recordException API [#4071](https://github.com/open-telemetry/opentelemetry-js/pull/4071)
 * feat(sdk-metrics): implement MetricProducer specification [#4007](https://github.com/open-telemetry/opentelemetry-js/pull/4007)
 * feat: update PeriodicExportingMetricReader and PrometheusExporter to accept optional metric producers [#4077](https://github.com/open-telemetry/opentelemetry-js/pull/4077) @aabmass
 

diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md
@@ -9,6 +9,8 @@ All notable changes to this project will be documented in this file.
 
 ### :rocket: (Enhancement)
 
+* feat(api): add attributes argument to recordException API (PR TBD)
+
 ### :bug: (Bug Fix)
 
 ### :books: (Refine Doc)

diff --git a/api/src/trace/NonRecordingSpan.ts b/api/src/trace/NonRecordingSpan.ts
@@ -80,5 +80,9 @@ export class NonRecordingSpan implements Span {
   }
 
   // By default does nothing
-  recordException(_exception: Exception, _time?: TimeInput): void {}
+  recordException(
+    _exception: Exception,
+    _attributesOrStartTime?: SpanAttributes | TimeInput,
+    _time?: TimeInput
+  ): void {}
 }
diff --git a/api/src/trace/span.ts b/api/src/trace/span.ts
@@ -149,4 +149,17 @@ export interface Span {
    *     use the current time.
    */
   recordException(exception: Exception, time?: TimeInput): void;
+
+  /**
+   * Sets exception as a span event
+   * @param exception the exception the only accepted values are string or Error
+   * @param [attributes] the attributes that will be added to the error event.
+   * @param [time] the time to set as Span's event time. If not provided,
+   *     use the current time.
+   */
+  recordException(
+    exception: Exception,
+    attributes?: SpanAttributes,
+    time?: TimeInput
+  ): void;
 }
diff --git a/packages/opentelemetry-sdk-trace-base/src/Span.ts b/packages/opentelemetry-sdk-trace-base/src/Span.ts
@@ -318,7 +318,18 @@ export class SpanImpl implements Span {
     return this._ended === false;
   }
 
-  recordException(exception: Exception, time?: TimeInput): void {
+  recordException(
+    exception: Exception,
+    attributesOrStartTime?: Attributes | TimeInput,
+    timeStamp?: TimeInput
+  ): void {
+    if (isTimeInput(attributesOrStartTime)) {
+      if (!isTimeInput(timeStamp)) {
+        timeStamp = attributesOrStartTime;
+      }
+      attributesOrStartTime = undefined;
+    }
+
     const attributes: Attributes = {};
     if (typeof exception === 'string') {
       attributes[SEMATTRS_EXCEPTION_MESSAGE] = exception;
@@ -335,13 +346,16 @@ export class SpanImpl implements Span {
         attributes[SEMATTRS_EXCEPTION_STACKTRACE] = exception.stack;
       }
     }
+    if (attributesOrStartTime) {
+      Object.assign(attributes, sanitizeAttributes(attributesOrStartTime));
+    }
 
     // these are minimum requirements from spec
     if (
       attributes[SEMATTRS_EXCEPTION_TYPE] ||
       attributes[SEMATTRS_EXCEPTION_MESSAGE]
     ) {
-      this.addEvent(ExceptionEventName, attributes, time);
+      this.addEvent(ExceptionEventName, attributes, timeStamp);
     } else {
       diag.warn(`Failed to record an exception ${exception}`);
     }

diff --git a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
@@ -1438,6 +1438,86 @@ describe('Span', () => {
         const event = span.events[0];
         assert.deepStrictEqual(event.time, [0, 123]);
       });
+
+      it('should record an exception with provided time as a 3rd arg', () => {
+        const span = new SpanImpl({
+          scope: tracer.instrumentationScope,
+          resource: tracer['_resource'],
+          context: ROOT_CONTEXT,
+          name,
+          spanContext,
+          kind: SpanKind.CLIENT,
+          spanLimits: tracer.getSpanLimits(),
+          spanProcessor: tracer['_spanProcessor'],
+        });
+        // @ts-expect-error writing readonly property. performance time origin is mocked to return ms value of [1,1]
+        span['_performanceOffset'] = 0;
+        assert.strictEqual(span.events.length, 0);
+        span.recordException('boom', undefined, [0, 123]);
+        const event = span.events[0];
+        assert.deepStrictEqual(event.time, [0, 123]);
+      });
+    });
+
+    describe('when attributes are provided', () => {
+      it('should sanitized and merge attributes when provided', () => {
+        const span = new SpanImpl({
+          scope: tracer.instrumentationScope,
+          resource: tracer['_resource'],
+          context: ROOT_CONTEXT,
+          name,
+          spanContext,
+          kind: SpanKind.CLIENT,
+          spanLimits: tracer.getSpanLimits(),
+          spanProcessor: tracer['_spanProcessor'],
+        });
+        // @ts-expect-error writing readonly property. performance time origin is mocked to return ms value of [1,1]
+        span['_performanceOffset'] = 0;
+        assert.strictEqual(span.events.length, 0);
+        const exception = { code: 'Error', message: 'boom', stack: 'bar' };
+        span.recordException(exception, {
+          ...validAttributes,
+          ...invalidAttributes,
+        } as unknown as Attributes);
+        const event = span.events[0];
+        assert.deepStrictEqual(event.attributes, {
+          [SEMATTRS_EXCEPTION_TYPE]: 'Error',
+          [SEMATTRS_EXCEPTION_MESSAGE]: 'boom',
+          [SEMATTRS_EXCEPTION_STACKTRACE]: 'bar',
+          ...validAttributes,
+        });
+      });
+
+      it('should prioritize the provided attributes over generated', () => {
+        const span = new SpanImpl({
+          scope: tracer.instrumentationScope,
+          resource: tracer['_resource'],
+          context: ROOT_CONTEXT,
+          name,
+          spanContext,
+          kind: SpanKind.CLIENT,
+          spanLimits: tracer.getSpanLimits(),
+          spanProcessor: tracer['_spanProcessor'],
+        });
+        // @ts-expect-error writing readonly property. performance time origin is mocked to return ms value of [1,1]
+        span['_performanceOffset'] = 0;
+        assert.strictEqual(span.events.length, 0);
+        const exception = { code: 'Error', message: 'boom', stack: 'bar' };
+        span.recordException(exception, {
+          [SEMATTRS_EXCEPTION_TYPE]: 'OverrideError',
+          [SEMATTRS_EXCEPTION_MESSAGE]: 'override-boom',
+          [SEMATTRS_EXCEPTION_STACKTRACE]: 'override-bar',
+          ...validAttributes,
+          ...invalidAttributes,
+        } as unknown as Attributes);
+        const event = span.events[0];
+        assert.deepStrictEqual(event.attributes, {
+          ...validAttributes,
+          [SEMATTRS_EXCEPTION_TYPE]: 'OverrideError',
+          [SEMATTRS_EXCEPTION_MESSAGE]: 'override-boom',
+          [SEMATTRS_EXCEPTION_STACKTRACE]: 'override-bar',
+        });
+      });
     });
 
     describe('when exception code is numeric', () => {