ref(browser): Ensure idle span ending is consistent (#12310)

getsentry · Jun 4, 2024 · 5494cb5 · 5494cb5
1 parent 119a923
commit 5494cb5
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 10 deletions.
diff --git a/...ation-tests/suites/tracing/browserTracingIntegration/pageloadWithChildSpanTimeout/init.js b/...ation-tests/suites/tracing/browserTracingIntegration/pageloadWithChildSpanTimeout/init.js
@@ -15,10 +15,9 @@ Sentry.init({
 });
 
 const activeSpan = Sentry.getActiveSpan();
-if (activeSpan) {
-  Sentry.startInactiveSpan({ name: 'pageload-child-span' });
-} else {
-  setTimeout(() => {
-    Sentry.startInactiveSpan({ name: 'pageload-child-span' });
-  }, 200);
-}
+Sentry.startInactiveSpan({
+  name: 'pageload-child-span',
+  onlyIfParent: true,
+  // Set this to ensure we do not discard this span due to timeout
+  startTime: activeSpan && Sentry.spanToJSON(activeSpan).start_timestamp,
+});
diff --git a/...ation-tests/suites/tracing/browserTracingIntegration/pageloadWithChildSpanTimeout/test.ts b/...ation-tests/suites/tracing/browserTracingIntegration/pageloadWithChildSpanTimeout/test.ts
@@ -21,6 +21,7 @@ sentryTest('should send a pageload span terminated via child span timeout', asyn
   const eventData = envelopeRequestParser(req);
 
   expect(eventData.contexts?.trace?.op).toBe('pageload');
+  expect(eventData.contexts?.trace?.data?.['sentry.idle_span_discarded_spans']).toBeUndefined();
   expect(eventData.spans?.length).toBeGreaterThanOrEqual(1);
   const testSpan = eventData.spans?.find(span => span.description === 'pageload-child-span');
   expect(testSpan).toBeDefined();

diff --git a/packages/core/src/tracing/idleSpan.ts b/packages/core/src/tracing/idleSpan.ts
@@ -121,7 +121,9 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
         beforeSpanEnd(span);
       }
 
-      const timestamp = args[0] || timestampInSeconds();
+      // Just ensuring that this keeps working, even if we ever have more arguments here
+      const [definedEndTimestamp, ...rest] = args;
+      const timestamp = definedEndTimestamp || timestampInSeconds();
       const spanEndTimestamp = spanTimeInputToSeconds(timestamp);
 
       // Ensure we end with the last span timestamp, if possible
@@ -130,7 +132,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
       // If we have no spans, we just end, nothing else to do here
       if (!spans.length) {
         onIdleSpanEnded(spanEndTimestamp);
-        return Reflect.apply(target, thisArg, args);
+        return Reflect.apply(target, thisArg, [spanEndTimestamp, ...rest]);
       }
 
       const childEndTimestamps = spans
@@ -152,7 +154,7 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
       );
 
       onIdleSpanEnded(endTimestamp);
-      return Reflect.apply(target, thisArg, [endTimestamp]);
+      return Reflect.apply(target, thisArg, [endTimestamp, ...rest]);
     },
   });