fix(browser): Ensure spans auto-ended for navigations have cancelled reason (#16277)

Noticed that this now has `externalFinish` as reason, which is not
really helpful. With this, you should be able to see that the
pageload/navigation was ended because a new one started.

Author: mydea

.size-limit.js

@@ -135,7 +135,7 @@ module.exports = [
     path: 'packages/vue/build/esm/index.js',
     import: createImport('init', 'browserTracingIntegration'),
     gzip: true,
-    limit: '40 KB',
+    limit: '41 KB',
   },
   // Svelte SDK (ESM)
   {

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/init.js

@@ -0,0 +1,12 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  integrations: [Sentry.browserTracingIntegration()],
+  tracesSampleRate: 1,
+});
+
+// Immediately navigate to a new page to abort the pageload
+window.location.href = '#foo';

dev-packages/browser-integration-tests/suites/tracing/browserTracingIntegration/navigation-aborting-pageload/test.ts

@@ -0,0 +1,58 @@
+import { expect } from '@playwright/test';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+} from '@sentry/core';
+import { sentryTest } from '../../../../utils/fixtures';
+import { envelopeRequestParser, shouldSkipTracingTest, waitForTransactionRequest } from '../../../../utils/helpers';
+
+sentryTest(
+  'should create a navigation transaction that aborts an ongoing pageload',
+  async ({ getLocalTestUrl, page }) => {
+    if (shouldSkipTracingTest()) {
+      sentryTest.skip();
+    }
+
+    const url = await getLocalTestUrl({ testDir: __dirname });
+
+    const pageloadRequestPromise = waitForTransactionRequest(page, event => event.contexts?.trace?.op === 'pageload');
+    const navigationRequestPromise = waitForTransactionRequest(
+      page,
+      event => event.contexts?.trace?.op === 'navigation',
+    );
+
+    await page.goto(url);
+
+    const pageloadRequest = envelopeRequestParser(await pageloadRequestPromise);
+    const navigationRequest = envelopeRequestParser(await navigationRequestPromise);
+
+    expect(pageloadRequest.contexts?.trace?.op).toBe('pageload');
+    expect(navigationRequest.contexts?.trace?.op).toBe('navigation');
+
+    expect(navigationRequest.transaction_info?.source).toEqual('url');
+
+    const pageloadTraceId = pageloadRequest.contexts?.trace?.trace_id;
+    const navigationTraceId = navigationRequest.contexts?.trace?.trace_id;
+
+    expect(pageloadTraceId).toBeDefined();
+    expect(navigationTraceId).toBeDefined();
+    expect(pageloadTraceId).not.toEqual(navigationTraceId);
+
+    expect(pageloadRequest.contexts?.trace?.data).toMatchObject({
+      [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
+      [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
+      [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
+      [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
+      ['sentry.idle_span_finish_reason']: 'cancelled',
+    });
+    expect(navigationRequest.contexts?.trace?.data).toMatchObject({
+      [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
+      [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
+      [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
+      [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
+      ['sentry.idle_span_finish_reason']: 'idleTimeout',
+    });
+  },
+);

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

@@ -1,5 +1,11 @@
 import { expect } from '@playwright/test';
 import type { Event } from '@sentry/core';
+import {
+  SEMANTIC_ATTRIBUTE_SENTRY_OP,
+  SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
+  SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE,
+  SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
+} from '@sentry/core';
 import { sentryTest } from '../../../../utils/fixtures';
 import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
 
@@ -25,6 +31,21 @@ sentryTest('should create a navigation transaction on page navigation', async ({
   expect(navigationTraceId).toBeDefined();
   expect(pageloadTraceId).not.toEqual(navigationTraceId);
 
+  expect(pageloadRequest.contexts?.trace?.data).toMatchObject({
+    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.pageload.browser',
+    [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
+    [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
+    [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
+    ['sentry.idle_span_finish_reason']: 'idleTimeout',
+  });
+  expect(navigationRequest.contexts?.trace?.data).toMatchObject({
+    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
+    [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
+    [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
+    [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'navigation',
+    ['sentry.idle_span_finish_reason']: 'idleTimeout',
+  });
+
   const pageloadSpans = pageloadRequest.spans;
   const navigationSpans = navigationRequest.spans;
 

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

@@ -30,6 +30,7 @@ sentryTest('creates a pageload transaction with url as source', async ({ getLoca
     [SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
     [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
     [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'pageload',
+    ['sentry.idle_span_finish_reason']: 'idleTimeout',
   });
 
   expect(eventData.contexts?.trace?.op).toBe('pageload');

packages/browser/src/tracing/browserTracingIntegration.ts

@@ -386,6 +386,7 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
         if (activeSpan && !spanToJSON(activeSpan).timestamp) {
           DEBUG_BUILD && logger.log(`[Tracing] Finishing current active span with op: ${spanToJSON(activeSpan).op}`);
           // If there's an open active span, we need to finish it before creating an new one.
+          activeSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_IDLE_SPAN_FINISH_REASON, 'cancelled');
           activeSpan.end();
         }
       }