feat(tracing): Record transaction name source when name set directly (#…

…5396)

This adds transaction name source to transaction metadata in various situations in which the name is set directly: starting a manual transaction, assigning to the `name` property of a transaction, calling a transaction's `setName` method, and changing the name using `beforeNavigate`.

In order to distinguish manually-created transactions from automatically-created ones, all internal uses of `startTransaction` have been changed so that they are happening directly on the hub, rather than through the top-level `Sentry.startTransaction()` wrapper.

packages/hub/src/exports.ts

@@ -168,6 +168,9 @@ export function withScope(callback: (scope: Scope) => void): ReturnType<Hub['wit
  * The transaction must be finished with a call to its `.finish()` method, at which point the transaction with all its
  * finished child spans will be sent to Sentry.
  *
+ * NOTE: This function should only be used for *manual* instrumentation. Auto-instrumentation should call
+ * `startTransaction` directly on the hub.
+ *
  * @param context Properties of the new `Transaction`.
  * @param customSamplingContext Information given to the transaction sampling function (along with context-dependent
  * default values). See {@link Options.tracesSampler}.
@@ -178,5 +181,11 @@ export function startTransaction(
   context: TransactionContext,
   customSamplingContext?: CustomSamplingContext,
 ): ReturnType<Hub['startTransaction']> {
-  return getCurrentHub().startTransaction({ ...context }, customSamplingContext);
+  return getCurrentHub().startTransaction(
+    {
+      metadata: { source: 'custom' },
+      ...context,
+    },
+    customSamplingContext,
+  );
 }

packages/hub/test/exports.test.ts

@@ -10,6 +10,7 @@ import {
   setTag,
   setTags,
   setUser,
+  startTransaction,
   withScope,
 } from '../src/exports';
 
@@ -184,6 +185,35 @@ describe('Top Level API', () => {
     });
   });
 
+  describe('startTransaction', () => {
+    beforeEach(() => {
+      global.__SENTRY__ = {
+        hub: undefined,
+        extensions: {
+          startTransaction: (context: any) => ({
+            name: context.name,
+            // Spread rather than assign in case it's undefined
+            metadata: { ...context.metadata },
+          }),
+        },
+      };
+    });
+
+    it("sets source to `'custom'` if no source provided", () => {
+      const transaction = startTransaction({ name: 'dogpark' });
+
+      expect(transaction.name).toEqual('dogpark');
+      expect(transaction.metadata.source).toEqual('custom');
+    });
+
+    it('uses given `source` value', () => {
+      const transaction = startTransaction({ name: 'dogpark', metadata: { source: 'route' } });
+
+      expect(transaction.name).toEqual('dogpark');
+      expect(transaction.metadata.source).toEqual('route');
+    });
+  });
+
   test('Clear Scope', () => {
     const client: any = new TestClient({});
     getCurrentHub().withScope(() => {

packages/remix/src/utils/instrumentServer.ts

@@ -1,4 +1,4 @@
-import { captureException, getCurrentHub, startTransaction } from '@sentry/node';
+import { captureException, getCurrentHub } from '@sentry/node';
 import { getActiveTransaction } from '@sentry/tracing';
 import { addExceptionMechanism, fill, loadModule, logger, stripUrlQueryAndFragment } from '@sentry/utils';
 
@@ -175,8 +175,9 @@ function makeWrappedLoader(origAction: DataFunction): DataFunction {
 
 function wrapRequestHandler(origRequestHandler: RequestHandler): RequestHandler {
   return async function (this: unknown, request: Request, loadContext?: unknown): Promise<Response> {
-    const currentScope = getCurrentHub().getScope();
-    const transaction = startTransaction({
+    const hub = getCurrentHub();
+    const currentScope = hub.getScope();
+    const transaction = hub.startTransaction({
       name: stripUrlQueryAndFragment(request.url),
       op: 'http.server',
       tags: {

packages/serverless/src/awslambda.ts

@@ -1,14 +1,6 @@
 /* eslint-disable max-lines */
 import * as Sentry from '@sentry/node';
-import {
-  captureException,
-  captureMessage,
-  flush,
-  getCurrentHub,
-  Scope,
-  startTransaction,
-  withScope,
-} from '@sentry/node';
+import { captureException, captureMessage, flush, getCurrentHub, Scope, withScope } from '@sentry/node';
 import { extractTraceparentData } from '@sentry/tracing';
 import { Integration } from '@sentry/types';
 import { dsnFromString, dsnToString, isString, logger, parseBaggageSetMutability } from '@sentry/utils';
@@ -320,14 +312,15 @@ export function wrapHandler<TEvent, TResult>(
       eventWithHeaders.headers && isString(eventWithHeaders.headers.baggage) && eventWithHeaders.headers.baggage;
     const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
-    const transaction = startTransaction({
+    const hub = getCurrentHub();
+
+    const transaction = hub.startTransaction({
       name: context.functionName,
       op: 'awslambda.handler',
       ...traceparentData,
       metadata: { baggage, source: 'component' },
     });
 
-    const hub = getCurrentHub();
     const scope = hub.pushScope();
     let rv: TResult;
     try {

packages/serverless/src/gcpfunction/cloud_events.ts

@@ -1,4 +1,4 @@
-import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
+import { captureException, flush, getCurrentHub } from '@sentry/node';
 import { logger } from '@sentry/utils';
 
 import { domainify, getActiveDomain, proxyFunction } from '../utils';
@@ -30,7 +30,9 @@ function _wrapCloudEventFunction(
     ...wrapOptions,
   };
   return (context, callback) => {
-    const transaction = startTransaction({
+    const hub = getCurrentHub();
+
+    const transaction = hub.startTransaction({
       name: context.type || '<unknown>',
       op: 'gcp.function.cloud_event',
       metadata: { source: 'component' },
@@ -39,7 +41,7 @@ function _wrapCloudEventFunction(
     // getCurrentHub() is expected to use current active domain as a carrier
     // since functions-framework creates a domain for each incoming request.
     // So adding of event processors every time should not lead to memory bloat.
-    getCurrentHub().configureScope(scope => {
+    hub.configureScope(scope => {
       scope.setContext('gcp.function.context', { ...context });
       // We put the transaction on the scope so users can attach children to it
       scope.setSpan(transaction);

packages/serverless/src/gcpfunction/events.ts

@@ -1,4 +1,4 @@
-import { captureException, flush, getCurrentHub, startTransaction } from '@sentry/node';
+import { captureException, flush, getCurrentHub } from '@sentry/node';
 import { logger } from '@sentry/utils';
 
 import { domainify, getActiveDomain, proxyFunction } from '../utils';
@@ -30,7 +30,9 @@ function _wrapEventFunction(
     ...wrapOptions,
   };
   return (data, context, callback) => {
-    const transaction = startTransaction({
+    const hub = getCurrentHub();
+
+    const transaction = hub.startTransaction({
       name: context.eventType,
       op: 'gcp.function.event',
       metadata: { source: 'component' },
@@ -39,7 +41,7 @@ function _wrapEventFunction(
     // getCurrentHub() is expected to use current active domain as a carrier
     // since functions-framework creates a domain for each incoming request.
     // So adding of event processors every time should not lead to memory bloat.
-    getCurrentHub().configureScope(scope => {
+    hub.configureScope(scope => {
       scope.setContext('gcp.function.context', { ...context });
       // We put the transaction on the scope so users can attach children to it
       scope.setSpan(transaction);

packages/serverless/src/gcpfunction/http.ts

@@ -4,7 +4,6 @@ import {
   captureException,
   flush,
   getCurrentHub,
-  startTransaction,
 } from '@sentry/node';
 import { extractTraceparentData } from '@sentry/tracing';
 import { isString, logger, parseBaggageSetMutability, stripUrlQueryAndFragment } from '@sentry/utils';
@@ -83,7 +82,9 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
 
     const baggage = parseBaggageSetMutability(rawBaggageString, traceparentData);
 
-    const transaction = startTransaction({
+    const hub = getCurrentHub();
+
+    const transaction = hub.startTransaction({
       name: `${reqMethod} ${reqUrl}`,
       op: 'gcp.function.http',
       ...traceparentData,
@@ -93,7 +94,7 @@ function _wrapHttpFunction(fn: HttpFunction, wrapOptions: Partial<HttpFunctionWr
     // getCurrentHub() is expected to use current active domain as a carrier
     // since functions-framework creates a domain for each incoming request.
     // So adding of event processors every time should not lead to memory bloat.
-    getCurrentHub().configureScope(scope => {
+    hub.configureScope(scope => {
       scope.addEventProcessor(event => addRequestDataToEvent(event, req, options.addRequestDataToEventOptions));
       // We put the transaction on the scope so users can attach children to it
       scope.setSpan(transaction);

packages/serverless/test/__mocks__/@sentry/node.ts

@@ -11,6 +11,7 @@ export const fakeHub = {
   pushScope: jest.fn(() => fakeScope),
   popScope: jest.fn(),
   getScope: jest.fn(() => fakeScope),
+  startTransaction: jest.fn(context => ({ ...fakeTransaction, ...context })),
 };
 export const fakeScope = {
   addEventProcessor: jest.fn(),

packages/serverless/test/awslambda.test.ts

@@ -38,9 +38,11 @@ const fakeCallback: Callback = (err, result) => {
   return err;
 };
 
-function expectScopeSettings() {
+function expectScopeSettings(fakeTransactionContext: any) {
   // @ts-ignore see "Why @ts-ignore" note
-  expect(Sentry.fakeScope.setSpan).toBeCalledWith(Sentry.fakeTransaction);
+  const fakeTransaction = { ...Sentry.fakeTransaction, ...fakeTransactionContext };
+  // @ts-ignore see "Why @ts-ignore" note
+  expect(Sentry.fakeScope.setSpan).toBeCalledWith(fakeTransaction);
   // @ts-ignore see "Why @ts-ignore" note
   expect(Sentry.fakeScope.setTag).toBeCalledWith('server_name', expect.anything());
   // @ts-ignore see "Why @ts-ignore" note
@@ -186,13 +188,17 @@ describe('AWSLambda', () => {
       };
       const wrappedHandler = wrapHandler(handler);
       const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
-      expect(rv).toStrictEqual(42);
-      expect(Sentry.startTransaction).toBeCalledWith({
+
+      const fakeTransactionContext = {
         name: 'functionName',
         op: 'awslambda.handler',
         metadata: { baggage: [{}, '', true], source: 'component' },
-      });
-      expectScopeSettings();
+      };
+
+      expect(rv).toStrictEqual(42);
+      // @ts-ignore see "Why @ts-ignore" note
+      expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
+      expectScopeSettings(fakeTransactionContext);
       // @ts-ignore see "Why @ts-ignore" note
       expect(Sentry.fakeTransaction.finish).toBeCalled();
       expect(Sentry.flush).toBeCalledWith(2000);
@@ -210,12 +216,15 @@ describe('AWSLambda', () => {
       try {
         await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
       } catch (e) {
-        expect(Sentry.startTransaction).toBeCalledWith({
+        const fakeTransactionContext = {
           name: 'functionName',
           op: 'awslambda.handler',
           metadata: { baggage: [{}, '', true], source: 'component' },
-        });
-        expectScopeSettings();
+        };
+
+        // @ts-ignore see "Why @ts-ignore" note
+        expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
+        expectScopeSettings(fakeTransactionContext);
         expect(Sentry.captureException).toBeCalledWith(error);
         // @ts-ignore see "Why @ts-ignore" note
         expect(Sentry.fakeTransaction.finish).toBeCalled();
@@ -244,7 +253,8 @@ describe('AWSLambda', () => {
       };
 
       const handler: Handler = (_event, _context, callback) => {
-        expect(Sentry.startTransaction).toBeCalledWith(
+        // @ts-ignore see "Why @ts-ignore" note
+        expect(Sentry.fakeHub.startTransaction).toBeCalledWith(
           expect.objectContaining({
             parentSpanId: '1121201211212012',
             parentSampled: false,
@@ -284,15 +294,18 @@ describe('AWSLambda', () => {
         fakeEvent.headers = { 'sentry-trace': '12312012123120121231201212312012-1121201211212012-0' };
         await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
       } catch (e) {
-        expect(Sentry.startTransaction).toBeCalledWith({
+        const fakeTransactionContext = {
           name: 'functionName',
           op: 'awslambda.handler',
           traceId: '12312012123120121231201212312012',
           parentSpanId: '1121201211212012',
           parentSampled: false,
           metadata: { baggage: [{}, '', false], source: 'component' },
-        });
-        expectScopeSettings();
+        };
+
+        // @ts-ignore see "Why @ts-ignore" note
+        expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
+        expectScopeSettings(fakeTransactionContext);
         expect(Sentry.captureException).toBeCalledWith(e);
         // @ts-ignore see "Why @ts-ignore" note
         expect(Sentry.fakeTransaction.finish).toBeCalled();
@@ -310,13 +323,17 @@ describe('AWSLambda', () => {
       };
       const wrappedHandler = wrapHandler(handler);
       const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
-      expect(rv).toStrictEqual(42);
-      expect(Sentry.startTransaction).toBeCalledWith({
+
+      const fakeTransactionContext = {
         name: 'functionName',
         op: 'awslambda.handler',
         metadata: { baggage: [{}, '', true], source: 'component' },
-      });
-      expectScopeSettings();
+      };
+
+      expect(rv).toStrictEqual(42);
+      // @ts-ignore see "Why @ts-ignore" note
+      expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
+      expectScopeSettings(fakeTransactionContext);
       // @ts-ignore see "Why @ts-ignore" note
       expect(Sentry.fakeTransaction.finish).toBeCalled();
       expect(Sentry.flush).toBeCalled();
@@ -345,12 +362,15 @@ describe('AWSLambda', () => {
       try {
         await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
       } catch (e) {
-        expect(Sentry.startTransaction).toBeCalledWith({
+        const fakeTransactionContext = {
           name: 'functionName',
           op: 'awslambda.handler',
           metadata: { baggage: [{}, '', true], source: 'component' },
-        });
-        expectScopeSettings();
+        };
+
+        // @ts-ignore see "Why @ts-ignore" note
+        expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
+        expectScopeSettings(fakeTransactionContext);
         expect(Sentry.captureException).toBeCalledWith(error);
         // @ts-ignore see "Why @ts-ignore" note
         expect(Sentry.fakeTransaction.finish).toBeCalled();
@@ -383,13 +403,17 @@ describe('AWSLambda', () => {
       };
       const wrappedHandler = wrapHandler(handler);
       const rv = await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
-      expect(rv).toStrictEqual(42);
-      expect(Sentry.startTransaction).toBeCalledWith({
+
+      const fakeTransactionContext = {
         name: 'functionName',
         op: 'awslambda.handler',
         metadata: { baggage: [{}, '', true], source: 'component' },
-      });
-      expectScopeSettings();
+      };
+
+      expect(rv).toStrictEqual(42);
+      // @ts-ignore see "Why @ts-ignore" note
+      expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
+      expectScopeSettings(fakeTransactionContext);
       // @ts-ignore see "Why @ts-ignore" note
       expect(Sentry.fakeTransaction.finish).toBeCalled();
       expect(Sentry.flush).toBeCalled();
@@ -418,12 +442,15 @@ describe('AWSLambda', () => {
       try {
         await wrappedHandler(fakeEvent, fakeContext, fakeCallback);
       } catch (e) {
-        expect(Sentry.startTransaction).toBeCalledWith({
+        const fakeTransactionContext = {
           name: 'functionName',
           op: 'awslambda.handler',
           metadata: { baggage: [{}, '', true], source: 'component' },
-        });
-        expectScopeSettings();
+        };
+
+        // @ts-ignore see "Why @ts-ignore" note
+        expect(Sentry.fakeHub.startTransaction).toBeCalledWith(fakeTransactionContext);
+        expectScopeSettings(fakeTransactionContext);
         expect(Sentry.captureException).toBeCalledWith(error);
         // @ts-ignore see "Why @ts-ignore" note
         expect(Sentry.fakeTransaction.finish).toBeCalled();