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

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://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/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',
+}