ref(browser): Move navigation span descriptions into op (#13527)

Makes some modifications to `browser` spans: - Moves the description of navigation related `browser` spans into the op, e.g. `browser - cache` -> `browser.cache` - Sets the description to the `performanceEntry` objects' `names`, in this context it is the URL of the page This change is being made so that these `browser` spans can be ingested and grouped. Currently, all browser spans are grouped into a singular `browser` span, despite each of the operations that these span represent doing very different things. This will improve the experience in the Spans tab of transaction summary and span summary, since we will be able to have proper groupings for `browser` spans.
getsentry · Sep 23, 2024 · 021c8c1 · 021c8c1
1 parent 1f898b6
commit 021c8c1
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,8 @@
 ## Unreleased
 
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
+- Moved description of `browser` spans into the operation, e.g. `browser - cache` -> `browser.cache` and set the URL as
+  the description
 
 ## 8.31.0
 

diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/pageload-browser-spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/pageload-browser-spans/test.ts
@@ -12,7 +12,7 @@ sentryTest('should add browser-related spans to pageload transaction', async ({
   const url = await getLocalTestPath({ testDir: __dirname });
 
   const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
-  const browserSpans = eventData.spans?.filter(({ op }) => op === 'browser');
+  const browserSpans = eventData.spans?.filter(({ op }) => op?.startsWith('browser'));
 
   // Spans `domContentLoadedEvent`, `connect`, `cache` and `DNS` are not
   // always inside `pageload` transaction.
@@ -21,7 +21,8 @@ sentryTest('should add browser-related spans to pageload transaction', async ({
   ['loadEvent', 'request', 'response'].forEach(eventDesc =>
     expect(browserSpans).toContainEqual(
       expect.objectContaining({
-        description: eventDesc,
+        op: `browser.${eventDesc}`,
+        description: page.url(),
         parent_span_id: eventData.contexts?.trace?.span_id,
       }),
     ),

diff --git a/dev-packages/browser-integration-tests/suites/tracing/metrics/pageload-measure-spans/test.ts b/dev-packages/browser-integration-tests/suites/tracing/metrics/pageload-measure-spans/test.ts
@@ -13,14 +13,15 @@ sentryTest('should add browser-related spans to pageload transaction', async ({
   const url = await getLocalTestPath({ testDir: __dirname });
 
   const eventData = await getFirstSentryEnvelopeRequest<Event>(page, url);
-  const browserSpans = eventData.spans?.filter(({ op }) => op === 'browser');
+  const browserSpans = eventData.spans?.filter(({ op }) => op?.startsWith('browser'));
 
   // Spans `domContentLoadedEvent`, `connect`, `cache` and `DNS` are not
   // always inside `pageload` transaction.
   expect(browserSpans?.length).toBeGreaterThanOrEqual(4);
 
-  const requestSpan = browserSpans!.find(({ description }) => description === 'request');
+  const requestSpan = browserSpans!.find(({ op }) => op === 'browser.request');
   expect(requestSpan).toBeDefined();
+  expect(requestSpan?.description).toBe(page.url());
 
   const measureSpan = eventData.spans?.find(({ op }) => op === 'measure');
   expect(measureSpan).toBeDefined();

diff --git a/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-create-hash-router/tests/transactions.test.ts
@@ -44,10 +44,10 @@ test('Captures a pageload transaction', async ({ page }) => {
   expect(transactionEvent.spans).toContainEqual({
     data: {
       'sentry.origin': 'auto.ui.browser.metrics',
-      'sentry.op': 'browser',
+      'sentry.op': 'browser.domContentLoadedEvent',
     },
-    description: 'domContentLoadedEvent',
-    op: 'browser',
+    description: page.url(),
+    op: 'browser.domContentLoadedEvent',
     parent_span_id: expect.any(String),
     span_id: expect.any(String),
     start_timestamp: expect.any(Number),
@@ -58,10 +58,10 @@ test('Captures a pageload transaction', async ({ page }) => {
   expect(transactionEvent.spans).toContainEqual({
     data: {
       'sentry.origin': 'auto.ui.browser.metrics',
-      'sentry.op': 'browser',
+      'sentry.op': 'browser.connect',
     },
-    description: 'connect',
-    op: 'browser',
+    description: page.url(),
+    op: 'browser.connect',
     parent_span_id: expect.any(String),
     span_id: expect.any(String),
     start_timestamp: expect.any(Number),
@@ -72,10 +72,10 @@ test('Captures a pageload transaction', async ({ page }) => {
   expect(transactionEvent.spans).toContainEqual({
     data: {
       'sentry.origin': 'auto.ui.browser.metrics',
-      'sentry.op': 'browser',
+      'sentry.op': 'browser.request',
     },
-    description: 'request',
-    op: 'browser',
+    description: page.url(),
+    op: 'browser.request',
     parent_span_id: expect.any(String),
     span_id: expect.any(String),
     start_timestamp: expect.any(Number),
@@ -86,10 +86,10 @@ test('Captures a pageload transaction', async ({ page }) => {
   expect(transactionEvent.spans).toContainEqual({
     data: {
       'sentry.origin': 'auto.ui.browser.metrics',
-      'sentry.op': 'browser',
+      'sentry.op': 'browser.response',
     },
-    description: 'response',
-    op: 'browser',
+    description: page.url(),
+    op: 'browser.response',
     parent_span_id: expect.any(String),
     span_id: expect.any(String),
     start_timestamp: expect.any(Number),

diff --git a/packages/browser-utils/src/metrics/browserMetrics.ts b/packages/browser-utils/src/metrics/browserMetrics.ts
@@ -470,8 +470,8 @@ function _addPerformanceNavigationTiming(
     return;
   }
   startAndEndSpan(span, timeOrigin + msToSec(start), timeOrigin + msToSec(end), {
-    op: 'browser',
-    name: name || event,
+    op: `browser.${name || event}`,
+    name: entry.name,
     attributes: {
       [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
     },
@@ -490,16 +490,16 @@ function _addRequest(span: Span, entry: Record<string, any>, timeOrigin: number)
     // In order not to produce faulty spans, where the end timestamp is before the start timestamp, we will only collect
     // these spans when the responseEnd value is available. The backend (Relay) would drop the entire span if it contained faulty spans.
     startAndEndSpan(span, requestStartTimestamp, responseEndTimestamp, {
-      op: 'browser',
-      name: 'request',
+      op: 'browser.request',
+      name: entry.name,
       attributes: {
         [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
       },
     });
 
     startAndEndSpan(span, responseStartTimestamp, responseEndTimestamp, {
-      op: 'browser',
-      name: 'response',
+      op: 'browser.response',
+      name: entry.name,
       attributes: {
         [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
       },