diff --git a/packages/core/src/request.ts b/packages/core/src/request.ts index 5a2e966f48f8..929841447fe7 100644 --- a/packages/core/src/request.ts +++ b/packages/core/src/request.ts @@ -51,14 +51,18 @@ 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 sdkInfo = getSdkMetadataForEnvelopeHeader(api); const eventType = event.type || 'event'; const useEnvelope = eventType === 'transaction'; + const { transactionSampling, ...metadata } = event.debug_meta || {}; + const { method: samplingMethod, rate: sampleRate } = transactionSampling || {}; + if (Object.keys(metadata).length === 0) { + delete event.debug_meta; + } else { + event.debug_meta = metadata; + } + const req: SentryRequest = { body: JSON.stringify(sdkInfo ? enhanceEventWithSdkInfo(event, api.metadata.sdk) : event), type: eventType, diff --git a/packages/core/test/lib/request.test.ts b/packages/core/test/lib/request.test.ts index 4e19cf5680a8..eb99d510dba5 100644 --- a/packages/core/test/lib/request.test.ts +++ b/packages/core/test/lib/request.test.ts @@ -1,21 +1,30 @@ -import { Event, TransactionSamplingMethod } from '@sentry/types'; +import { DebugMeta, Event, SentryRequest, TransactionSamplingMethod } from '@sentry/types'; import { API } from '../../src/api'; import { eventToSentryRequest } from '../../src/request'; describe('eventToSentryRequest', () => { - let api: API; + function parseEnvelopeRequest(request: SentryRequest): any { + const [envelopeHeaderString, itemHeaderString, eventString] = request.body.split('\n'); + + return { + envelopeHeader: JSON.parse(envelopeHeaderString), + itemHeader: JSON.parse(itemHeaderString), + event: JSON.parse(eventString), + }; + } + + const api = new API('https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', { + sdk: { + integrations: ['AWSLambda'], + name: 'sentry.javascript.browser', + version: `12.31.12`, + packages: [{ name: 'npm:@sentry/browser', version: `12.31.12` }], + }, + }); let event: Event; beforeEach(() => { - api = new API('https://dogsarebadatkeepingsecrets@squirrelchasers.ingest.sentry.io/12312012', { - sdk: { - integrations: ['AWSLambda'], - name: 'sentry.javascript.browser', - version: `12.31.12`, - packages: [{ name: 'npm:@sentry/browser', version: `6.6.6` }], - }, - }); event = { contexts: { trace: { trace_id: '1231201211212012', span_id: '12261980', op: 'pageload' } }, environment: 'dogpark', @@ -28,68 +37,63 @@ describe('eventToSentryRequest', () => { }; }); - [ - { 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, 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); - }); + it(`adds transaction sampling information to item header`, () => { + event.debug_meta = { transactionSampling: { method: TransactionSamplingMethod.Rate, rate: 0.1121 } }; + + const result = eventToSentryRequest(event, api); + const envelope = parseEnvelopeRequest(result); + + expect(envelope.itemHeader).toEqual( + expect.objectContaining({ + sample_rates: [{ id: TransactionSamplingMethod.Rate, rate: 0.1121 }], + }), + ); }); - it('adds sdk info to envelope header', () => { + it('removes transaction sampling information (and only that) from debug_meta', () => { + event.debug_meta = { + transactionSampling: { method: TransactionSamplingMethod.Sampler, rate: 0.1121 }, + dog: 'Charlie', + } as DebugMeta; + const result = eventToSentryRequest(event, api); + const envelope = parseEnvelopeRequest(result); - const envelopeHeaderString = result.body.split('\n')[0]; - const parsedHeader = JSON.parse(envelopeHeaderString); + expect('transactionSampling' in envelope.event.debug_meta).toBe(false); + expect('dog' in envelope.event.debug_meta).toBe(true); + }); - expect(parsedHeader).toEqual( + it('removes debug_meta entirely if it ends up empty', () => { + event.debug_meta = { + transactionSampling: { method: TransactionSamplingMethod.Rate, rate: 0.1121 }, + } as DebugMeta; + + const result = eventToSentryRequest(event, api); + const envelope = parseEnvelopeRequest(result); + + expect('debug_meta' in envelope.event).toBe(false); + }); + + it('adds sdk info to envelope header', () => { + const result = eventToSentryRequest(event, api); + const envelope = parseEnvelopeRequest(result); + + expect(envelope.envelopeHeader).toEqual( expect.objectContaining({ sdk: { name: 'sentry.javascript.browser', version: '12.31.12' } }), ); }); it('adds sdk info to event body', () => { const result = eventToSentryRequest(event, api); + const envelope = parseEnvelopeRequest(result); - const eventString = result.body.split('\n')[2]; - const parsedEvent = JSON.parse(eventString); - - expect(parsedEvent).toEqual( + expect(envelope.event).toEqual( expect.objectContaining({ sdk: { integrations: ['AWSLambda'], name: 'sentry.javascript.browser', version: `12.31.12`, - packages: [{ name: 'npm:@sentry/browser', version: `6.6.6` }], + packages: [{ name: 'npm:@sentry/browser', version: `12.31.12` }], }, }), ); @@ -99,22 +103,21 @@ describe('eventToSentryRequest', () => { event.sdk = { integrations: ['Clojure'], name: 'foo', - packages: [{ name: 'npm:@sentry/clj', version: `6.6.6` }], + packages: [{ name: 'npm:@sentry/clj', version: `12.31.12` }], version: '1337', }; - const result = eventToSentryRequest(event, api); - const eventString = result.body.split('\n')[2]; - const parsedEvent = JSON.parse(eventString); + const result = eventToSentryRequest(event, api); + const envelope = parseEnvelopeRequest(result); - expect(parsedEvent).toEqual( + expect(envelope.event).toEqual( expect.objectContaining({ sdk: { integrations: ['Clojure', 'AWSLambda'], name: 'foo', packages: [ - { name: 'npm:@sentry/clj', version: `6.6.6` }, - { name: 'npm:@sentry/browser', version: `6.6.6` }, + { name: 'npm:@sentry/clj', version: `12.31.12` }, + { name: 'npm:@sentry/browser', version: `12.31.12` }, ], version: '1337', }, diff --git a/packages/tracing/src/hubextensions.ts b/packages/tracing/src/hubextensions.ts index b770155fe760..5baf4aae9e04 100644 --- a/packages/tracing/src/hubextensions.ts +++ b/packages/tracing/src/hubextensions.ts @@ -45,7 +45,9 @@ function sample(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 }; + transaction.setMetadata({ + transactionSampling: { method: TransactionSamplingMethod.Explicit }, + }); return transaction; } @@ -54,25 +56,27 @@ function sample(hub: Hub, transaction: T, samplingContext 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)), - }; + transaction.setMetadata({ + transactionSampling: { + method: TransactionSamplingMethod.Sampler, + // cast to number in case it's a boolean + rate: Number(sampleRate), + }, + }); } else if (samplingContext.parentSampled !== undefined) { sampleRate = samplingContext.parentSampled; - transaction.tags = { ...transaction.tags, __sentry_samplingMethod: TransactionSamplingMethod.Inheritance }; + transaction.setMetadata({ + transactionSampling: { method: 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)), - }; + transaction.setMetadata({ + transactionSampling: { + method: TransactionSamplingMethod.Rate, + // cast to number in case it's a boolean + rate: Number(sampleRate), + }, + }); } // Since this is coming from the user (or from a function provided by the user), who knows what we might get. (The diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts index 7b8b163fc377..cba9962ac8dd 100644 --- a/packages/tracing/src/transaction.ts +++ b/packages/tracing/src/transaction.ts @@ -4,9 +4,16 @@ import { dropUndefinedKeys, isInstanceOf, logger } from '@sentry/utils'; import { Span as SpanClass, SpanRecorder } from './span'; +interface TransactionMetadata { + transactionSampling?: { [key: string]: string | number }; +} + /** JSDoc */ export class Transaction extends SpanClass implements TransactionInterface { public name: string; + + private _metadata: TransactionMetadata = {}; + private _measurements: Measurements = {}; /** @@ -64,6 +71,14 @@ export class Transaction extends SpanClass implements TransactionInterface { this._measurements = { ...measurements }; } + /** + * Set metadata for this transaction. + * @hidden + */ + public setMetadata(newMetadata: TransactionMetadata): void { + this._metadata = { ...this._metadata, ...newMetadata }; + } + /** * @inheritDoc */ @@ -108,6 +123,7 @@ export class Transaction extends SpanClass implements TransactionInterface { timestamp: this.endTimestamp, transaction: this.name, type: 'transaction', + debug_meta: this._metadata, }; const hasMeasurements = Object.keys(this._measurements).length > 0; diff --git a/packages/tracing/test/hub.test.ts b/packages/tracing/test/hub.test.ts index b053512f8b26..001e4cad8a1e 100644 --- a/packages/tracing/test/hub.test.ts +++ b/packages/tracing/test/hub.test.ts @@ -2,17 +2,20 @@ import { BrowserClient } from '@sentry/browser'; import { Hub } from '@sentry/hub'; import * as hubModule from '@sentry/hub'; +import { TransactionSamplingMethod } from '@sentry/types'; import * as utilsModule from '@sentry/utils'; // for mocking import { logger } from '@sentry/utils'; import { BrowserTracing } from '../src/browser/browsertracing'; import { addExtensionMethods } from '../src/hubextensions'; +import { Transaction } from '../src/transaction'; import { extractTraceparentData, TRACEPARENT_REGEXP } from '../src/utils'; import { addDOMPropertiesToGlobal, getSymbolObjectKeyByName } from './testutils'; addExtensionMethods(); const mathRandom = jest.spyOn(Math, 'random'); +jest.spyOn(Transaction.prototype, 'setMetadata'); // we have to add things into the real global object (rather than mocking the return value of getGlobalObject) because // there are modules which call getGlobalObject as they load, which is seemingly too early for jest to intervene @@ -26,7 +29,7 @@ describe('Hub', () => { }); afterEach(() => { - jest.restoreAllMocks(); + jest.clearAllMocks(); jest.useRealTimers(); }); @@ -184,35 +187,39 @@ describe('Hub', () => { 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 }); + hub.startTransaction({ name: 'dogpark', sampled: true }); - expect(transaction.tags).toEqual(expect.objectContaining({ __sentry_samplingMethod: 'explicitly_set' })); + expect(Transaction.prototype.setMetadata).toHaveBeenCalledWith({ + transactionSampling: { method: TransactionSamplingMethod.Explicit }, + }); }); 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' }); + hub.startTransaction({ name: 'dogpark' }); - expect(transaction.tags).toEqual( - expect.objectContaining({ __sentry_samplingMethod: 'client_sampler', __sentry_sampleRate: '0.1121' }), - ); + expect(Transaction.prototype.setMetadata).toHaveBeenCalledWith({ + transactionSampling: { method: TransactionSamplingMethod.Sampler, rate: 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 }); + hub.startTransaction({ name: 'dogpark', parentSampled: true }); - expect(transaction.tags).toEqual(expect.objectContaining({ __sentry_samplingMethod: 'inheritance' })); + expect(Transaction.prototype.setMetadata).toHaveBeenCalledWith({ + transactionSampling: { method: TransactionSamplingMethod.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' }); + hub.startTransaction({ name: 'dogpark' }); - expect(transaction.tags).toEqual( - expect.objectContaining({ __sentry_samplingMethod: 'client_rate', __sentry_sampleRate: '0.1121' }), - ); + expect(Transaction.prototype.setMetadata).toHaveBeenCalledWith({ + transactionSampling: { method: TransactionSamplingMethod.Rate, rate: 0.1121 }, + }); }); }); diff --git a/packages/types/src/debugMeta.ts b/packages/types/src/debugMeta.ts index 91f21201349e..0c00304dc636 100644 --- a/packages/types/src/debugMeta.ts +++ b/packages/types/src/debugMeta.ts @@ -3,6 +3,7 @@ **/ export interface DebugMeta { images?: Array; + transactionSampling?: { rate?: number; method?: string }; } /**