feat(tracing): Send sample rate and type in transaction item header in envelope (#3068)

MVP of this feature for now, as it's only being used internally at the moment.

Author: lobsterkatie

packages/core/src/request.ts

@@ -20,6 +20,10 @@ export function sessionToSentryRequest(session: Session, api: API): SentryReques
 
 /** Creates a SentryRequest from an event. */
 export function eventToSentryRequest(event: Event, api: API): SentryRequest {
+  // since JS has no Object.prototype.pop()
+  const { __sentry_samplingMethod: samplingMethod, __sentry_sampleRate: sampleRate, ...otherTags } = event.tags || {};
+  event.tags = otherTags;
+
   const useEnvelope = event.type === 'transaction';
 
   const req: SentryRequest = {
@@ -41,6 +45,11 @@ export function eventToSentryRequest(event: Event, api: API): SentryRequest {
     });
     const itemHeaders = JSON.stringify({
       type: event.type,
+
+      // TODO: Right now, sampleRate may or may not be defined (it won't be in the cases of inheritance and
+      // explicitly-set sampling decisions). Are we good with that?
+      sample_rates: [{ id: samplingMethod, rate: sampleRate }],
+
       // The content-type is assumed to be 'application/json' and not part of
       // the current spec for transaction items, so we don't bloat the request
       // body with it.

packages/core/test/lib/request.test.ts

@@ -0,0 +1,56 @@
+import { Event, TransactionSamplingMethod } from '@sentry/types';
+
+import { API } from '../../src/api';
+import { eventToSentryRequest } from '../../src/request';
+
+describe('eventToSentryRequest', () => {
+  const api = new API('https://[email protected]/12312012');
+  const event: Event = {
+    contexts: { trace: { trace_id: '1231201211212012', span_id: '12261980', op: 'pageload' } },
+    environment: 'dogpark',
+    event_id: '0908201304152013',
+    release: 'off.leash.park',
+    spans: [],
+    transaction: '/dogs/are/great/',
+    type: 'transaction',
+    user: { id: '1121', username: 'CharlieDog', ip_address: '11.21.20.12' },
+  };
+
+  [
+    { method: TransactionSamplingMethod.Rate, rate: '0.1121', dog: 'Charlie' },
+    { method: TransactionSamplingMethod.Sampler, rate: '0.1231', dog: 'Maisey' },
+    { method: TransactionSamplingMethod.Inheritance, dog: 'Cory' },
+    { method: TransactionSamplingMethod.Explicit, dog: 'Bodhi' },
+
+    // this shouldn't ever happen (at least the method should always be included in tags), but good to know that things
+    // won't blow up if it does
+    { dog: 'Lucy' },
+  ].forEach(({ method, rate, dog }) => {
+    it(`adds transaction sampling information to item header - ${method}, ${rate}, ${dog}`, () => {
+      // TODO kmclb - once tag types are loosened, don't need to cast to string here
+      event.tags = { __sentry_samplingMethod: String(method), __sentry_sampleRate: String(rate), dog };
+
+      const result = eventToSentryRequest(event as Event, api);
+
+      const [envelopeHeaderString, itemHeaderString, eventString] = result.body.split('\n');
+
+      const envelope = {
+        envelopeHeader: JSON.parse(envelopeHeaderString),
+        itemHeader: JSON.parse(itemHeaderString),
+        event: JSON.parse(eventString),
+      };
+
+      // the right stuff is added to the item header
+      expect(envelope.itemHeader).toEqual({
+        type: 'transaction',
+        // TODO kmclb - once tag types are loosened, don't need to cast to string here
+        sample_rates: [{ id: String(method), rate: String(rate) }],
+      });
+
+      // show that it pops the right tags and leaves the rest alone
+      expect('__sentry_samplingMethod' in envelope.event.tags).toBe(false);
+      expect('__sentry_sampleRate' in envelope.event.tags).toBe(false);
+      expect('dog' in envelope.event.tags).toBe(true);
+    });
+  });
+});

packages/tracing/src/hubextensions.ts

@@ -1,5 +1,5 @@
 import { getActiveDomain, getMainCarrier, Hub } from '@sentry/hub';
-import { CustomSamplingContext, SamplingContext, TransactionContext } from '@sentry/types';
+import { CustomSamplingContext, SamplingContext, TransactionContext, TransactionSamplingMethod } from '@sentry/types';
 import {
   dynamicRequire,
   extractNodeRequestData,
@@ -28,18 +28,6 @@ function traceHeaders(this: Hub): { [key: string]: string } {
   return {};
 }
 
-/**
- * Implements sampling inheritance and falls back to user-provided static rate if no parent decision is available.
- *
- * @param parentSampled: The parent transaction's sampling decision, if any.
- * @param givenRate: The rate to use if no parental decision is available.
- *
- * @returns The parent's sampling decision (if one exists), or the provided static rate
- */
-function _inheritOrUseGivenRate(parentSampled: boolean | undefined, givenRate: unknown): boolean | unknown {
-  return parentSampled !== undefined ? parentSampled : givenRate;
-}
-
 /**
  * Makes a sampling decision for the given transaction and stores it on the transaction.
  *
@@ -64,15 +52,35 @@ function sample<T extends Transaction>(hub: Hub, transaction: T, samplingContext
 
   // if the user has forced a sampling decision by passing a `sampled` value in their transaction context, go with that
   if (transaction.sampled !== undefined) {
+    transaction.tags = { ...transaction.tags, __sentry_samplingMethod: TransactionSamplingMethod.Explicit };
     return transaction;
   }
 
   // we would have bailed already if neither `tracesSampler` nor `tracesSampleRate` were defined, so one of these should
   // work; prefer the hook if so
-  const sampleRate =
-    typeof options.tracesSampler === 'function'
-      ? options.tracesSampler(samplingContext)
-      : _inheritOrUseGivenRate(samplingContext.parentSampled, options.tracesSampleRate);
+  let sampleRate;
+  if (typeof options.tracesSampler === 'function') {
+    sampleRate = options.tracesSampler(samplingContext);
+    // cast the rate to a number first in case it's a boolean
+    transaction.tags = {
+      ...transaction.tags,
+      __sentry_samplingMethod: TransactionSamplingMethod.Sampler,
+      // TODO kmclb - once tag types are loosened, don't need to cast to string here
+      __sentry_sampleRate: String(Number(sampleRate)),
+    };
+  } else if (samplingContext.parentSampled !== undefined) {
+    sampleRate = samplingContext.parentSampled;
+    transaction.tags = { ...transaction.tags, __sentry_samplingMethod: TransactionSamplingMethod.Inheritance };
+  } else {
+    sampleRate = options.tracesSampleRate;
+    // cast the rate to a number first in case it's a boolean
+    transaction.tags = {
+      ...transaction.tags,
+      __sentry_samplingMethod: TransactionSamplingMethod.Rate,
+      // TODO kmclb - once tag types are loosened, don't need to cast to string here
+      __sentry_sampleRate: String(Number(sampleRate)),
+    };
+  }
 
   // Since this is coming from the user (or from a function provided by the user), who knows what we might get. (The
   // only valid values are booleans or numbers between 0 and 1.)
@@ -88,7 +96,7 @@ function sample<T extends Transaction>(hub: Hub, transaction: T, samplingContext
       `[Tracing] Discarding transaction because ${
         typeof options.tracesSampler === 'function'
           ? 'tracesSampler returned 0 or false'
-          : 'tracesSampleRate is set to 0'
+          : 'a negative sampling decision was inherited or tracesSampleRate is set to 0'
       }`,
     );
     transaction.sampled = false;

packages/tracing/test/hub.test.ts

@@ -171,7 +171,7 @@ describe('Hub', () => {
     });
 
     describe('sample()', () => {
-      it('should not sample transactions when tracing is disabled', () => {
+      it('should set sampled = false when tracing is disabled', () => {
         // neither tracesSampleRate nor tracesSampler is defined -> tracing disabled
         const hub = new Hub(new BrowserClient({}));
         const transaction = hub.startTransaction({ name: 'dogpark' });
@@ -219,7 +219,7 @@ describe('Hub', () => {
         expect(transaction.sampled).toBe(true);
       });
 
-      it('should not try to override positive sampling decision provided in transaction context', () => {
+      it('should not try to override explicitly set positive sampling decision', () => {
         // so that the decision otherwise would be false
         const tracesSampler = jest.fn().mockReturnValue(0);
         const hub = new Hub(new BrowserClient({ tracesSampler }));
@@ -228,7 +228,7 @@ describe('Hub', () => {
         expect(transaction.sampled).toBe(true);
       });
 
-      it('should not try to override negative sampling decision provided in transaction context', () => {
+      it('should not try to override explicitly set negative sampling decision', () => {
         // so that the decision otherwise would be true
         const tracesSampler = jest.fn().mockReturnValue(1);
         const hub = new Hub(new BrowserClient({ tracesSampler }));
@@ -255,6 +255,40 @@ describe('Hub', () => {
         expect(tracesSampler).toHaveBeenCalled();
         expect(transaction.sampled).toBe(true);
       });
+
+      it('should record sampling method when sampling decision is explicitly set', () => {
+        const tracesSampler = jest.fn().mockReturnValue(0.1121);
+        const hub = new Hub(new BrowserClient({ tracesSampler }));
+        const transaction = hub.startTransaction({ name: 'dogpark', sampled: true });
+
+        expect(transaction.tags).toEqual(expect.objectContaining({ __sentry_samplingMethod: 'explicitly_set' }));
+      });
+
+      it('should record sampling method and rate when sampling decision comes from tracesSampler', () => {
+        const tracesSampler = jest.fn().mockReturnValue(0.1121);
+        const hub = new Hub(new BrowserClient({ tracesSampler }));
+        const transaction = hub.startTransaction({ name: 'dogpark' });
+
+        expect(transaction.tags).toEqual(
+          expect.objectContaining({ __sentry_samplingMethod: 'client_sampler', __sentry_sampleRate: '0.1121' }),
+        );
+      });
+
+      it('should record sampling method when sampling decision is inherited', () => {
+        const hub = new Hub(new BrowserClient({ tracesSampleRate: 0.1121 }));
+        const transaction = hub.startTransaction({ name: 'dogpark', parentSampled: true });
+
+        expect(transaction.tags).toEqual(expect.objectContaining({ __sentry_samplingMethod: 'inheritance' }));
+      });
+
+      it('should record sampling method and rate when sampling decision comes from traceSampleRate', () => {
+        const hub = new Hub(new BrowserClient({ tracesSampleRate: 0.1121 }));
+        const transaction = hub.startTransaction({ name: 'dogpark' });
+
+        expect(transaction.tags).toEqual(
+          expect.objectContaining({ __sentry_samplingMethod: 'client_rate', __sentry_sampleRate: '0.1121' }),
+        );
+      });
     });
 
     describe('isValidSampleRate()', () => {

packages/types/src/index.ts

@@ -32,6 +32,7 @@ export {
   TraceparentData,
   Transaction,
   TransactionContext,
+  TransactionSamplingMethod,
 } from './transaction';
 export { Thread } from './thread';
 export { Transport, TransportOptions, TransportClass } from './transport';

packages/types/src/transaction.ts

@@ -100,3 +100,10 @@ export interface SamplingContext extends CustomSamplingContext {
 }
 
 export type Measurements = Record<string, { value: number }>;
+
+export enum TransactionSamplingMethod {
+  Explicit = 'explicitly_set',
+  Sampler = 'client_sampler',
+  Rate = 'client_rate',
+  Inheritance = 'inheritance',
+}