make startNewTrace use a callback

Lms24 · Lms24 · commit 91316eacda03 · 2024-05-24T16:26:31.000+02:00
diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/startNewTrace/subject.js b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/startNewTrace/subject.js
@@ -1,7 +1,15 @@
-const fetchBtn = document.getElementById('fetchBtn');
-fetchBtn.addEventListener('click', async () => {
-  Sentry.startNewTrace();
-  Sentry.startSpan({ op: 'ui.interaction.click', name: 'fetch click' }, async () => {
+const newTraceBtn = document.getElementById('newTrace');
+newTraceBtn.addEventListener('click', async () => {
+  Sentry.startNewTrace(() => {
+    Sentry.startSpan({ op: 'ui.interaction.click', name: 'new-trace' }, async () => {
+      await fetch('http://example.com');
+    });
+  });
+});
+
+const oldTraceBtn = document.getElementById('oldTrace');
+oldTraceBtn.addEventListener('click', async () => {
+  Sentry.startSpan({ op: 'ui.interaction.click', name: 'old-trace' }, async () => {
     await fetch('http://example.com');
   });
 });
diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/startNewTrace/template.html b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/startNewTrace/template.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+  </head>
+  <body>
+    <button id="oldTrace">Old Trace</button>
+    <button id="newTrace">new Trace</button>
+  </body>
+</html>
diff --git a/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/startNewTrace/test.ts b/dev-packages/browser-integration-tests/suites/tracing/trace-lifetime/startNewTrace/test.ts
@@ -47,34 +47,64 @@ sentryTest('should create a new trace if `startNewTrace` is called', async ({ ge
     trace_id: pageloadTraceContext?.trace_id,
   });
 
-  const customTransactionPromise = getFirstSentryEnvelopeRequest<EventAndTraceHeader>(
+  const newTraceTransactionPromise = getFirstSentryEnvelopeRequest<EventAndTraceHeader>(
     page,
     undefined,
     eventAndTraceHeaderRequestParser,
   );
 
-  await page.locator('#fetchBtn').click();
+  await page.locator('#newTrace').click();
 
-  const [customTransactionEvent, customTransactionTraceHeaders] = await customTransactionPromise;
+  const [newTraceTransactionEvent, newTraceTransactionTraceHeaders] = await newTraceTransactionPromise;
 
-  expect(customTransactionEvent.type).toEqual('transaction');
-  expect(customTransactionEvent.transaction).toEqual('fetch click');
+  expect(newTraceTransactionEvent.type).toEqual('transaction');
+  expect(newTraceTransactionEvent.transaction).toEqual('new-trace');
 
-  const customTransactionTraceContext = customTransactionEvent.contexts?.trace;
-  expect(customTransactionTraceContext).toMatchObject({
+  const newTraceTransactionTraceContext = newTraceTransactionEvent.contexts?.trace;
+  expect(newTraceTransactionTraceContext).toMatchObject({
     op: 'ui.interaction.click',
     trace_id: expect.stringMatching(/^[0-9a-f]{32}$/),
     span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
   });
 
-  expect(customTransactionTraceHeaders).toEqual({
+  expect(newTraceTransactionTraceHeaders).toEqual({
     environment: 'production',
     public_key: 'public',
     sample_rate: '1',
     sampled: 'true',
-    trace_id: customTransactionTraceContext?.trace_id,
-    transaction: 'fetch click',
+    trace_id: newTraceTransactionTraceContext?.trace_id,
+    transaction: 'new-trace',
   });
 
-  expect(customTransactionTraceContext?.trace_id).not.toEqual(pageloadTraceContext?.trace_id);
+  const oldTraceTransactionPromise = getFirstSentryEnvelopeRequest<EventAndTraceHeader>(
+    page,
+    undefined,
+    eventAndTraceHeaderRequestParser,
+  );
+  await page.locator('#oldTrace').click();
+
+  const [oldTraceTransactionEvent, oldTraceTransactionTraceHeaders] = await oldTraceTransactionPromise;
+
+  expect(oldTraceTransactionEvent.type).toEqual('transaction');
+  expect(oldTraceTransactionEvent.transaction).toEqual('old-trace');
+
+  const oldTraceTransactionEventTraceContext = oldTraceTransactionEvent.contexts?.trace;
+  expect(oldTraceTransactionEventTraceContext).toMatchObject({
+    op: 'ui.interaction.click',
+    trace_id: expect.stringMatching(/^[0-9a-f]{32}$/),
+    span_id: expect.stringMatching(/^[0-9a-f]{16}$/),
+  });
+
+  expect(oldTraceTransactionTraceHeaders).toEqual({
+    environment: 'production',
+    public_key: 'public',
+    sample_rate: '1',
+    sampled: 'true',
+    trace_id: oldTraceTransactionTraceHeaders?.trace_id,
+    // transaction: 'old-trace', <-- this is not in the DSC because the DSC is continued from the pageload transaction
+    // which does not have a `transaction` field because its source is URL.
+  });
+
+  expect(oldTraceTransactionEventTraceContext?.trace_id).toEqual(pageloadTraceContext?.trace_id);
+  expect(newTraceTransactionTraceContext?.trace_id).not.toEqual(pageloadTraceContext?.trace_id);
 });
diff --git a/packages/browser/src/tracing/browserTracingIntegration.ts b/packages/browser/src/tracing/browserTracingIntegration.ts
@@ -28,6 +28,7 @@ import type { Client, IntegrationFn, StartSpanOptions, TransactionSource } from
 import type { Span } from '@sentry/types';
 import {
   browserPerformanceTimeOrigin,
+  generatePropagationContext,
   getDomElement,
   logger,
   propagationContextFromHeaders,
@@ -413,7 +414,8 @@ export function startBrowserTracingPageLoadSpan(
  * This will only do something if a browser tracing integration has been setup.
  */
 export function startBrowserTracingNavigationSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined {
-  startNewTrace();
+  getIsolationScope().setPropagationContext(generatePropagationContext());
+  getCurrentScope().setPropagationContext(generatePropagationContext());
 
   client.emit('startNavigationSpan', spanOptions);
 
diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts
@@ -21,10 +21,9 @@ import type {
   SeverityLevel,
   User,
 } from '@sentry/types';
-import { dateTimestampInSeconds, isPlainObject, logger, uuid4 } from '@sentry/utils';
+import { dateTimestampInSeconds, generatePropagationContext, isPlainObject, logger, uuid4 } from '@sentry/utils';
 
 import { updateSession } from './session';
-import { generatePropagationContext } from './tracing/propagationContext';
 import { _getSpanForScope, _setSpanForScope } from './utils/spanOnScope';
 
 /**
diff --git a/packages/core/src/tracing/trace.ts b/packages/core/src/tracing/trace.ts
@@ -1,5 +1,5 @@
 import type { ClientOptions, Scope, SentrySpanArguments, Span, SpanTimeInput, StartSpanOptions } from '@sentry/types';
-import { logger, propagationContextFromHeaders, uuid4 } from '@sentry/utils';
+import { generatePropagationContext, logger, propagationContextFromHeaders } from '@sentry/utils';
 import type { AsyncContextStrategy } from '../asyncContext/types';
 import { getMainCarrier } from '../carrier';
 
@@ -14,7 +14,6 @@ import { _getSpanForScope, _setSpanForScope } from '../utils/spanOnScope';
 import { addChildSpanToSpan, getRootSpan, spanIsSampled, spanTimeInputToSeconds, spanToJSON } from '../utils/spanUtils';
 import { freezeDscOnSpan, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
 import { logSpanStart } from './logSpans';
-import { generatePropagationContext } from './propagationContext';
 import { sampleSpan } from './sampling';
 import { SentryNonRecordingSpan } from './sentryNonRecordingSpan';
 import { SentrySpan } from './sentrySpan';
@@ -215,22 +214,27 @@ export function suppressTracing<T>(callback: () => T): T {
 }
 
 /**
- * Starts a new trace by creating a new trace id. Spans started after this function is called
- * will be part of the new trace instead of a potentially previous existing trace.
+ * Starts a new trace for the duration of the provided callback. Spans started within the
+ * callback will be part of the new trace instead of a potentially previously started trace.
  *
- * Important: Only use this function if you want to override the default trace lifetime SDK.
- * Please also note that as soon as you call this function, a previously distributed can no longer be
- * continued. Instead, the newly created trace will also be the root of a new distributed trace.
+ * Important: Only use this function if you want to override the default trace lifetime and
+ * propagation mechanism of the SDK for the duration and scope of the provided callback.
+ * The newly created trace will also be the root of a new distributed trace, for example if
+ * you make http requests within the callback.
+ * This function might be useful if the operation you want to instrument should not be part
+ * of a potentially ongoing trace.
  *
  * Default behavior:
  * - Server-side: A new trace is started for each incoming request.
  * - Browser: A new trace is started for each page our route. Navigating to a new route
  *            or page will automatically create a new trace.
  */
-export function startNewTrace(): void {
-  getIsolationScope().setPropagationContext(generatePropagationContext());
-  getCurrentScope().setPropagationContext(generatePropagationContext());
-  DEBUG_BUILD && logger.info(`Starting a new trace with id ${getCurrentScope().getPropagationContext().traceId}`);
+export function startNewTrace<T>(callback: () => T): T {
+  return withScope(scope => {
+    scope.setPropagationContext(generatePropagationContext());
+    DEBUG_BUILD && logger.info(`Starting a new trace with id ${scope.getPropagationContext().traceId}`);
+    return withActiveSpan(null, callback);
+  });
 }
 
 function createChildOrRootSpan({
diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts
@@ -35,3 +35,4 @@ export * from './eventbuilder';
 export * from './anr';
 export * from './lru';
 export * from './buildPolyfills';
+export * from './propagationContext';
diff --git a/packages/utils/src/propagationContext.ts b/packages/utils/src/propagationContext.ts
@@ -1,8 +1,8 @@
 import type { PropagationContext } from '@sentry/types';
-import { uuid4 } from '@sentry/utils';
+import { uuid4 } from './misc';
 
 /**
- * Generates a new minimal propagation context
+ * Returns a new minimal propagation context
  */
 export function generatePropagationContext(): PropagationContext {
   return {
diff --git a/packages/utils/test/proagationContext.test.ts b/packages/utils/test/proagationContext.test.ts
@@ -1,4 +1,4 @@
-import { generatePropagationContext } from '../../../src/tracing/propagationContext';
+import { generatePropagationContext } from '../src/propagationContext';
 
 describe('generatePropagationContext', () => {
   it('generates a new minimal propagation context', () => {

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-import { generatePropagationContext } from '../../../src/tracing/propagationContext';`
	`1`	`+import { generatePropagationContext } from '../src/propagationContext';`
`2`	`2`
`3`	`3`	`describe('generatePropagationContext', () => {`
`4`	`4`	`it('generates a new minimal propagation context', () => {`