From 66558e915ec9315d602c9f7cf0f83e51623854e7 Mon Sep 17 00:00:00 2001
From: Brian Phillips <28457+brianphillips@users.noreply.github.com>
Date: Wed, 22 Jan 2025 19:53:51 -0600
Subject: [PATCH] fix(api): clean up method signature and logic flow

This clarifies the `recordException` method signature, cleans up the
logic sequence and avoids an extra invocation of
`sanitizeAttributes(...)`.

 See [this
comment](https://github.com/open-telemetry/opentelemetry-js/pull/5333/files#r1926210496)
for context

Co-authored-by: Godfrey Chan <godfreykfc@gmail.com>
---
 api/src/trace/NonRecordingSpan.ts             | 13 +++++++---
 api/src/trace/span.ts                         |  6 ++++-
 .../opentelemetry-sdk-trace-base/src/Span.ts  | 26 ++++++++++---------
 3 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/api/src/trace/NonRecordingSpan.ts b/api/src/trace/NonRecordingSpan.ts
index 4d0467daa5..572d3bfbb4 100644
--- a/api/src/trace/NonRecordingSpan.ts
+++ b/api/src/trace/NonRecordingSpan.ts
@@ -81,8 +81,13 @@ export class NonRecordingSpan implements Span {
 
   // By default does nothing
   recordException(
-    _exception: Exception,
-    _attributesOrStartTime?: SpanAttributes | TimeInput,
-    _time?: TimeInput
-  ): void {}
+    exception: Exception,
+    time?: TimeInput
+  ): void;
+  recordException(
+    exception: Exception,
+    attributes: SpanAttributes | undefined,
+    time?: TimeInput
+  ): void;
+  recordException(): void {}
 }
diff --git a/api/src/trace/span.ts b/api/src/trace/span.ts
index 2abf8e3cef..a749c7929a 100644
--- a/api/src/trace/span.ts
+++ b/api/src/trace/span.ts
@@ -159,7 +159,11 @@ export interface Span {
    */
   recordException(
     exception: Exception,
-    attributes?: SpanAttributes,
+    time?: TimeInput
+  ): void;
+  recordException(
+    exception: Exception,
+    attributes: SpanAttributes | undefined,
     time?: TimeInput
   ): void;
 }
diff --git a/packages/opentelemetry-sdk-trace-base/src/Span.ts b/packages/opentelemetry-sdk-trace-base/src/Span.ts
index 8d9205fd42..1db4d1f9a4 100644
--- a/packages/opentelemetry-sdk-trace-base/src/Span.ts
+++ b/packages/opentelemetry-sdk-trace-base/src/Span.ts
@@ -318,18 +318,17 @@ export class SpanImpl implements Span {
     return this._ended === false;
   }
 
+  recordException(exception: Exception, time?: TimeInput): void;
   recordException(
     exception: Exception,
-    attributesOrStartTime?: Attributes | TimeInput,
-    timeStamp?: TimeInput
+    attributes: Attributes | undefined,
+    time?: TimeInput
+  ): void;
+  recordException(
+    exception: Exception,
+    attributesOrTime?: Attributes | TimeInput,
+    time?: TimeInput
   ): void {
-    if (isTimeInput(attributesOrStartTime)) {
-      if (!isTimeInput(timeStamp)) {
-        timeStamp = attributesOrStartTime;
-      }
-      attributesOrStartTime = undefined;
-    }
-
     const attributes: Attributes = {};
     if (typeof exception === 'string') {
       attributes[SEMATTRS_EXCEPTION_MESSAGE] = exception;
@@ -346,8 +345,11 @@ export class SpanImpl implements Span {
         attributes[SEMATTRS_EXCEPTION_STACKTRACE] = exception.stack;
       }
     }
-    if (attributesOrStartTime) {
-      Object.assign(attributes, sanitizeAttributes(attributesOrStartTime));
+
+    if (isTimeInput(attributesOrTime)) {
+      time = attributesOrTime;
+    } else {
+      Object.assign(attributes, sanitizeAttributes(attributesOrTime));
     }
 
     // these are minimum requirements from spec
@@ -355,7 +357,7 @@ export class SpanImpl implements Span {
       attributes[SEMATTRS_EXCEPTION_TYPE] ||
       attributes[SEMATTRS_EXCEPTION_MESSAGE]
     ) {
-      this.addEvent(ExceptionEventName, attributes, timeStamp);
+      this.addEvent(ExceptionEventName, attributes, time);
     } else {
       diag.warn(`Failed to record an exception ${exception}`);
     }