fix(tracing-internal): Only collect request/response spans when brows…

…er performance timing is available (#10207)

dev-packages/e2e-tests/test-applications/nextjs-app-dir/app/very-slow-component/page.tsx

@@ -0,0 +1,6 @@
+export const dynamic = 'force-dynamic';
+
+export default async function SuperSlowPage() {
+  await new Promise(resolve => setTimeout(resolve, 10000));
+  return null;
+}

dev-packages/e2e-tests/test-applications/nextjs-app-dir/tests/transactions.test.ts

@@ -139,3 +139,20 @@ test('Should send a transaction for instrumented server actions', async ({ page
 
   expect(Object.keys((await serverComponentTransactionPromise).request?.headers || {}).length).toBeGreaterThan(0);
 });
+
+test('Will not include spans in pageload transaction with faulty timestamps for slow loading pages', async ({
+  page,
+}) => {
+  const pageloadTransactionEventPromise = waitForTransaction('nextjs-13-app-dir', transactionEvent => {
+    return (
+      transactionEvent?.contexts?.trace?.op === 'pageload' && transactionEvent?.transaction === '/very-slow-component'
+    );
+  });
+
+  await page.goto('/very-slow-component');
+
+  const pageLoadTransaction = await pageloadTransactionEventPromise;
+
+  // @ts-expect-error We are looking at the serialized span format here
+  expect(pageLoadTransaction.spans?.filter(span => span.timestamp < span.start_timestamp)).toHaveLength(0);
+});

packages/tracing-internal/src/browser/metrics/index.ts

@@ -372,21 +372,27 @@ function _addPerformanceNavigationTiming(
 /** Create request and response related spans */
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 function _addRequest(transaction: Transaction, entry: Record<string, any>, timeOrigin: number): void {
-  _startChild(transaction, {
-    op: 'browser',
-    origin: 'auto.browser.browser.metrics',
-    description: 'request',
-    startTimestamp: timeOrigin + msToSec(entry.requestStart as number),
-    endTimestamp: timeOrigin + msToSec(entry.responseEnd as number),
-  });
+  if (entry.responseEnd) {
+    // It is possible that we are collecting these metrics when the page hasn't finished loading yet, for example when the HTML slowly streams in.
+    // In this case, ie. when the document request hasn't finished yet, `entry.responseEnd` will be 0.
+    // 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 transaction if it contained faulty spans.
+    _startChild(transaction, {
+      op: 'browser',
+      origin: 'auto.browser.browser.metrics',
+      description: 'request',
+      startTimestamp: timeOrigin + msToSec(entry.requestStart as number),
+      endTimestamp: timeOrigin + msToSec(entry.responseEnd as number),
+    });
 
-  _startChild(transaction, {
-    op: 'browser',
-    origin: 'auto.browser.browser.metrics',
-    description: 'response',
-    startTimestamp: timeOrigin + msToSec(entry.responseStart as number),
-    endTimestamp: timeOrigin + msToSec(entry.responseEnd as number),
-  });
+    _startChild(transaction, {
+      op: 'browser',
+      origin: 'auto.browser.browser.metrics',
+      description: 'response',
+      startTimestamp: timeOrigin + msToSec(entry.responseStart as number),
+      endTimestamp: timeOrigin + msToSec(entry.responseEnd as number),
+    });
+  }
 }
 
 export interface ResourceEntry extends Record<string, unknown> {