fix(tracing): Make sure performance spans change transaction start timestamp

Author: AbhiPrasad

packages/tracing/src/browser/metrics.ts

@@ -4,6 +4,7 @@ import { Span } from '../span';
 import { Transaction } from '../transaction';
 
 import { msToSec } from './utils';
+import { TransactionContext, SpanContext } from '@sentry/types';
 
 const global = getGlobalObject<Window>();
 
@@ -163,7 +164,7 @@ export class MetricsInstrumentation {
       });
 
     if (entryScriptStartTimestamp !== undefined && tracingInitMarkStartTime !== undefined) {
-      transaction.startChild({
+      _startChild(transaction, {
         description: 'evaluation',
         endTimestamp: tracingInitMarkStartTime,
         op: 'script',
@@ -196,7 +197,7 @@ function addMeasureSpans(
   const measureStartTimestamp = timeOrigin + startTime;
   const measureEndTimestamp = measureStartTimestamp + duration;
 
-  transaction.startChild({
+  _startChild(transaction, {
     description: entry.name as string,
     endTimestamp: measureEndTimestamp,
     op: entry.entryType as string,
@@ -229,7 +230,7 @@ function addResourceSpans(
     const startTimestamp = timeOrigin + startTime;
     const endTimestamp = startTimestamp + duration;
 
-    transaction.startChild({
+    _startChild(transaction, {
       description: `${entry.initiatorType} ${resourceName}`,
       endTimestamp,
       op: 'resource',
@@ -254,7 +255,7 @@ function addPerformanceNavigationTiming(
   if (!start || !end) {
     return;
   }
-  transaction.startChild({
+  _startChild(transaction, {
     description: event,
     endTimestamp: timeOrigin + msToSec(end),
     op: 'browser',
@@ -264,17 +265,33 @@ function addPerformanceNavigationTiming(
 
 /** Create request and response related spans */
 function addRequest(transaction: Transaction, entry: Record<string, any>, timeOrigin: number): void {
-  transaction.startChild({
+  _startChild(transaction, {
     description: 'request',
     endTimestamp: timeOrigin + msToSec(entry.responseEnd as number),
     op: 'browser',
     startTimestamp: timeOrigin + msToSec(entry.requestStart as number),
   });
 
-  transaction.startChild({
+  _startChild(transaction, {
     description: 'response',
     endTimestamp: timeOrigin + msToSec(entry.responseEnd as number),
     op: 'browser',
     startTimestamp: timeOrigin + msToSec(entry.responseStart as number),
   });
 }
+
+/**
+ * Helper function to start child on transactions. This function will make sure that the transaction will
+ * will use the start timestamp of the created child span if it is earlier than the transactions actual
+ * start timestamp.
+ */
+export function _startChild(transaction: Transaction, { startTimestamp, ...ctx }: SpanContext): Span {
+  if (startTimestamp && transaction.startTimestamp > startTimestamp) {
+    transaction.startTimestamp = startTimestamp;
+  }
+
+  return transaction.startChild({
+    startTimestamp,
+    ...ctx,
+  });
+}

packages/tracing/test/browser/metrics.test.ts

@@ -0,0 +1,40 @@
+import { Span, Transaction } from '../../src';
+import { _startChild } from '../../src/browser/metrics';
+
+describe('_startChild()', () => {
+  it('creates a span with given properties', () => {
+    const transaction = new Transaction({ name: 'test' });
+    const span = _startChild(transaction, {
+      description: 'evaluation',
+      op: 'script',
+    });
+
+    expect(span).toBeInstanceOf(Span);
+    expect(span.description).toBe('evaluation');
+    expect(span.op).toBe('script');
+  });
+
+  it('adjusts the start timestamp if child span starts before transaction', () => {
+    const transaction = new Transaction({ name: 'test', startTimestamp: 123 });
+    const span = _startChild(transaction, {
+      description: 'script.js',
+      op: 'resource',
+      startTimestamp: 100,
+    });
+
+    expect(transaction.startTimestamp).toEqual(span.startTimestamp);
+    expect(transaction.startTimestamp).toEqual(100);
+  });
+
+  it('does not adjust start timestamp if child span starts after transaction', () => {
+    const transaction = new Transaction({ name: 'test', startTimestamp: 123 });
+    const span = _startChild(transaction, {
+      description: 'script.js',
+      op: 'resource',
+      startTimestamp: 150,
+    });
+
+    expect(transaction.startTimestamp).not.toEqual(span.startTimestamp);
+    expect(transaction.startTimestamp).toEqual(123);
+  });
+});