fix(browser): Ensure idle span duration is adjusted when child spans are ignored (#17700)

This PR fixes a problem reported in
#17451 where
ignoring spans in idle root spans (pageload and navigations most
prominently) caused the root span duration to be perceived much longer
than reasonable. This is only a problem for idle spans in browser, so
this PR applies a pragmatic fix to adjust the end time stamp:

We already adjust the time stamp when we end the idle span. So we might
as well at this point take any to-be-removed-because-of-`ignoreSpans`
spans out of this calculation. This should work well enough without
throwing a bunch of idle-span specific logic into the client or
completely changing the point in the event lifecycle where `ignoreSpans`
is applied.

closes #17451

Author: Lms24

dev-packages/browser-integration-tests/suites/tracing/ignoreSpans/init.js

@@ -0,0 +1,49 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  integrations: [
+    Sentry.browserTracingIntegration({
+      idleTimeout: 3000,
+      finalTimeout: 3000,
+      childSpanTimeout: 3000,
+    }),
+  ],
+  ignoreSpans: [/ignore/],
+  tracesSampleRate: 1,
+  debug: true,
+});
+
+const waitFor = time => new Promise(resolve => setTimeout(resolve, time));
+
+Sentry.startSpanManual(
+  {
+    name: 'take-me',
+  },
+  async span => {
+    await waitFor(500);
+    span.end();
+  },
+);
+
+Sentry.startSpanManual(
+  {
+    name: 'ignore-me',
+  },
+  async span => {
+    await waitFor(1500);
+    span.end();
+  },
+);
+
+Sentry.startSpanManual(
+  {
+    name: 'ignore-me-too',
+  },
+  async span => {
+    await waitFor(2500);
+    span.end();
+  },
+);

dev-packages/browser-integration-tests/suites/tracing/ignoreSpans/test.ts

@@ -0,0 +1,29 @@
+import { expect } from '@playwright/test';
+import { sentryTest } from '../../../utils/fixtures';
+import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../utils/helpers';
+
+sentryTest(
+  'adjusts the end timestamp of the root idle span if child spans are ignored',
+  async ({ getLocalTestUrl, page }) => {
+    if (shouldSkipTracingTest()) {
+      sentryTest.skip();
+    }
+
+    const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload');
+    const url = await getLocalTestUrl({ testDir: __dirname });
+    await page.goto(url);
+
+    const eventData = envelopeRequestParser(await pageloadRequestPromise);
+
+    const { start_timestamp: startTimestamp, timestamp: endTimestamp } = eventData;
+    const durationSeconds = endTimestamp! - startTimestamp!;
+
+    const spans = eventData.spans || [];
+
+    expect(durationSeconds).toBeGreaterThan(0);
+    expect(durationSeconds).toBeLessThan(1.5);
+
+    expect(spans.some(span => span.description === 'take-me')).toBe(true);
+    expect(spans.some(span => span.description?.includes('ignore-me'))).toBe(false);
+  },
+);

packages/core/src/client.ts

@@ -1347,6 +1347,7 @@ function processBeforeSend(
         if (droppedSpans) {
           client.recordDroppedEvent('before_send', 'span', droppedSpans);
         }
+
         processedEvent.spans = processedSpans;
       }
     }

packages/core/src/tracing/idleSpan.ts

@@ -6,6 +6,7 @@ import type { Span } from '../types-hoist/span';
 import type { StartSpanOptions } from '../types-hoist/startSpanOptions';
 import { debug } from '../utils/debug-logger';
 import { hasSpansEnabled } from '../utils/hasSpansEnabled';
+import { shouldIgnoreSpan } from '../utils/should-ignore-span';
 import { _setSpanForScope } from '../utils/spanOnScope';
 import {
   getActiveSpan,
@@ -156,10 +157,21 @@ export function startIdleSpan(startSpanOptions: StartSpanOptions, options: Parti
         return Reflect.apply(target, thisArg, [spanEndTimestamp, ...rest]);
       }
 
-      const childEndTimestamps = spans
-        .map(span => spanToJSON(span).timestamp)
-        .filter(timestamp => !!timestamp) as number[];
-      const latestSpanEndTimestamp = childEndTimestamps.length ? Math.max(...childEndTimestamps) : undefined;
+      const ignoreSpans = client.getOptions().ignoreSpans;
+
+      const latestSpanEndTimestamp = spans?.reduce((acc: number | undefined, current) => {
+        const currentSpanJson = spanToJSON(current);
+        if (!currentSpanJson.timestamp) {
+          return acc;
+        }
+        // Ignored spans will get dropped later (in the client) but since we already adjust
+        // the idle span end timestamp here, we can already take to-be-ignored spans out of
+        // the calculation here.
+        if (ignoreSpans && shouldIgnoreSpan(currentSpanJson, ignoreSpans)) {
+          return acc;
+        }
+        return acc ? Math.max(acc, currentSpanJson.timestamp) : currentSpanJson.timestamp;
+      }, undefined);
 
       // In reality this should always exist here, but type-wise it may be undefined...
       const spanStartTimestamp = spanToJSON(span).start_timestamp;