From 6d528aafa12b101288b3bb1c41d55e5df681b1ae Mon Sep 17 00:00:00 2001
From: dvoytenko <dima.voytenko@vercel.com>
Date: Wed, 16 Aug 2023 11:24:09 -0700
Subject: [PATCH] Add attributes argument to recordException API

---
 api/src/trace/NonRecordingSpan.ts             |  6 ++-
 api/src/trace/span.ts                         |  6 ++-
 .../opentelemetry-sdk-trace-base/src/Span.ts  | 17 ++++++--
 .../test/common/Span.test.ts                  | 40 +++++++++++++++++++
 4 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/api/src/trace/NonRecordingSpan.ts b/api/src/trace/NonRecordingSpan.ts
index a9e5bcaf9be..dc7e3de3d78 100644
--- a/api/src/trace/NonRecordingSpan.ts
+++ b/api/src/trace/NonRecordingSpan.ts
@@ -71,5 +71,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
index d80b8c2626d..061c89cf4bb 100644
--- a/api/src/trace/span.ts
+++ b/api/src/trace/span.ts
@@ -125,5 +125,9 @@ export interface Span {
    * @param [time] the time to set as Span's event time. If not provided,
    *     use the current time.
    */
-  recordException(exception: Exception, time?: TimeInput): void;
+  recordException(
+    exception: Exception,
+    attributesOrStartTime?: SpanAttributes | TimeInput,
+    time?: TimeInput
+  ): void;
 }
diff --git a/packages/opentelemetry-sdk-trace-base/src/Span.ts b/packages/opentelemetry-sdk-trace-base/src/Span.ts
index 31fb1555ac7..2d267bbb090 100644
--- a/packages/opentelemetry-sdk-trace-base/src/Span.ts
+++ b/packages/opentelemetry-sdk-trace-base/src/Span.ts
@@ -272,8 +272,19 @@ export class Span implements APISpan, ReadableSpan {
     return this._ended === false;
   }
 
-  recordException(exception: Exception, time?: TimeInput): void {
-    const attributes: SpanAttributes = {};
+  recordException(
+    exception: Exception,
+    attributesOrStartTime?: SpanAttributes | TimeInput,
+    timeStamp?: TimeInput
+  ): void {
+    if (isTimeInput(attributesOrStartTime)) {
+      if (!isTimeInput(timeStamp)) {
+        timeStamp = attributesOrStartTime;
+      }
+      attributesOrStartTime = undefined;
+    }
+
+    const attributes = sanitizeAttributes(attributesOrStartTime);
     if (typeof exception === 'string') {
       attributes[SemanticAttributes.EXCEPTION_MESSAGE] = exception;
     } else if (exception) {
@@ -296,7 +307,7 @@ export class Span implements APISpan, ReadableSpan {
       attributes[SemanticAttributes.EXCEPTION_TYPE] ||
       attributes[SemanticAttributes.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
index 11a94ffc7c2..0567af8ba5f 100644
--- a/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
+++ b/packages/opentelemetry-sdk-trace-base/test/common/Span.test.ts
@@ -1203,6 +1203,46 @@ 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 Span(
+          tracer,
+          ROOT_CONTEXT,
+          name,
+          spanContext,
+          SpanKind.CLIENT
+        );
+        // @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 Span(
+          tracer,
+          ROOT_CONTEXT,
+          name,
+          spanContext,
+          SpanKind.CLIENT
+        );
+        // @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', {
+          ...validAttributes,
+          ...invalidAttributes,
+        } as unknown as SpanAttributes);
+        const event = span.events[0];
+        assert.deepStrictEqual(event.attributes, {
+          [SemanticAttributes.EXCEPTION_MESSAGE]: 'boom',
+          ...validAttributes,
+        });
+      });
     });
 
     describe('when exception code is numeric', () => {