fix(browser): Fix idle span ending

Author: mydea

packages/core/src/tracing/idleSpan.ts

@@ -113,37 +113,48 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
   const previousActiveSpan = getActiveSpan();
   const span = _startIdleSpan(startSpanOptions);
 
-  function _endSpan(timestamp: number = timestampInSeconds()): void {
-    // Ensure we end with the last span timestamp, if possible
-    const spans = getSpanDescendants(span).filter(child => child !== span);
+  // We patch span.end to ensure we can run some things before the span is ended
+  // eslint-disable-next-line @typescript-eslint/unbound-method
+  span.end = new Proxy(span.end, {
+    apply(target, thisArg, args: Parameters<Span['end']>) {
+      if (beforeSpanEnd) {
+        beforeSpanEnd(span);
+      }
 
-    // If we have no spans, we just end, nothing else to do here
-    if (!spans.length) {
-      span.end(timestamp);
-      return;
-    }
+      onIdleSpanEnded();
 
-    const childEndTimestamps = spans
-      .map(span => spanToJSON(span).timestamp)
-      .filter(timestamp => !!timestamp) as number[];
-    const latestSpanEndTimestamp = childEndTimestamps.length ? Math.max(...childEndTimestamps) : undefined;
-
-    const spanEndTimestamp = spanTimeInputToSeconds(timestamp);
-    // In reality this should always exist here, but type-wise it may be undefined...
-    const spanStartTimestamp = spanToJSON(span).start_timestamp;
-
-    // The final endTimestamp should:
-    // * Never be before the span start timestamp
-    // * Be the latestSpanEndTimestamp, if there is one, and it is smaller than the passed span end timestamp
-    // * Otherwise be the passed end timestamp
-    // Final timestamp can never be after finalTimeout
-    const endTimestamp = Math.min(
-      spanStartTimestamp ? spanStartTimestamp + finalTimeout / 1000 : Infinity,
-      Math.max(spanStartTimestamp || -Infinity, Math.min(spanEndTimestamp, latestSpanEndTimestamp || Infinity)),
-    );
-
-    span.end(endTimestamp);
-  }
+      const timestamp = args[0] || timestampInSeconds();
+
+      // Ensure we end with the last span timestamp, if possible
+      const spans = getSpanDescendants(span).filter(child => child !== span);
+
+      // If we have no spans, we just end, nothing else to do here
+      if (!spans.length) {
+        return Reflect.apply(target, thisArg, args);
+      }
+
+      const childEndTimestamps = spans
+        .map(span => spanToJSON(span).timestamp)
+        .filter(timestamp => !!timestamp) as number[];
+      const latestSpanEndTimestamp = childEndTimestamps.length ? Math.max(...childEndTimestamps) : undefined;
+
+      const spanEndTimestamp = spanTimeInputToSeconds(timestamp);
+      // In reality this should always exist here, but type-wise it may be undefined...
+      const spanStartTimestamp = spanToJSON(span).start_timestamp;
+
+      // The final endTimestamp should:
+      // * Never be before the span start timestamp
+      // * Be the latestSpanEndTimestamp, if there is one, and it is smaller than the passed span end timestamp
+      // * Otherwise be the passed end timestamp
+      // Final timestamp can never be after finalTimeout
+      const endTimestamp = Math.min(
+        spanStartTimestamp ? spanStartTimestamp + finalTimeout / 1000 : Infinity,
+        Math.max(spanStartTimestamp || -Infinity, Math.min(spanEndTimestamp, latestSpanEndTimestamp || Infinity)),
+      );
+
+      return Reflect.apply(target, thisArg, [endTimestamp]);
+    },
+  });
 
   /**
    * Cancels the existing idle timeout, if there is one.
@@ -173,7 +184,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
     _idleTimeoutID = setTimeout(() => {
       if (!_finished && activities.size === 0 && _autoFinishAllowed) {
         _finishReason = FINISH_REASON_IDLE_TIMEOUT;
-        _endSpan(endTimestamp);
+        span.end(endTimestamp);
       }
     }, idleTimeout);
   }
@@ -186,7 +197,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
     _idleTimeoutID = setTimeout(() => {
       if (!_finished && _autoFinishAllowed) {
         _finishReason = FINISH_REASON_HEARTBEAT_FAILED;
-        _endSpan(endTimestamp);
+        span.end(endTimestamp);
       }
     }, childSpanTimeout);
   }
@@ -227,10 +238,6 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
     _finished = true;
     activities.clear();
 
-    if (beforeSpanEnd) {
-      beforeSpanEnd(span);
-    }
-
     _setSpanForScope(scope, previousActiveSpan);
 
     const spanJSON = spanToJSON(span);
@@ -312,10 +319,6 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
     }
 
     _popActivity(endedSpan.spanContext().spanId);
-
-    if (endedSpan === span) {
-      onIdleSpanEnded();
-    }
   });
 
   client.on('idleSpanEnableAutoFinish', spanToAllowAutoFinish => {
@@ -338,7 +341,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
     if (!_finished) {
       span.setStatus({ code: SPAN_STATUS_ERROR, message: 'deadline_exceeded' });
       _finishReason = FINISH_REASON_FINAL_TIMEOUT;
-      _endSpan();
+      span.end();
     }
   }, finalTimeout);
 

packages/core/src/tracing/sentrySpan.ts

@@ -342,7 +342,10 @@ export class SentrySpan implements Span {
 
     if (hasMeasurements) {
       DEBUG_BUILD &&
-        logger.log('[Measurements] Adding measurements to transaction', JSON.stringify(measurements, undefined, 2));
+        logger.log(
+          '[Measurements] Adding measurements to transaction event',
+          JSON.stringify(measurements, undefined, 2),
+        );
       transaction.measurements = measurements;
     }