ref: Deprecate non-callback based continueTrace

MIGRATION.md

@@ -190,6 +190,32 @@ be removed. Instead, use the new performance APIs:
 
 You can [read more about the new performance APIs here](./docs/v8-new-performance-apis.md).
 
+## Deprecate variations of `Sentry.continueTrace()`
+
+The version of `Sentry.continueTrace()` which does not take a callback argument will be removed in favor of the version
+that does. Additionally, the callback argument will not receive an argument with the next major version.
+
+Use `Sentry.continueTrace()` as follows:
+
+```ts
+app.get('/your-route', req => {
+  Sentry.withIsolationScope(isolationScope => {
+    Sentry.continueTrace(
+      {
+        sentryTrace: req.headers.get('sentry-trace'),
+        baggage: req.headers.get('baggage'),
+      },
+      () => {
+        // All events recorded in this callback will be associated with the incoming trace. For example:
+        Sentry.startSpan({ name: '/my-route' }, async () => {
+          await doExpensiveWork();
+        });
+      },
+    );
+  });
+});
+```
+
 ## Deprecate `Sentry.lastEventId()` and `hub.lastEventId()`
 
 `Sentry.lastEventId()` sometimes causes race conditions, so we are deprecating it in favour of the `beforeSend`

packages/astro/src/server/middleware.ts

@@ -94,97 +94,95 @@ async function instrumentRequest(
 
   const { method, headers } = ctx.request;
 
-  const traceCtx = continueTrace({
-    sentryTrace: headers.get('sentry-trace') || undefined,
-    baggage: headers.get('baggage'),
-  });
+  return continueTrace(
+    {
+      sentryTrace: headers.get('sentry-trace') || undefined,
+      baggage: headers.get('baggage'),
+    },
+    async () => {
+      const allHeaders: Record<string, string> = {};
 
-  const allHeaders: Record<string, string> = {};
+      if (options.trackHeaders) {
+        headers.forEach((value, key) => {
+          allHeaders[key] = value;
+        });
+      }
 
-  if (options.trackHeaders) {
-    headers.forEach((value, key) => {
-      allHeaders[key] = value;
-    });
-  }
+      if (options.trackClientIp) {
+        getCurrentScope().setUser({ ip_address: ctx.clientAddress });
+      }
 
-  if (options.trackClientIp) {
-    getCurrentScope().setUser({ ip_address: ctx.clientAddress });
-  }
+      try {
+        const interpolatedRoute = interpolateRouteFromUrlAndParams(ctx.url.pathname, ctx.params);
+        const source = interpolatedRoute ? 'route' : 'url';
+        // storing res in a variable instead of directly returning is necessary to
+        // invoke the catch block if next() throws
+        const res = await startSpan(
+          {
+            attributes: {
+              [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.astro',
+            },
+            name: `${method} ${interpolatedRoute || ctx.url.pathname}`,
+            op: 'http.server',
+            status: 'ok',
+            data: {
+              method,
+              url: stripUrlQueryAndFragment(ctx.url.href),
+              [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
+              ...(ctx.url.search && { 'http.query': ctx.url.search }),
+              ...(ctx.url.hash && { 'http.fragment': ctx.url.hash }),
+              ...(options.trackHeaders && { headers: allHeaders }),
+            },
+          },
+          async span => {
+            const originalResponse = await next();
 
-  try {
-    const interpolatedRoute = interpolateRouteFromUrlAndParams(ctx.url.pathname, ctx.params);
-    const source = interpolatedRoute ? 'route' : 'url';
-    // storing res in a variable instead of directly returning is necessary to
-    // invoke the catch block if next() throws
-    const res = await startSpan(
-      {
-        ...traceCtx,
-        attributes: {
-          [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.astro',
-        },
-        name: `${method} ${interpolatedRoute || ctx.url.pathname}`,
-        op: 'http.server',
-        status: 'ok',
-        metadata: {
-          // eslint-disable-next-line deprecation/deprecation
-          ...traceCtx?.metadata,
-        },
-        data: {
-          method,
-          url: stripUrlQueryAndFragment(ctx.url.href),
-          [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: source,
-          ...(ctx.url.search && { 'http.query': ctx.url.search }),
-          ...(ctx.url.hash && { 'http.fragment': ctx.url.hash }),
-          ...(options.trackHeaders && { headers: allHeaders }),
-        },
-      },
-      async span => {
-        const originalResponse = await next();
-
-        if (span && originalResponse.status) {
-          setHttpStatus(span, originalResponse.status);
-        }
-
-        const scope = getCurrentScope();
-        const client = getClient();
-        const contentType = originalResponse.headers.get('content-type');
-
-        const isPageloadRequest = contentType && contentType.startsWith('text/html');
-        if (!isPageloadRequest || !client) {
-          return originalResponse;
-        }
-
-        // Type case necessary b/c the body's ReadableStream type doesn't include
-        // the async iterator that is actually available in Node
-        // We later on use the async iterator to read the body chunks
-        // see https://github.com/microsoft/TypeScript/issues/39051
-        const originalBody = originalResponse.body as NodeJS.ReadableStream | null;
-        if (!originalBody) {
-          return originalResponse;
-        }
-
-        const decoder = new TextDecoder();
-
-        const newResponseStream = new ReadableStream({
-          start: async controller => {
-            for await (const chunk of originalBody) {
-              const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true });
-              const modifiedHtml = addMetaTagToHead(html, scope, client, span);
-              controller.enqueue(new TextEncoder().encode(modifiedHtml));
+            if (span && originalResponse.status) {
+              setHttpStatus(span, originalResponse.status);
             }
-            controller.close();
-          },
-        });
 
-        return new Response(newResponseStream, originalResponse);
-      },
-    );
-    return res;
-  } catch (e) {
-    sendErrorToSentry(e);
-    throw e;
-  }
-  // TODO: flush if serverless (first extract function)
+            const scope = getCurrentScope();
+            const client = getClient();
+            const contentType = originalResponse.headers.get('content-type');
+
+            const isPageloadRequest = contentType && contentType.startsWith('text/html');
+            if (!isPageloadRequest || !client) {
+              return originalResponse;
+            }
+
+            // Type case necessary b/c the body's ReadableStream type doesn't include
+            // the async iterator that is actually available in Node
+            // We later on use the async iterator to read the body chunks
+            // see https://github.com/microsoft/TypeScript/issues/39051
+            const originalBody = originalResponse.body as NodeJS.ReadableStream | null;
+            if (!originalBody) {
+              return originalResponse;
+            }
+
+            const decoder = new TextDecoder();
+
+            const newResponseStream = new ReadableStream({
+              start: async controller => {
+                for await (const chunk of originalBody) {
+                  const html = typeof chunk === 'string' ? chunk : decoder.decode(chunk, { stream: true });
+                  const modifiedHtml = addMetaTagToHead(html, scope, client, span);
+                  controller.enqueue(new TextEncoder().encode(modifiedHtml));
+                }
+                controller.close();
+              },
+            });
+
+            return new Response(newResponseStream, originalResponse);
+          },
+        );
+        return res;
+      } catch (e) {
+        sendErrorToSentry(e);
+        throw e;
+      }
+      // TODO: flush if serverless (first extract function)
+    },
+  );
 }
 
 /**

packages/astro/test/server/middleware.test.ts

@@ -66,7 +66,6 @@ describe('sentryMiddleware', () => {
           url: 'https://mydomain.io/users/123/details',
           [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
         },
-        metadata: {},
         name: 'GET /users/[id]/details',
         op: 'http.server',
         status: 'ok',
@@ -104,7 +103,6 @@ describe('sentryMiddleware', () => {
           url: 'http://localhost:1234/a%xx',
           [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
         },
-        metadata: {},
         name: 'GET a%xx',
         op: 'http.server',
         status: 'ok',
@@ -144,43 +142,6 @@ describe('sentryMiddleware', () => {
     });
   });
 
-  it('attaches tracing headers', async () => {
-    const middleware = handleRequest();
-    const ctx = {
-      request: {
-        method: 'GET',
-        url: '/users',
-        headers: new Headers({
-          'sentry-trace': '12345678901234567890123456789012-1234567890123456-1',
-          baggage: 'sentry-release=1.0.0',
-        }),
-      },
-      params: {},
-      url: new URL('https://myDomain.io/users/'),
-    };
-    const next = vi.fn(() => nextResult);
-
-    // @ts-expect-error, a partial ctx object is fine here
-    await middleware(ctx, next);
-
-    expect(startSpanSpy).toHaveBeenCalledWith(
-      expect.objectContaining({
-        data: expect.objectContaining({
-          [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
-        }),
-        metadata: {
-          dynamicSamplingContext: {
-            release: '1.0.0',
-          },
-        },
-        parentSampled: true,
-        parentSpanId: '1234567890123456',
-        traceId: '12345678901234567890123456789012',
-      }),
-      expect.any(Function), // the `next` function
-    );
-  });
-
   it('attaches client IP and request headers if options are set', async () => {
     const middleware = handleRequest({ trackClientIp: true, trackHeaders: true });
     const ctx = {

packages/core/src/tracing/trace.ts

@@ -1,6 +1,5 @@
 import type { Span, SpanTimeInput, StartSpanOptions, TransactionContext } from '@sentry/types';
 
-import type { propagationContextFromHeaders } from '@sentry/utils';
 import { dropUndefinedKeys, logger, tracingContextFromHeaders } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
@@ -225,31 +224,58 @@ export function getActiveSpan(): Span | undefined {
   return getCurrentScope().getSpan();
 }
 
-export function continueTrace({
-  sentryTrace,
-  baggage,
-}: {
-  sentryTrace: Parameters<typeof propagationContextFromHeaders>[0];
-  baggage: Parameters<typeof propagationContextFromHeaders>[1];
-}): Partial<TransactionContext>;
-export function continueTrace<V>(
-  {
+interface ContinueTrace {
+  /**
+   * Continue a trace from `sentry-trace` and `baggage` values.
+   * These values can be obtained from incoming request headers,
+   * or in the browser from `<meta name="sentry-trace">` and `<meta name="baggage">` HTML tags.
+   *
+   * @deprecated Use the version of this function taking a callback as second parameter instead:
+   *
+   * ```
+   * Sentry.continueTrace(sentryTrace: '...', baggage: '...' }, () => {
+   *    // ...
+   * })
+   * ```
+   *
+   */
+  ({
     sentryTrace,
     baggage,
   }: {
-    sentryTrace: Parameters<typeof propagationContextFromHeaders>[0];
-    baggage: Parameters<typeof propagationContextFromHeaders>[1];
-  },
-  callback: (transactionContext: Partial<TransactionContext>) => V,
-): V;
-/**
- * Continue a trace from `sentry-trace` and `baggage` values.
- * These values can be obtained from incoming request headers,
- * or in the browser from `<meta name="sentry-trace">` and `<meta name="baggage">` HTML tags.
- *
- * The callback receives a transactionContext that may be used for `startTransaction` or `startSpan`.
- */
-export function continueTrace<V>(
+    // eslint-disable-next-line deprecation/deprecation
+    sentryTrace: Parameters<typeof tracingContextFromHeaders>[0];
+    // eslint-disable-next-line deprecation/deprecation
+    baggage: Parameters<typeof tracingContextFromHeaders>[1];
+  }): Partial<TransactionContext>;
+
+  /**
+   * Continue a trace from `sentry-trace` and `baggage` values.
+   * These values can be obtained from incoming request headers, or in the browser from `<meta name="sentry-trace">`
+   * and `<meta name="baggage">` HTML tags.
+   *
+   * Spans started with `startSpan`, `startSpanManual` and `startInactiveSpan`, within the callback will automatically
+   * be attached to the incoming trace.
+   *
+   * Deprecation notice: In the next major version of the SDK the provided callback will not receive a transaction
+   * context argument.
+   */
+  <V>(
+    {
+      sentryTrace,
+      baggage,
+    }: {
+      // eslint-disable-next-line deprecation/deprecation
+      sentryTrace: Parameters<typeof tracingContextFromHeaders>[0];
+      // eslint-disable-next-line deprecation/deprecation
+      baggage: Parameters<typeof tracingContextFromHeaders>[1];
+    },
+    // TODO(v8): Remove parameter from this callback.
+    callback: (transactionContext: Partial<TransactionContext>) => V,
+  ): V;
+}
+
+export const continueTrace: ContinueTrace = <V>(
   {
     sentryTrace,
     baggage,
@@ -260,12 +286,14 @@ export function continueTrace<V>(
     baggage: Parameters<typeof tracingContextFromHeaders>[1];
   },
   callback?: (transactionContext: Partial<TransactionContext>) => V,
-): V | Partial<TransactionContext> {
+): V | Partial<TransactionContext> => {
   // TODO(v8): Change this function so it doesn't do anything besides setting the propagation context on the current scope:
   /*
-    const propagationContext = propagationContextFromHeaders(sentryTrace, baggage);
-    getCurrentScope().setPropagationContext(propagationContext);
-    return;
+    return withScope((scope) => {
+      const propagationContext = propagationContextFromHeaders(sentryTrace, baggage);
+      scope.setPropagationContext(propagationContext);
+      return callback();
+    })
   */
 
   const currentScope = getCurrentScope();
@@ -293,8 +321,10 @@ export function continueTrace<V>(
     return transactionContext;
   }
 
-  return callback(transactionContext);
-}
+  return runWithAsyncContext(() => {
+    return callback(transactionContext);
+  });
+};
 
 function createChildSpanOrTransaction(
   hub: Hub,