feat(tracing): Add hook for trace sampling function to SDK options (#…

…2820) Currently, tracing is either on, with a static sample rate for all transactions, or off. This doesn't let users either a) sample different transactions at different rates or b) filter out certain transactions all together. This PR introduces a new SDK option, `tracesSampler`, whose value is a callback with the signature `tracesSampler(samplingContext: SamplingContext): number | boolean`, where `samplingContext` is a combination of automatically-provided data (varying by SDK/integration) and data passed by the user to `startTransaction()` as an optional third parameter. Given that data, the `tracesSampler` function can then compute the appropriate sample rate and return it to the sampling function. Example use cases for variable sample rate: - Collect traces for all enterprise customers but only a fraction of regular customers - Collect a higher percentage of traces on rarely-visited pages, to ensure a decent sample size - Collect more traces on a page you recently revamped compared to one whose code you already have well-profiled Example use cases for filtering: - Don't trace pre-flight CORS requests - Don't trace health check requests - Don't collect traces from users with a certain browser extension enabled, because it's known to break things - Don't trace navigation to deprecated parts of your web app (A user can, of course, combine these ideas as well, returning a variable sample rate for some traces and 0 to filter out others.) In order to support this addition, a few other changes needed to be made: - Move method for extracting node request data to `@sentry/utils` - Break up `misc.ts` to eliminate circular dependencies within `@sentry/utils` - Move method for extracting `sentry-trace` data into `@sentry/tracing` utils - Tweak some types related to domains and request data, and polyfill `WorkerLocation` type
getsentry · Sep 9, 2020 · 7c14283 · 7c14283
1 parent f398514
commit 7c14283
Show file tree

Hide file tree

Showing 32 changed files with 1,134 additions and 436 deletions.
diff --git a/packages/gatsby/README.md b/packages/gatsby/README.md
@@ -40,7 +40,7 @@ To automatically capture the `release` value on Vercel you will need to register
 
 ## Sentry Performance
 
-To enable Tracing support, supply the `tracesSampleRate` to the options and make sure you have installed the `@sentry/tracing` package. This will also turn on the `BrowserTracing` integration for automatic instrumentation of the browser.
+To enable tracing, supply either `tracesSampleRate` or `tracesSampler` to the options and make sure you have installed the `@sentry/tracing` package. This will also turn on the `BrowserTracing` integration for automatic instrumentation of pageloads and navigations.
 
 ```javascript
 {
@@ -49,8 +49,31 @@ To enable Tracing support, supply the `tracesSampleRate` to the options and make
     {
       resolve: "@sentry/gatsby",
       options: {
-          dsn: process.env.SENTRY_DSN, // this is the default
-          tracesSampleRate: 1, // this is just to test, you should lower this in production
+        dsn: process.env.SENTRY_DSN, // this is the default
+
+        // A rate of 1 means all traces will be sent, so it's good for testing.
+        // In production, you'll likely want to either choose a lower rate or use `tracesSampler` instead (see below).
+        tracesSampleRate: 1,
+
+        // Alternatively:
+        tracesSampler: samplingContext => {
+          // Examine provided context data (along with anything in the global namespace) to decide the sample rate
+          // for this transaction.
+          // Can return 0 to drop the transaction entirely.
+
+          if ("...") {
+            return 0.5 // These are important - take a big sample
+          }
+          else if ("...") {
+            return 0.01 // These are less important or happen much more frequently - only take 1% of them
+          }
+          else if ("...") {
+            return 0 // These aren't something worth tracking - drop all transactions like this
+          }
+          else {
+            return 0.1 // Default sample rate
+          }
+        }
       }
     },
     // ...
@@ -68,7 +91,7 @@ If you want to supply options to the `BrowserTracing` integration, use the `brow
       resolve: "@sentry/gatsby",
       options: {
           dsn: process.env.SENTRY_DSN, // this is the default
-          tracesSampleRate: 1, // this is just to test, you should lower this in production
+          tracesSampleRate: 1, // or tracesSampler (see above)
           browserTracingOptions: {
             // disable creating spans for XHR requests
             traceXHR: false,

diff --git a/packages/gatsby/gatsby-browser.js b/packages/gatsby/gatsby-browser.js
@@ -6,10 +6,9 @@ exports.onClientEntry = function(_, pluginParams) {
     return;
   }
 
-  const tracesSampleRate = pluginParams.tracesSampleRate !== undefined ? pluginParams.tracesSampleRate : 0;
   const integrations = [...(pluginParams.integrations || [])];
 
-  if (tracesSampleRate && !integrations.some(ele => ele.name === 'BrowserTracing')) {
+  if (Tracing.hasTracingEnabled(pluginParams) && !integrations.some(ele => ele.name === 'BrowserTracing')) {
     integrations.push(new Tracing.Integrations.BrowserTracing(pluginParams.browserTracingOptions));
   }
 
@@ -22,7 +21,6 @@ exports.onClientEntry = function(_, pluginParams) {
     // eslint-disable-next-line no-undef
     dsn: __SENTRY_DSN__,
     ...pluginParams,
-    tracesSampleRate,
     integrations,
   });
 

diff --git a/packages/gatsby/test/gatsby-browser.test.ts b/packages/gatsby/test/gatsby-browser.test.ts
@@ -53,7 +53,6 @@ describe('onClientEntry', () => {
       environment: process.env.NODE_ENV,
       integrations: [],
       release: (global as any).__SENTRY_RELEASE__,
-      tracesSampleRate: 0,
     });
   });
 
@@ -100,6 +99,16 @@ describe('onClientEntry', () => {
     );
   });
 
+  it('sets a tracesSampler if defined as option', () => {
+    const tracesSampler = jest.fn();
+    onClientEntry(undefined, { tracesSampler });
+    expect(sentryInit).toHaveBeenLastCalledWith(
+      expect.objectContaining({
+        tracesSampler,
+      }),
+    );
+  });
+
   it('adds `BrowserTracing` integration if tracesSampleRate is defined', () => {
     onClientEntry(undefined, { tracesSampleRate: 0.5 });
     expect(sentryInit).toHaveBeenLastCalledWith(
@@ -109,6 +118,16 @@ describe('onClientEntry', () => {
     );
   });
 
+  it('adds `BrowserTracing` integration if tracesSampler is defined', () => {
+    const tracesSampler = jest.fn();
+    onClientEntry(undefined, { tracesSampler });
+    expect(sentryInit).toHaveBeenLastCalledWith(
+      expect.objectContaining({
+        integrations: [expect.objectContaining({ name: 'BrowserTracing' })],
+      }),
+    );
+  });
+
   it('only defines a single `BrowserTracing` integration', () => {
     const Tracing = jest.requireActual('@sentry/tracing');
     const integrations = [new Tracing.Integrations.BrowserTracing()];

diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts
@@ -3,6 +3,7 @@ import {
   Breadcrumb,
   BreadcrumbHint,
   Client,
+  CustomSamplingContext,
   Event,
   EventHint,
   Extra,
@@ -19,7 +20,7 @@ import {
 } from '@sentry/types';
 import { consoleSandbox, getGlobalObject, isNodeEnv, logger, timestampWithMs, uuid4 } from '@sentry/utils';
 
-import { Carrier, Layer } from './interfaces';
+import { Carrier, DomainAsCarrier, Layer } from './interfaces';
 import { Scope } from './scope';
 
 /**
@@ -369,8 +370,8 @@ export class Hub implements HubInterface {
   /**
    * @inheritDoc
    */
-  public startTransaction(context: TransactionContext): Transaction {
-    return this._callExtensionMethod('startTransaction', context);
+  public startTransaction(context: TransactionContext, customSamplingContext?: CustomSamplingContext): Transaction {
+    return this._callExtensionMethod('startTransaction', context, customSamplingContext);
   }
 
   /**
@@ -456,22 +457,24 @@ export function getCurrentHub(): Hub {
   return getHubFromCarrier(registry);
 }
 
+/**
+ * Returns the active domain, if one exists
+ *
+ * @returns The domain, or undefined if there is no active domain
+ */
+export function getActiveDomain(): DomainAsCarrier | undefined {
+  const sentry = getMainCarrier().__SENTRY__;
+
+  return sentry && sentry.extensions && sentry.extensions.domain && sentry.extensions.domain.active;
+}
+
 /**
  * Try to read the hub from an active domain, and fallback to the registry if one doesn't exist
  * @returns discovered hub
  */
 function getHubFromActiveDomain(registry: Carrier): Hub {
   try {
-    const property = 'domain';
-    const carrier = getMainCarrier();
-    const sentry = carrier.__SENTRY__;
-    if (!sentry || !sentry.extensions || !sentry.extensions[property]) {
-      return getHubFromCarrier(registry);
-    }
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    const domain = sentry.extensions[property] as any;
-    // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-    const activeDomain = domain.active;
+    const activeDomain = getActiveDomain();
 
     // If there's no active domain, just return global hub
     if (!activeDomain) {

diff --git a/packages/hub/src/index.ts b/packages/hub/src/index.ts
@@ -1,3 +1,11 @@
-export { Carrier, Layer } from './interfaces';
+export { Carrier, DomainAsCarrier, Layer } from './interfaces';
 export { addGlobalEventProcessor, Scope } from './scope';
-export { getCurrentHub, getHubFromCarrier, getMainCarrier, Hub, makeMain, setHubOnCarrier } from './hub';
+export {
+  getActiveDomain,
+  getCurrentHub,
+  getHubFromCarrier,
+  getMainCarrier,
+  Hub,
+  makeMain,
+  setHubOnCarrier,
+} from './hub';
diff --git a/packages/hub/src/interfaces.ts b/packages/hub/src/interfaces.ts
@@ -1,4 +1,5 @@
 import { Client } from '@sentry/types';
+import * as domain from 'domain';
 
 import { Hub } from './hub';
 import { Scope } from './scope';
@@ -20,9 +21,23 @@ export interface Carrier {
   __SENTRY__?: {
     hub?: Hub;
     /**
-     * These are extension methods for the hub, the current instance of the hub will be bound to it
+     * Extra Hub properties injected by various SDKs
      */
-    // eslint-disable-next-line @typescript-eslint/ban-types
-    extensions?: { [key: string]: Function };
+    extensions?: {
+      /** Hack to prevent bundlers from breaking our usage of the domain package in the cross-platform Hub package */
+      domain?: typeof domain & {
+        /**
+         * The currently active domain. This is part of the domain package, but for some reason not declared in the
+         * package's typedef.
+         */
+        active?: domain.Domain;
+      };
+    } & {
+      /** Extension methods for the hub, which are bound to the current Hub instance */
+      // eslint-disable-next-line @typescript-eslint/ban-types
+      [key: string]: Function;
+    };
   };
 }
+
+export interface DomainAsCarrier extends domain.Domain, Carrier {}
diff --git a/packages/minimal/src/index.ts b/packages/minimal/src/index.ts
@@ -2,6 +2,7 @@ import { getCurrentHub, Hub, Scope } from '@sentry/hub';
 import {
   Breadcrumb,
   CaptureContext,
+  CustomSamplingContext,
   Event,
   Extra,
   Extras,
@@ -190,22 +191,25 @@ export function _callOnClient(method: string, ...args: any[]): void {
 }
 
 /**
- * Starts a new `Transaction` and returns it. This is the entry point to manual
- * tracing instrumentation.
+ * Starts a new `Transaction` and returns it. This is the entry point to manual tracing instrumentation.
  *
- * A tree structure can be built by adding child spans to the transaction, and
- * child spans to other spans. To start a new child span within the transaction
- * or any span, call the respective `.startChild()` method.
+ * A tree structure can be built by adding child spans to the transaction, and child spans to other spans. To start a
+ * new child span within the transaction or any span, call the respective `.startChild()` method.
  *
- * Every child span must be finished before the transaction is finished,
- * otherwise the unfinished spans are discarded.
+ * Every child span must be finished before the transaction is finished, otherwise the unfinished spans are discarded.
  *
- * 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.
+ * 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.
  *
  * @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}.
+ *
+ * @returns The transaction which was just started
  */
-export function startTransaction(context: TransactionContext): Transaction {
-  return callOnHub('startTransaction', { ...context });
+export function startTransaction(
+  context: TransactionContext,
+  customSamplingContext?: CustomSamplingContext,
+): Transaction {
+  return callOnHub('startTransaction', { ...context }, customSamplingContext);
 }