fix(core): Only start spans in trace if tracing is enabled (#8357)

In our (still internal) `trace` function, we don't yet check if
`hasTracingEnabled` returns `true` before trying to start a span or a
transaction. This leads to the following problematic UX path that was
discovered
[here](#5838 (reply in thread)):

1. Users don't set `tracesSampleRate` (or `tracesSampler`), causing some
SDKs (NextJS, SvelteKit) to **not** add the `BrowserTracing`
integration, which in turn means that tracing extension methods are not
added/registered.
2. Users or, more likely, other parts of our SDK (e.g. SvelteKit's
wrappers/handlers) call `trace` which will still try to start a
span/txn.
3. Users will get a console warning that tracing extension methods were
not installed and they should manually call `addExtensionMethods` which
is completely misleading in this case.

This fix makes `trace` check for `hasTracingEnabled()` in which case it
will not start a span but just invoke the error callback if an error
occurred.

Author: Lms24

packages/core/src/tracing/trace.ts

@@ -2,13 +2,15 @@ import type { TransactionContext } from '@sentry/types';
 import { isThenable } from '@sentry/utils';
 
 import { getCurrentHub } from '../hub';
+import { hasTracingEnabled } from '../utils/hasTracingEnabled';
 import type { Span } from './span';
 
 /**
  * Wraps a function with a transaction/span and finishes the span after the function is done.
  *
- * Note that if you have not enabled tracing extensions via `addTracingExtensions`, this function
- * will not generate spans, and the `span` returned from the callback may be undefined.
+ * Note that if you have not enabled tracing extensions via `addTracingExtensions`
+ * or you didn't set `tracesSampleRate`, this function will not generate spans
+ * and the `span` returned from the callback will be undefined.
  *
  * This function is meant to be used internally and may break at any time. Use at your own risk.
  *
@@ -31,7 +33,15 @@ export function trace<T>(
   const scope = hub.getScope();
 
   const parentSpan = scope.getSpan();
-  const activeSpan = parentSpan ? parentSpan.startChild(ctx) : hub.startTransaction(ctx);
+
+  function getActiveSpan(): Span | undefined {
+    if (!hasTracingEnabled()) {
+      return undefined;
+    }
+    return parentSpan ? parentSpan.startChild(ctx) : hub.startTransaction(ctx);
+  }
+
+  const activeSpan = getActiveSpan();
   scope.setSpan(activeSpan);
 
   function finishAndSetSpan(): void {

packages/core/test/lib/tracing/trace.test.ts

@@ -185,5 +185,33 @@ describe('trace', () => {
       }
       expect(onError).toHaveBeenCalledTimes(isError ? 1 : 0);
     });
+
+    it("doesn't create spans but calls onError if tracing is disabled", async () => {
+      const options = getDefaultTestClientOptions({
+        /* we don't set tracesSampleRate or tracesSampler */
+      });
+      client = new TestClient(options);
+      hub = new Hub(client);
+      makeMain(hub);
+
+      const startTxnSpy = jest.spyOn(hub, 'startTransaction');
+
+      const onError = jest.fn();
+      try {
+        await trace(
+          { name: 'GET users/[id]' },
+          () => {
+            return callback();
+          },
+          onError,
+        );
+      } catch (e) {
+        expect(onError).toHaveBeenCalledTimes(1);
+        expect(onError).toHaveBeenCalledWith(e);
+      }
+      expect(onError).toHaveBeenCalledTimes(isError ? 1 : 0);
+
+      expect(startTxnSpy).not.toHaveBeenCalled();
+    });
   });
 });