diff --git a/MIGRATION.md b/MIGRATION.md index a5c4677541fc..72dc963c0719 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -64,6 +64,8 @@ In v8, the Span class is heavily reworked. The following properties & methods ar * `span.toTraceparent()`: use `spanToTraceHeader(span)` util instead. * `span.getTraceContext()`: Use `spanToTraceContext(span)` utility function instead. * `span.sampled`: Use `span.isRecording()` instead. +* `transaction.setMetadata(metadata)`: Use `spanSetMetadata(span, metadata)` utility function instead. +* `transaction.metadata`: Use `spanGetMetadata(span)` utility function instead. ## Deprecate `pushScope` & `popScope` in favor of `withScope` diff --git a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts index 061a7815ca75..feaf137b3713 100644 --- a/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts +++ b/dev-packages/node-integration-tests/suites/express/sentry-trace/baggage-transaction-name/server.ts @@ -1,4 +1,5 @@ import http from 'http'; +import { spanSetMetadata } from '@sentry/core'; import * as Sentry from '@sentry/node'; import * as Tracing from '@sentry/tracing'; import cors from 'cors'; @@ -32,7 +33,7 @@ app.get('/test/express', (_req, res) => { const transaction = Sentry.getCurrentHub().getScope().getTransaction(); if (transaction) { transaction.traceId = '86f39e84263a4de99c326acab3bfe3bd'; - transaction.setMetadata({ source: 'route' }); + spanSetMetadata(transaction, { source: 'route' }); } const headers = http.get('http://somewhere.not.sentry/').getHeaders(); diff --git a/packages/angular-ivy/ng-package.json b/packages/angular-ivy/ng-package.json index b50faf694df3..38d9d7f5ac68 100644 --- a/packages/angular-ivy/ng-package.json +++ b/packages/angular-ivy/ng-package.json @@ -5,9 +5,10 @@ "entryFile": "src/index.ts", "umdModuleIds": { "@sentry/browser": "Sentry", - "@sentry/utils": "Sentry.util" + "@sentry/utils": "Sentry.util", + "@sentry/core": "Sentry.core" } }, - "allowedNonPeerDependencies": ["@sentry/browser", "@sentry/utils", "@sentry/types", "tslib"], + "allowedNonPeerDependencies": ["@sentry/browser", "@sentry/core", "@sentry/utils", "@sentry/types", "tslib"], "assets": ["README.md", "LICENSE"] } diff --git a/packages/angular-ivy/package.json b/packages/angular-ivy/package.json index 9d9ccd91e4db..14b10ae27954 100644 --- a/packages/angular-ivy/package.json +++ b/packages/angular-ivy/package.json @@ -22,6 +22,7 @@ }, "dependencies": { "@sentry/browser": "7.92.0", + "@sentry/core": "7.92.0", "@sentry/types": "7.92.0", "@sentry/utils": "7.92.0", "tslib": "^2.4.1" diff --git a/packages/angular/ng-package.json b/packages/angular/ng-package.json index 88df70c1c7bd..28794322dd0a 100644 --- a/packages/angular/ng-package.json +++ b/packages/angular/ng-package.json @@ -5,9 +5,10 @@ "entryFile": "src/index.ts", "umdModuleIds": { "@sentry/browser": "Sentry", - "@sentry/utils": "Sentry.util" + "@sentry/utils": "Sentry.util", + "@sentry/core": "Sentry.core" } }, - "whitelistedNonPeerDependencies": ["@sentry/browser", "@sentry/utils", "@sentry/types", "tslib"], + "whitelistedNonPeerDependencies": ["@sentry/browser", "@sentry/core", "@sentry/utils", "@sentry/types", "tslib"], "assets": ["README.md", "LICENSE"] } diff --git a/packages/angular/package.json b/packages/angular/package.json index c1cfc9765071..55998227fc7c 100644 --- a/packages/angular/package.json +++ b/packages/angular/package.json @@ -22,6 +22,7 @@ }, "dependencies": { "@sentry/browser": "7.92.0", + "@sentry/core": "7.92.0", "@sentry/types": "7.92.0", "@sentry/utils": "7.92.0", "tslib": "^2.4.1" diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts index e65ecd84d8df..28a45943e2e7 100644 --- a/packages/angular/src/tracing.ts +++ b/packages/angular/src/tracing.ts @@ -8,6 +8,7 @@ import type { ActivatedRouteSnapshot, Event, RouterState } from '@angular/router import { NavigationCancel, NavigationError, Router } from '@angular/router'; import { NavigationEnd, NavigationStart, ResolveEnd } from '@angular/router'; import { WINDOW, getCurrentScope } from '@sentry/browser'; +import { spanSetMetadata } from '@sentry/core'; import type { Span, Transaction, TransactionContext } from '@sentry/types'; import { logger, stripUrlQueryAndFragment, timestampInSeconds } from '@sentry/utils'; import type { Observable } from 'rxjs'; @@ -119,7 +120,7 @@ export class TraceService implements OnDestroy { // TODO (v8 / #5416): revisit the source condition. Do we want to make the parameterized route the default? if (transaction && transaction.metadata.source === 'url') { transaction.updateName(route); - transaction.setMetadata({ source: 'route' }); + spanSetMetadata(transaction, { source: 'route' }); } }), ); diff --git a/packages/angular/test/tracing.test.ts b/packages/angular/test/tracing.test.ts index 695e3d7af564..bdf739ece747 100644 --- a/packages/angular/test/tracing.test.ts +++ b/packages/angular/test/tracing.test.ts @@ -1,6 +1,7 @@ import { Component } from '@angular/core'; import type { ActivatedRouteSnapshot, CanActivate, RouterStateSnapshot } from '@angular/router'; +import * as SentryCore from '@sentry/core'; import { TraceClassDecorator, TraceDirective, TraceMethodDecorator, instrumentAngularRouting } from '../src'; import { getParameterizedRouteFromSnapshot } from '../src/tracing'; import { AppComponent, TestEnv } from './utils/index'; @@ -11,7 +12,6 @@ const defaultStartTransaction = (ctx: any) => { transaction = { ...ctx, updateName: jest.fn(name => (transaction.name = name)), - setMetadata: jest.fn(), }; return transaction; @@ -31,9 +31,12 @@ jest.mock('@sentry/browser', () => { }; }); +const mockSpanSetMetadata = jest.spyOn(SentryCore, 'spanSetMetadata'); + describe('Angular Tracing', () => { beforeEach(() => { transaction = undefined; + mockSpanSetMetadata.mockClear(); }); describe('instrumentAngularRouting', () => { @@ -329,7 +332,7 @@ describe('Angular Tracing', () => { metadata: { source: 'url' }, }); expect(transaction.updateName).toHaveBeenCalledWith(result); - expect(transaction.setMetadata).toHaveBeenCalledWith({ source: 'route' }); + expect(mockSpanSetMetadata).toHaveBeenCalledWith(expect.anything(), { source: 'route' }); env.destroy(); }); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 2c423f0744c3..404abc10fc6a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -72,7 +72,7 @@ export { createCheckInEnvelope } from './checkin'; export { hasTracingEnabled } from './utils/hasTracingEnabled'; export { isSentryRequestUrl } from './utils/isSentryRequestUrl'; export { handleCallbackErrors } from './utils/handleCallbackErrors'; -export { spanToTraceHeader } from './utils/spanUtils'; +export { spanToTraceHeader, spanSetMetadata, spanGetMetadata } from './utils/spanUtils'; export { DEFAULT_ENVIRONMENT } from './constants'; export { ModuleMetadata } from './integrations/metadata'; export { RequestData } from './integrations/requestdata'; diff --git a/packages/core/src/tracing/sampling.ts b/packages/core/src/tracing/sampling.ts index 739303e1fe8c..a459a3f629fb 100644 --- a/packages/core/src/tracing/sampling.ts +++ b/packages/core/src/tracing/sampling.ts @@ -3,6 +3,7 @@ import { isNaN, logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; import { hasTracingEnabled } from '../utils/hasTracingEnabled'; +import { spanSetMetadata } from '../utils/spanUtils'; import type { Transaction } from './transaction'; /** @@ -29,7 +30,7 @@ export function sampleTransaction( // if the user has forced a sampling decision by passing a `sampled` value in their transaction context, go with that // eslint-disable-next-line deprecation/deprecation if (transaction.sampled !== undefined) { - transaction.setMetadata({ + spanSetMetadata(transaction, { // eslint-disable-next-line deprecation/deprecation sampleRate: Number(transaction.sampled), }); @@ -41,20 +42,20 @@ export function sampleTransaction( let sampleRate; if (typeof options.tracesSampler === 'function') { sampleRate = options.tracesSampler(samplingContext); - transaction.setMetadata({ + spanSetMetadata(transaction, { sampleRate: Number(sampleRate), }); } else if (samplingContext.parentSampled !== undefined) { sampleRate = samplingContext.parentSampled; } else if (typeof options.tracesSampleRate !== 'undefined') { sampleRate = options.tracesSampleRate; - transaction.setMetadata({ - sampleRate: Number(sampleRate), + spanSetMetadata(transaction, { + sampleRate, }); } else { // When `enableTracing === true`, we use a sample rate of 100% sampleRate = 1; - transaction.setMetadata({ + spanSetMetadata(transaction, { sampleRate, }); } diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index 7142ec3419e7..bc3f5d1a2ae7 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -15,14 +15,12 @@ import { dropUndefinedKeys, logger } from '@sentry/utils'; import { DEBUG_BUILD } from '../debug-build'; import type { Hub } from '../hub'; import { getCurrentHub } from '../hub'; -import { spanTimeInputToSeconds, spanToTraceContext } from '../utils/spanUtils'; +import { spanGetMetadata, spanSetMetadata, spanTimeInputToSeconds, spanToTraceContext } from '../utils/spanUtils'; import { getDynamicSamplingContextFromClient } from './dynamicSamplingContext'; import { Span as SpanClass, SpanRecorder } from './span'; /** JSDoc */ export class Transaction extends SpanClass implements TransactionInterface { - public metadata: TransactionMetadata; - /** * The reference to the current hub. */ @@ -58,11 +56,11 @@ export class Transaction extends SpanClass implements TransactionInterface { this._name = transactionContext.name || ''; - this.metadata = { + spanSetMetadata(this, { source: 'custom', ...transactionContext.metadata, spanMetadata: {}, - }; + }); this._trimEnd = transactionContext.trimEnd; @@ -71,13 +69,18 @@ export class Transaction extends SpanClass implements TransactionInterface { // If Dynamic Sampling Context is provided during the creation of the transaction, we freeze it as it usually means // there is incoming Dynamic Sampling Context. (Either through an incoming request, a baggage meta-tag, or other means) - const incomingDynamicSamplingContext = this.metadata.dynamicSamplingContext; + const incomingDynamicSamplingContext = ( + spanGetMetadata(this) as { dynamicSamplingContext: DynamicSamplingContext | undefined } + ).dynamicSamplingContext; if (incomingDynamicSamplingContext) { // We shallow copy this in case anything writes to the original reference of the passed in `dynamicSamplingContext` this._frozenDynamicSamplingContext = { ...incomingDynamicSamplingContext }; } } + // This sadly conflicts with the getter/setter ordering :( + /* eslint-disable @typescript-eslint/member-ordering */ + /** Getter for `name` property */ public get name(): string { return this._name; @@ -91,6 +94,24 @@ export class Transaction extends SpanClass implements TransactionInterface { this.setName(newName); } + /** + * Get the metadata for this transaction. + * @deprecated Use `spanGetMetadata(transaction)` instead. + */ + public get metadata(): TransactionMetadata { + return spanGetMetadata(this) as TransactionMetadata; + } + + /** + * Update the metadata for this transaction. + * @deprecated Use `spanGetMetadata(transaction)` instead. + */ + public set metadata(metadata: TransactionMetadata) { + spanSetMetadata(this, metadata); + } + + /* eslint-enable @typescript-eslint/member-ordering */ + /** * Setter for `name` property, which also sets `source` on the metadata. * @@ -98,7 +119,7 @@ export class Transaction extends SpanClass implements TransactionInterface { */ public setName(name: string, source: TransactionMetadata['source'] = 'custom'): void { this._name = name; - this.metadata.source = source; + spanSetMetadata(this, { source }); } /** @inheritdoc */ @@ -138,10 +159,11 @@ export class Transaction extends SpanClass implements TransactionInterface { } /** - * @inheritDoc + * Set metadata for this transaction. + * @deprecated Use `spanSetMetadata(span, metadata)` instead. */ public setMetadata(newMetadata: Partial): void { - this.metadata = { ...this.metadata, ...newMetadata }; + spanSetMetadata(this, newMetadata); } /** @@ -202,13 +224,15 @@ export class Transaction extends SpanClass implements TransactionInterface { const scope = hub.getScope(); const dsc = getDynamicSamplingContextFromClient(this.traceId, client, scope); - const maybeSampleRate = this.metadata.sampleRate; + const metadata = spanGetMetadata(this) as TransactionMetadata; + + const maybeSampleRate = metadata.sampleRate; if (maybeSampleRate !== undefined) { dsc.sample_rate = `${maybeSampleRate}`; } // We don't want to have a transaction name in the DSC if the source is "url" because URLs might contain PII - const source = this.metadata.source; + const source = metadata.source; if (source && source !== 'url') { dsc.transaction = this.name; } @@ -277,7 +301,7 @@ export class Transaction extends SpanClass implements TransactionInterface { }).endTimestamp; } - const metadata = this.metadata; + const metadata = spanGetMetadata(this) as TransactionMetadata; const transaction: TransactionEvent = { contexts: { diff --git a/packages/core/src/utils/spanUtils.ts b/packages/core/src/utils/spanUtils.ts index 9c4dcff17b62..2f4b5c4f6648 100644 --- a/packages/core/src/utils/spanUtils.ts +++ b/packages/core/src/utils/spanUtils.ts @@ -1,6 +1,8 @@ -import type { Span, SpanTimeInput, TraceContext } from '@sentry/types'; +import type { Span, SpanMetadata, SpanTimeInput, TraceContext } from '@sentry/types'; import { dropUndefinedKeys, generateSentryTraceHeader, timestampInSeconds } from '@sentry/utils'; +const SPAN_METADATA = new WeakMap(); + /** * Convert a span to a trace context, which can be sent as the `trace` context in an event. */ @@ -54,3 +56,19 @@ function ensureTimestampInSeconds(timestamp: number): number { const isMs = timestamp > 9999999999; return isMs ? timestamp / 1000 : timestamp; } + +/** + * Get the metadata for a span. + */ +export function spanGetMetadata(span: Span): SpanMetadata { + return SPAN_METADATA.get(span) || {}; +} + +/** + * Update metadata for a span. + * This will merge the given new metadata with existing metadata. + */ +export function spanSetMetadata(span: Span, newMetadata: SpanMetadata): void { + const existingMetadata = spanGetMetadata(span); + SPAN_METADATA.set(span, { ...existingMetadata, ...newMetadata }); +} diff --git a/packages/core/test/lib/tracing/transaction.test.ts b/packages/core/test/lib/tracing/transaction.test.ts index 3be3d7dccfcc..2411211b15dc 100644 --- a/packages/core/test/lib/tracing/transaction.test.ts +++ b/packages/core/test/lib/tracing/transaction.test.ts @@ -1,4 +1,5 @@ import { Transaction } from '../../../src'; +import { spanSetMetadata } from '../../../src/utils/spanUtils'; describe('transaction', () => { it('works with name', () => { @@ -8,37 +9,38 @@ describe('transaction', () => { it('allows to update the name via setter', () => { const transaction = new Transaction({ name: 'span name' }); - transaction.setMetadata({ source: 'route' }); + spanSetMetadata(transaction, { source: 'route' }); expect(transaction.name).toEqual('span name'); transaction.name = 'new name'; expect(transaction.name).toEqual('new name'); + // eslint-disable-next-line deprecation/deprecation expect(transaction.metadata.source).toEqual('custom'); }); it('allows to update the name via setName', () => { const transaction = new Transaction({ name: 'span name' }); - transaction.setMetadata({ source: 'route' }); + spanSetMetadata(transaction, { source: 'route' }); expect(transaction.name).toEqual('span name'); - transaction.setMetadata({ source: 'route' }); - // eslint-disable-next-line deprecation/deprecation transaction.setName('new name'); expect(transaction.name).toEqual('new name'); + // eslint-disable-next-line deprecation/deprecation expect(transaction.metadata.source).toEqual('custom'); }); it('allows to update the name via updateName', () => { const transaction = new Transaction({ name: 'span name' }); - transaction.setMetadata({ source: 'route' }); + spanSetMetadata(transaction, { source: 'route' }); expect(transaction.name).toEqual('span name'); transaction.updateName('new name'); expect(transaction.name).toEqual('new name'); + // eslint-disable-next-line deprecation/deprecation expect(transaction.metadata.source).toEqual('route'); }); }); diff --git a/packages/core/test/lib/utils/spanUtils.test.ts b/packages/core/test/lib/utils/spanUtils.test.ts index a6d84cf31166..595ae406663f 100644 --- a/packages/core/test/lib/utils/spanUtils.test.ts +++ b/packages/core/test/lib/utils/spanUtils.test.ts @@ -1,6 +1,12 @@ import { TRACEPARENT_REGEXP, timestampInSeconds } from '@sentry/utils'; -import { Span, spanToTraceHeader } from '../../../src'; -import { spanTimeInputToSeconds } from '../../../src/utils/spanUtils'; +import { Span } from '../../../src/tracing/span'; +import { Transaction } from '../../../src/tracing/transaction'; +import { + spanGetMetadata, + spanSetMetadata, + spanTimeInputToSeconds, + spanToTraceHeader, +} from '../../../src/utils/spanUtils'; describe('spanToTraceHeader', () => { test('simple', () => { @@ -46,3 +52,46 @@ describe('spanTimeInputToSeconds', () => { expect(spanTimeInputToSeconds(timestamp)).toEqual(seconds + 0.000009); }); }); + +describe('span metadata', () => { + it('allows to get empty metadata', () => { + const span = new Span(); + expect(spanGetMetadata(span)).toEqual({}); + }); + + it('allows to set & get metadata', () => { + const span = new Span(); + expect(spanGetMetadata(span)).toEqual({}); + + spanSetMetadata(span, { one: 'one', two: 2 }); + expect(spanGetMetadata(span)).toEqual({ one: 'one', two: 2 }); + + // partially overwrite + spanSetMetadata(span, { one: 'two' }); + expect(spanGetMetadata(span)).toEqual({ one: 'two', two: 2 }); + }); + + it('interacts with transaction.metadata correctly', () => { + const transaction = new Transaction({ name: 'test' }); + expect(spanGetMetadata(transaction)).toEqual({ + source: 'custom', + spanMetadata: {}, + }); + // eslint-disable-next-line deprecation/deprecation + expect(transaction.metadata).toEqual({ + source: 'custom', + spanMetadata: {}, + }); + + spanSetMetadata(transaction, { one: 'one', two: 2 }); + expect(spanGetMetadata(transaction)).toEqual({ source: 'custom', spanMetadata: {}, one: 'one', two: 2 }); + // eslint-disable-next-line deprecation/deprecation + expect(transaction.metadata).toEqual({ source: 'custom', spanMetadata: {}, one: 'one', two: 2 }); + + // eslint-disable-next-line deprecation/deprecation + transaction.setMetadata({ one: 'two' }); + expect(spanGetMetadata(transaction)).toEqual({ source: 'custom', spanMetadata: {}, one: 'two', two: 2 }); + // eslint-disable-next-line deprecation/deprecation + expect(transaction.metadata).toEqual({ source: 'custom', spanMetadata: {}, one: 'two', two: 2 }); + }); +}); diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 6d4c6f5a4494..28552c22b8e3 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -8,6 +8,7 @@ import { getCurrentScope, hasTracingEnabled, runWithAsyncContext, + spanSetMetadata, startTransaction, withScope, } from '@sentry/core'; @@ -332,7 +333,7 @@ export function trpcMiddleware(options: SentryTrpcMiddlewareOptions = {}) { if (sentryTransaction) { sentryTransaction.updateName(`trpc/${path}`); - sentryTransaction.setMetadata({ source: 'route' }); + spanSetMetadata(sentryTransaction, { source: 'route' }); sentryTransaction.op = 'rpc.server'; const trpcContext: Record = { diff --git a/packages/opentelemetry-node/src/spanprocessor.ts b/packages/opentelemetry-node/src/spanprocessor.ts index 5b1a1684c9cf..98d01923ca64 100644 --- a/packages/opentelemetry-node/src/spanprocessor.ts +++ b/packages/opentelemetry-node/src/spanprocessor.ts @@ -2,7 +2,14 @@ import type { Context } from '@opentelemetry/api'; import { SpanKind, context, trace } from '@opentelemetry/api'; import { suppressTracing } from '@opentelemetry/core'; import type { Span as OtelSpan, SpanProcessor as OtelSpanProcessor } from '@opentelemetry/sdk-trace-base'; -import { Transaction, addEventProcessor, addTracingExtensions, getClient, getCurrentHub } from '@sentry/core'; +import { + Transaction, + addEventProcessor, + addTracingExtensions, + getClient, + getCurrentHub, + spanSetMetadata, +} from '@sentry/core'; import type { DynamicSamplingContext, Span as SentrySpan, TraceparentData, TransactionContext } from '@sentry/types'; import { logger } from '@sentry/utils'; @@ -218,7 +225,7 @@ function updateTransactionWithOtelData(transaction: Transaction, otelSpan: OtelS transaction.op = op; transaction.updateName(description); - transaction.setMetadata({ source }); + spanSetMetadata(transaction, { source }); } function convertOtelTimeToSeconds([seconds, nano]: [number, number]): number { diff --git a/packages/opentelemetry-node/src/utils/spanData.ts b/packages/opentelemetry-node/src/utils/spanData.ts index 1cdbacf74955..5d3a0c1202cf 100644 --- a/packages/opentelemetry-node/src/utils/spanData.ts +++ b/packages/opentelemetry-node/src/utils/spanData.ts @@ -1,5 +1,5 @@ /* eslint-disable deprecation/deprecation */ -import { Transaction } from '@sentry/core'; +import { Transaction, spanSetMetadata } from '@sentry/core'; import type { Context, SpanOrigin } from '@sentry/types'; import { getSentrySpan } from './spanMap'; @@ -51,7 +51,7 @@ export function addOtelSpanData( if (sentrySpan instanceof Transaction) { if (metadata) { - sentrySpan.setMetadata(metadata); + spanSetMetadata(sentrySpan, metadata as Record); } if (contexts) { diff --git a/packages/opentelemetry/test/custom/transaction.test.ts b/packages/opentelemetry/test/custom/transaction.test.ts index 043d76235140..770758e752e3 100644 --- a/packages/opentelemetry/test/custom/transaction.test.ts +++ b/packages/opentelemetry/test/custom/transaction.test.ts @@ -152,6 +152,7 @@ describe('startTranscation', () => { expect(transaction.sampled).toBe(undefined); expect(transaction.spanRecorder).toBeDefined(); expect(transaction.spanRecorder?.spans).toHaveLength(1); + // eslint-disable-next-line deprecation/deprecation expect(transaction.metadata).toEqual({ source: 'custom', spanMetadata: {}, @@ -181,6 +182,7 @@ describe('startTranscation', () => { expect(transaction).toBeInstanceOf(OpenTelemetryTransaction); + // eslint-disable-next-line deprecation/deprecation expect(transaction.metadata).toEqual({ source: 'custom', spanMetadata: {}, diff --git a/packages/react/package.json b/packages/react/package.json index 437642015f78..faec21438a92 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -30,6 +30,7 @@ }, "dependencies": { "@sentry/browser": "7.92.0", + "@sentry/core": "7.92.0", "@sentry/types": "7.92.0", "@sentry/utils": "7.92.0", "hoist-non-react-statics": "^3.3.2" diff --git a/packages/react/src/reactrouter.tsx b/packages/react/src/reactrouter.tsx index 8a42c5ff96f1..9fa5bb1a2f46 100644 --- a/packages/react/src/reactrouter.tsx +++ b/packages/react/src/reactrouter.tsx @@ -1,4 +1,5 @@ import { WINDOW } from '@sentry/browser'; +import { spanSetMetadata } from '@sentry/core'; import type { Transaction, TransactionSource } from '@sentry/types'; import hoistNonReactStatics from 'hoist-non-react-statics'; import * as React from 'react'; @@ -166,7 +167,7 @@ export function withSentryRouting

, R extends React const WrappedRoute: React.FC

= (props: P) => { if (activeTransaction && props && props.computedMatch && props.computedMatch.isExact) { activeTransaction.updateName(props.computedMatch.path); - activeTransaction.setMetadata({ source: 'route' }); + spanSetMetadata(activeTransaction, { source: 'route' }); } // @ts-expect-error Setting more specific React Component typing for `R` generic above diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index 920a6b4f8a0d..40549a9f529c 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -2,6 +2,7 @@ // https://gist.github.com/wontondon/e8c4bdf2888875e4c755712e99279536 import { WINDOW } from '@sentry/browser'; +import { spanSetMetadata } from '@sentry/core'; import type { Transaction, TransactionContext, TransactionSource } from '@sentry/types'; import { getNumberOfUrlSegments, logger } from '@sentry/utils'; import hoistNonReactStatics from 'hoist-non-react-statics'; @@ -136,7 +137,7 @@ function updatePageloadTransaction( if (activeTransaction && branches) { const [name, source] = getNormalizedName(routes, location, branches, basename); activeTransaction.updateName(name); - activeTransaction.setMetadata({ source }); + spanSetMetadata(activeTransaction, { source }); } } diff --git a/packages/react/test/reactrouterv4.test.tsx b/packages/react/test/reactrouterv4.test.tsx index 2b06b3a196a5..48c5fe02264e 100644 --- a/packages/react/test/reactrouterv4.test.tsx +++ b/packages/react/test/reactrouterv4.test.tsx @@ -1,3 +1,4 @@ +import * as SentryCore from '@sentry/core'; import { act, render } from '@testing-library/react'; import { createMemoryHistory } from 'history-4'; // biome-ignore lint/nursery/noUnusedImports: Need React import for JSX @@ -7,12 +8,14 @@ import { Route, Router, Switch, matchPath } from 'react-router-4'; import { reactRouterV4Instrumentation, withSentryRouting } from '../src'; import type { RouteConfig } from '../src/reactrouter'; +const mockSpanSetMetadata = jest.spyOn(SentryCore, 'spanSetMetadata'); + describe('React Router v4', () => { function createInstrumentation(_opts?: { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; routes?: RouteConfig[]; - }): [jest.Mock, any, { mockUpdateName: jest.Mock; mockFinish: jest.Mock; mockSetMetadata: jest.Mock }] { + }): [jest.Mock, any, { mockUpdateName: jest.Mock; mockFinish: jest.Mock }] { const options = { matchPath: _opts && _opts.routes !== undefined ? matchPath : undefined, routes: undefined, @@ -23,18 +26,19 @@ describe('React Router v4', () => { const history = createMemoryHistory(); const mockFinish = jest.fn(); const mockUpdateName = jest.fn(); - const mockSetMetadata = jest.fn(); - const mockStartTransaction = jest - .fn() - .mockReturnValue({ updateName: mockUpdateName, end: mockFinish, setMetadata: mockSetMetadata }); + const mockStartTransaction = jest.fn().mockReturnValue({ updateName: mockUpdateName, end: mockFinish }); reactRouterV4Instrumentation(history, options.routes, options.matchPath)( mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange, ); - return [mockStartTransaction, history, { mockUpdateName, mockFinish, mockSetMetadata }]; + return [mockStartTransaction, history, { mockUpdateName, mockFinish }]; } + beforeEach(function () { + mockSpanSetMetadata.mockClear(); + }); + it('starts a pageload transaction when instrumentation is started', () => { const [mockStartTransaction] = createInstrumentation(); expect(mockStartTransaction).toHaveBeenCalledTimes(1); @@ -169,7 +173,7 @@ describe('React Router v4', () => { }); it('normalizes transaction name with custom Route', () => { - const [mockStartTransaction, history, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); + const [mockStartTransaction, history, { mockUpdateName }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -196,11 +200,11 @@ describe('React Router v4', () => { }); expect(mockUpdateName).toHaveBeenCalledTimes(2); expect(mockUpdateName).toHaveBeenLastCalledWith('/users/:userid'); - expect(mockSetMetadata).toHaveBeenCalledWith({ source: 'route' }); + expect(mockSpanSetMetadata).toHaveBeenCalledWith(expect.anything(), { source: 'route' }); }); it('normalizes nested transaction names with custom Route', () => { - const [mockStartTransaction, history, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); + const [mockStartTransaction, history, { mockUpdateName }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -227,7 +231,7 @@ describe('React Router v4', () => { }); expect(mockUpdateName).toHaveBeenCalledTimes(2); expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid'); - expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); + expect(mockSpanSetMetadata).toHaveBeenLastCalledWith(expect.anything(), { source: 'route' }); act(() => { history.push('/organizations/543'); @@ -244,7 +248,7 @@ describe('React Router v4', () => { }); expect(mockUpdateName).toHaveBeenCalledTimes(3); expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid'); - expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); + expect(mockSpanSetMetadata).toHaveBeenLastCalledWith(expect.anything(), { source: 'route' }); }); it('matches with route object', () => { diff --git a/packages/react/test/reactrouterv5.test.tsx b/packages/react/test/reactrouterv5.test.tsx index fba57df9a5f8..c991e8458473 100644 --- a/packages/react/test/reactrouterv5.test.tsx +++ b/packages/react/test/reactrouterv5.test.tsx @@ -4,15 +4,18 @@ import { createMemoryHistory } from 'history-4'; import * as React from 'react'; import { Route, Router, Switch, matchPath } from 'react-router-5'; +import * as SentryCore from '@sentry/core'; import { reactRouterV5Instrumentation, withSentryRouting } from '../src'; import type { RouteConfig } from '../src/reactrouter'; +const mockSpanSetMetadata = jest.spyOn(SentryCore, 'spanSetMetadata'); + describe('React Router v5', () => { function createInstrumentation(_opts?: { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; routes?: RouteConfig[]; - }): [jest.Mock, any, { mockUpdateName: jest.Mock; mockFinish: jest.Mock; mockSetMetadata: jest.Mock }] { + }): [jest.Mock, any, { mockUpdateName: jest.Mock; mockFinish: jest.Mock }] { const options = { matchPath: _opts && _opts.routes !== undefined ? matchPath : undefined, routes: undefined, @@ -23,18 +26,20 @@ describe('React Router v5', () => { const history = createMemoryHistory(); const mockFinish = jest.fn(); const mockUpdateName = jest.fn(); - const mockSetMetadata = jest.fn(); - const mockStartTransaction = jest - .fn() - .mockReturnValue({ updateName: mockUpdateName, end: mockFinish, setMetadata: mockSetMetadata }); + + const mockStartTransaction = jest.fn().mockReturnValue({ updateName: mockUpdateName, end: mockFinish }); reactRouterV5Instrumentation(history, options.routes, options.matchPath)( mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange, ); - return [mockStartTransaction, history, { mockUpdateName, mockFinish, mockSetMetadata }]; + return [mockStartTransaction, history, { mockUpdateName, mockFinish }]; } + beforeEach(() => { + mockSpanSetMetadata.mockClear(); + }); + it('starts a pageload transaction when instrumentation is started', () => { const [mockStartTransaction] = createInstrumentation(); expect(mockStartTransaction).toHaveBeenCalledTimes(1); @@ -169,7 +174,7 @@ describe('React Router v5', () => { }); it('normalizes transaction name with custom Route', () => { - const [mockStartTransaction, history, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); + const [mockStartTransaction, history, { mockUpdateName }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -196,11 +201,11 @@ describe('React Router v5', () => { }); expect(mockUpdateName).toHaveBeenCalledTimes(2); expect(mockUpdateName).toHaveBeenLastCalledWith('/users/:userid'); - expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); + expect(mockSpanSetMetadata).toHaveBeenLastCalledWith(expect.anything(), { source: 'route' }); }); it('normalizes nested transaction names with custom Route', () => { - const [mockStartTransaction, history, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); + const [mockStartTransaction, history, { mockUpdateName }] = createInstrumentation(); const SentryRoute = withSentryRouting(Route); const { getByText } = render( @@ -228,7 +233,7 @@ describe('React Router v5', () => { }); expect(mockUpdateName).toHaveBeenCalledTimes(2); expect(mockUpdateName).toHaveBeenLastCalledWith('/organizations/:orgid/v1/:teamid'); - expect(mockSetMetadata).toHaveBeenLastCalledWith({ source: 'route' }); + expect(mockSpanSetMetadata).toHaveBeenLastCalledWith(expect.anything(), { source: 'route' }); act(() => { history.push('/organizations/543'); diff --git a/packages/react/test/reactrouterv6.4.test.tsx b/packages/react/test/reactrouterv6.4.test.tsx index a89bb50e1f82..2ffbbdd145b0 100644 --- a/packages/react/test/reactrouterv6.4.test.tsx +++ b/packages/react/test/reactrouterv6.4.test.tsx @@ -12,9 +12,12 @@ import { useNavigationType, } from 'react-router-6.4'; +import * as SentryCore from '@sentry/core'; import { reactRouterV6Instrumentation, wrapCreateBrowserRouter } from '../src'; import type { CreateRouterFunction } from '../src/types'; +const mockSpanSetMetadata = jest.spyOn(SentryCore, 'spanSetMetadata'); + beforeAll(() => { // @ts-expect-error need to override global Request because it's not in the jest environment (even with an // `@jest-environment jsdom` directive, for some reason) @@ -25,7 +28,7 @@ describe('React Router v6.4', () => { function createInstrumentation(_opts?: { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; - }): [jest.Mock, { mockUpdateName: jest.Mock; mockFinish: jest.Mock; mockSetMetadata: jest.Mock }] { + }): [jest.Mock, { mockUpdateName: jest.Mock; mockFinish: jest.Mock }] { const options = { matchPath: _opts ? matchPath : undefined, startTransactionOnLocationChange: true, @@ -34,10 +37,7 @@ describe('React Router v6.4', () => { }; const mockFinish = jest.fn(); const mockUpdateName = jest.fn(); - const mockSetMetadata = jest.fn(); - const mockStartTransaction = jest - .fn() - .mockReturnValue({ updateName: mockUpdateName, end: mockFinish, setMetadata: mockSetMetadata }); + const mockStartTransaction = jest.fn().mockReturnValue({ updateName: mockUpdateName, end: mockFinish }); reactRouterV6Instrumentation( React.useEffect, @@ -46,9 +46,13 @@ describe('React Router v6.4', () => { createRoutesFromChildren, matchRoutes, )(mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange); - return [mockStartTransaction, { mockUpdateName, mockFinish, mockSetMetadata }]; + return [mockStartTransaction, { mockUpdateName, mockFinish }]; } + beforeEach(() => { + mockSpanSetMetadata.mockClear(); + }); + describe('wrapCreateBrowserRouter', () => { it('starts a pageload transaction', () => { const [mockStartTransaction] = createInstrumentation(); @@ -246,7 +250,7 @@ describe('React Router v6.4', () => { }); it('updates pageload transaction to a parameterized route', () => { - const [mockStartTransaction, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); + const [mockStartTransaction, { mockUpdateName }] = createInstrumentation(); const sentryCreateBrowserRouter = wrapCreateBrowserRouter(createMemoryRouter as CreateRouterFunction); const router = sentryCreateBrowserRouter( @@ -272,7 +276,7 @@ describe('React Router v6.4', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(1); expect(mockUpdateName).toHaveBeenLastCalledWith('/about/:page'); - expect(mockSetMetadata).toHaveBeenCalledWith({ source: 'route' }); + expect(mockSpanSetMetadata).toHaveBeenCalledWith(expect.anything(), { source: 'route' }); }); it('works with `basename` option', () => { diff --git a/packages/react/test/reactrouterv6.test.tsx b/packages/react/test/reactrouterv6.test.tsx index 965ce134bb74..850e9109f88e 100644 --- a/packages/react/test/reactrouterv6.test.tsx +++ b/packages/react/test/reactrouterv6.test.tsx @@ -14,14 +14,17 @@ import { useRoutes, } from 'react-router-6'; +import * as SentryCore from '@sentry/core'; import { reactRouterV6Instrumentation } from '../src'; import { withSentryReactRouterV6Routing, wrapUseRoutes } from '../src/reactrouterv6'; +const mockSpanSetMetadata = jest.spyOn(SentryCore, 'spanSetMetadata'); + describe('React Router v6', () => { function createInstrumentation(_opts?: { startTransactionOnPageLoad?: boolean; startTransactionOnLocationChange?: boolean; - }): [jest.Mock, { mockUpdateName: jest.Mock; mockFinish: jest.Mock; mockSetMetadata: jest.Mock }] { + }): [jest.Mock, { mockUpdateName: jest.Mock; mockFinish: jest.Mock }] { const options = { matchPath: _opts ? matchPath : undefined, startTransactionOnLocationChange: true, @@ -30,10 +33,7 @@ describe('React Router v6', () => { }; const mockFinish = jest.fn(); const mockUpdateName = jest.fn(); - const mockSetMetadata = jest.fn(); - const mockStartTransaction = jest - .fn() - .mockReturnValue({ updateName: mockUpdateName, end: mockFinish, setMetadata: mockSetMetadata }); + const mockStartTransaction = jest.fn().mockReturnValue({ updateName: mockUpdateName, end: mockFinish }); reactRouterV6Instrumentation( React.useEffect, @@ -42,7 +42,7 @@ describe('React Router v6', () => { createRoutesFromChildren, matchRoutes, )(mockStartTransaction, options.startTransactionOnPageLoad, options.startTransactionOnLocationChange); - return [mockStartTransaction, { mockUpdateName, mockFinish, mockSetMetadata }]; + return [mockStartTransaction, { mockUpdateName, mockFinish }]; } describe('withSentryReactRouterV6Routing', () => { @@ -545,7 +545,7 @@ describe('React Router v6', () => { }); it('does not add double slashes to URLS', () => { - const [mockStartTransaction, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); + const [mockStartTransaction, { mockUpdateName }] = createInstrumentation(); const wrappedUseRoutes = wrapUseRoutes(useRoutes); const Routes = () => @@ -588,11 +588,11 @@ describe('React Router v6', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(1); // should be /tests not //tests expect(mockUpdateName).toHaveBeenLastCalledWith('/tests'); - expect(mockSetMetadata).toHaveBeenCalledWith({ source: 'route' }); + expect(mockSpanSetMetadata).toHaveBeenCalledWith(expect.anything(), { source: 'route' }); }); it('handles wildcard routes properly', () => { - const [mockStartTransaction, { mockUpdateName, mockSetMetadata }] = createInstrumentation(); + const [mockStartTransaction, { mockUpdateName }] = createInstrumentation(); const wrappedUseRoutes = wrapUseRoutes(useRoutes); const Routes = () => @@ -634,7 +634,7 @@ describe('React Router v6', () => { expect(mockStartTransaction).toHaveBeenCalledTimes(1); expect(mockUpdateName).toHaveBeenLastCalledWith('/tests/:testId/*'); - expect(mockSetMetadata).toHaveBeenCalledWith({ source: 'route' }); + expect(mockSpanSetMetadata).toHaveBeenCalledWith(expect.anything(), { source: 'route' }); }); }); }); diff --git a/packages/remix/src/client/performance.tsx b/packages/remix/src/client/performance.tsx index 597f6daae48f..d69a38da362b 100644 --- a/packages/remix/src/client/performance.tsx +++ b/packages/remix/src/client/performance.tsx @@ -1,3 +1,4 @@ +import { spanSetMetadata } from '@sentry/core'; import type { ErrorBoundaryProps } from '@sentry/react'; import { WINDOW, withErrorBoundary } from '@sentry/react'; import type { Transaction, TransactionContext } from '@sentry/types'; @@ -126,7 +127,7 @@ export function withSentry

, R extends React.Co _useEffect(() => { if (activeTransaction && matches && matches.length) { activeTransaction.updateName(matches[matches.length - 1].id); - activeTransaction.setMetadata({ source: 'route' }); + spanSetMetadata(activeTransaction, { source: 'route' }); } isBaseLocation = true; diff --git a/packages/sveltekit/src/client/router.ts b/packages/sveltekit/src/client/router.ts index 0d327335326b..07c1041a7f0c 100644 --- a/packages/sveltekit/src/client/router.ts +++ b/packages/sveltekit/src/client/router.ts @@ -1,4 +1,4 @@ -import { getActiveTransaction } from '@sentry/core'; +import { getActiveTransaction, spanSetMetadata } from '@sentry/core'; import { WINDOW } from '@sentry/svelte'; import type { Span, Transaction, TransactionContext } from '@sentry/types'; @@ -57,7 +57,7 @@ function instrumentPageload(startTransactionFn: (context: TransactionContext) => if (pageloadTransaction && routeId) { pageloadTransaction.updateName(routeId); - pageloadTransaction.setMetadata({ source: 'route' }); + spanSetMetadata(pageloadTransaction, { source: 'route' }); } }); } diff --git a/packages/sveltekit/test/client/router.test.ts b/packages/sveltekit/test/client/router.test.ts index 648e5344e6c8..e28f83c69260 100644 --- a/packages/sveltekit/test/client/router.test.ts +++ b/packages/sveltekit/test/client/router.test.ts @@ -1,3 +1,4 @@ +import * as SentryCore from '@sentry/core'; /* eslint-disable @typescript-eslint/unbound-method */ import type { Transaction } from '@sentry/types'; import { writable } from 'svelte/store'; @@ -27,7 +28,6 @@ describe('sveltekitRoutingInstrumentation', () => { returnedTransaction = { ...txnCtx, updateName: vi.fn(), - setMetadata: vi.fn(), startChild: vi.fn().mockImplementation(ctx => { return { ...mockedRoutingSpan, ...ctx }; }), @@ -50,6 +50,8 @@ describe('sveltekitRoutingInstrumentation', () => { it("starts a pageload transaction when it's called with default params", () => { svelteKitRoutingInstrumentation(mockedStartTransaction); + const spanSetMetadataSpy = vi.spyOn(SentryCore, 'spanSetMetadata'); + expect(mockedStartTransaction).toHaveBeenCalledTimes(1); expect(mockedStartTransaction).toHaveBeenCalledWith({ name: '/', @@ -71,7 +73,7 @@ describe('sveltekitRoutingInstrumentation', () => { // This should update the transaction name with the parameterized route: expect(returnedTransaction?.updateName).toHaveBeenCalledTimes(1); expect(returnedTransaction?.updateName).toHaveBeenCalledWith('testRoute'); - expect(returnedTransaction?.setMetadata).toHaveBeenCalledWith({ source: 'route' }); + expect(spanSetMetadataSpy).toHaveBeenCalledWith(expect.anything(), { source: 'route' }); }); it("doesn't start a pageload transaction if `startTransactionOnPageLoad` is false", () => { diff --git a/packages/tracing-internal/src/node/integrations/express.ts b/packages/tracing-internal/src/node/integrations/express.ts index 7754fff3ad5b..8a1895a015f9 100644 --- a/packages/tracing-internal/src/node/integrations/express.ts +++ b/packages/tracing-internal/src/node/integrations/express.ts @@ -1,4 +1,5 @@ /* eslint-disable max-lines */ +import { spanSetMetadata } from '@sentry/core'; import type { Hub, Integration, PolymorphicRequest, Transaction } from '@sentry/types'; import { GLOBAL_OBJ, @@ -378,7 +379,7 @@ function instrumentRouter(appOrRouter: ExpressRouter): void { const [name, source] = extractPathForTransaction(req, { path: true, method: true, customRoute: finalRoute }); transaction.updateName(name); - transaction.setMetadata({ source }); + spanSetMetadata(transaction, { source }); } } diff --git a/packages/tracing/test/hub.test.ts b/packages/tracing/test/hub.test.ts index f7cf93cc5e32..7db31830c5c0 100644 --- a/packages/tracing/test/hub.test.ts +++ b/packages/tracing/test/hub.test.ts @@ -1,11 +1,11 @@ /* eslint-disable deprecation/deprecation */ /* eslint-disable @typescript-eslint/unbound-method */ import { BrowserClient } from '@sentry/browser'; -import { Hub, makeMain } from '@sentry/core'; +import { Hub, makeMain, spanGetMetadata } from '@sentry/core'; import * as utilsModule from '@sentry/utils'; // for mocking import { logger } from '@sentry/utils'; -import { BrowserTracing, TRACEPARENT_REGEXP, Transaction, addExtensionMethods, extractTraceparentData } from '../src'; +import { BrowserTracing, TRACEPARENT_REGEXP, addExtensionMethods, extractTraceparentData } from '../src'; import { addDOMPropertiesToGlobal, getDefaultBrowserClientOptions, @@ -16,7 +16,6 @@ import { addExtensionMethods(); const mathRandom = jest.spyOn(Math, 'random'); -jest.spyOn(Transaction.prototype, 'setMetadata'); jest.spyOn(logger, 'warn'); jest.spyOn(logger, 'log'); jest.spyOn(logger, 'error'); @@ -284,11 +283,13 @@ describe('Hub', () => { const options = getDefaultBrowserClientOptions({ tracesSampler }); const hub = new Hub(new BrowserClient(options)); makeMain(hub); - hub.startTransaction({ name: 'dogpark', sampled: true }); + const transaction = hub.startTransaction({ name: 'dogpark', sampled: true }); - expect(Transaction.prototype.setMetadata).toHaveBeenCalledWith({ - sampleRate: 1.0, - }); + expect(spanGetMetadata(transaction)).toEqual( + expect.objectContaining({ + sampleRate: 1.0, + }), + ); }); it('should record sampling method and rate when sampling decision comes from tracesSampler', () => { @@ -296,31 +297,35 @@ describe('Hub', () => { const options = getDefaultBrowserClientOptions({ tracesSampler }); const hub = new Hub(new BrowserClient(options)); makeMain(hub); - hub.startTransaction({ name: 'dogpark' }); + const transaction = hub.startTransaction({ name: 'dogpark' }); - expect(Transaction.prototype.setMetadata).toHaveBeenCalledWith({ - sampleRate: 0.1121, - }); + expect(spanGetMetadata(transaction)).toEqual( + expect.objectContaining({ + sampleRate: 0.1121, + }), + ); }); it('should record sampling method when sampling decision is inherited', () => { const options = getDefaultBrowserClientOptions({ tracesSampleRate: 0.1121 }); const hub = new Hub(new BrowserClient(options)); makeMain(hub); - hub.startTransaction({ name: 'dogpark', parentSampled: true }); + const transaction = hub.startTransaction({ name: 'dogpark', parentSampled: true }); - expect(Transaction.prototype.setMetadata).toHaveBeenCalledTimes(0); + expect(spanGetMetadata(transaction).sampleRate).toEqual(undefined); }); it('should record sampling method and rate when sampling decision comes from traceSampleRate', () => { const options = getDefaultBrowserClientOptions({ tracesSampleRate: 0.1121 }); const hub = new Hub(new BrowserClient(options)); makeMain(hub); - hub.startTransaction({ name: 'dogpark' }); + const transaction = hub.startTransaction({ name: 'dogpark' }); - expect(Transaction.prototype.setMetadata).toHaveBeenCalledWith({ - sampleRate: 0.1121, - }); + expect(spanGetMetadata(transaction)).toEqual( + expect.objectContaining({ + sampleRate: 0.1121, + }), + ); }); }); diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index 011af5ba10d5..066e228dc8e4 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -1,6 +1,6 @@ /* eslint-disable deprecation/deprecation */ import { BrowserClient } from '@sentry/browser'; -import { Hub, Scope, makeMain } from '@sentry/core'; +import { Hub, Scope, makeMain, spanSetMetadata } from '@sentry/core'; import type { BaseTransportOptions, ClientOptions, TransactionSource } from '@sentry/types'; import { Span, TRACEPARENT_REGEXP, Transaction } from '../src'; @@ -645,9 +645,7 @@ describe('Span', () => { test('is included when transaction metadata is set', () => { const spy = jest.spyOn(hub as any, 'captureEvent') as any; const transaction = hub.startTransaction({ name: 'test', sampled: true }); - transaction.setMetadata({ - source: 'url', - }); + spanSetMetadata(transaction, { source: 'url' }); expect(spy).toHaveBeenCalledTimes(0); transaction.end(); diff --git a/packages/tracing/test/transaction.test.ts b/packages/tracing/test/transaction.test.ts index 12c6c799883a..9047d2ebe4f1 100644 --- a/packages/tracing/test/transaction.test.ts +++ b/packages/tracing/test/transaction.test.ts @@ -1,5 +1,6 @@ /* eslint-disable deprecation/deprecation */ import { BrowserClient, Hub } from '@sentry/browser'; +import { spanSetMetadata } from '@sentry/core'; import { Transaction, addExtensionMethods } from '../src'; import { getDefaultBrowserClientOptions } from './testutils'; @@ -65,7 +66,7 @@ describe('`Transaction` class', () => { describe('`updateName` method', () => { it('does not change the source', () => { const transaction = new Transaction({ name: 'dogpark' }); - transaction.setMetadata({ source: 'route' }); + spanSetMetadata(transaction, { source: 'route' }); transaction.updateName('ballpit'); expect(transaction.name).toEqual('ballpit'); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 655962f592fc..21c5aee065e8 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -89,7 +89,15 @@ export type { // eslint-disable-next-line deprecation/deprecation export type { Severity, SeverityLevel } from './severity'; -export type { Span, SpanContext, SpanOrigin, SpanAttributeValue, SpanAttributes, SpanTimeInput } from './span'; +export type { + Span, + SpanContext, + SpanOrigin, + SpanAttributeValue, + SpanAttributes, + SpanTimeInput, + SpanMetadata, +} from './span'; export type { StackFrame } from './stackframe'; export type { Stacktrace, StackParser, StackLineParser, StackLineParserFn } from './stacktrace'; export type { TextEncoderInternal } from './textencoder'; diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts index 4ad0e5aa1110..32daad425601 100644 --- a/packages/types/src/span.ts +++ b/packages/types/src/span.ts @@ -28,6 +28,8 @@ export type SpanAttributes = Record; /** This type is aligned with the OpenTelemetry TimeInput type. */ export type SpanTimeInput = HrTime | number | Date; +export type SpanMetadata = Record; + /** Interface holding all properties that can be set on a Span on creation. */ export interface SpanContext { /** diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index a4bee40983a5..44e2c2c2d666 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -4,7 +4,7 @@ import type { Instrumenter } from './instrumenter'; import type { MeasurementUnit } from './measurement'; import type { ExtractedNodeRequestData, Primitive, WorkerLocation } from './misc'; import type { PolymorphicRequest } from './polymorphics'; -import type { Span, SpanAttributes, SpanContext } from './span'; +import type { Span, SpanAttributes, SpanContext, SpanMetadata } from './span'; /** * Interface holding Transaction-specific properties @@ -116,7 +116,7 @@ export interface Transaction extends TransactionContext, Omit): void; @@ -159,7 +159,7 @@ export interface SamplingContext extends CustomSamplingContext { request?: ExtractedNodeRequestData; } -export interface TransactionMetadata { +export interface TransactionMetadata extends SpanMetadata { /** The sample rate used when sampling this transaction */ sampleRate?: number; diff --git a/packages/utils/src/requestdata.ts b/packages/utils/src/requestdata.ts index 0249b7a2b481..4d0bfcedd8d2 100644 --- a/packages/utils/src/requestdata.ts +++ b/packages/utils/src/requestdata.ts @@ -76,6 +76,7 @@ export function addRequestDataToTransaction( // Attempt to grab a parameterized route off of the request const [name, source] = extractPathForTransaction(req, { path: true, method: true }); transaction.updateName(name); + // eslint-disable-next-line deprecation/deprecation transaction.setMetadata({ source }); } transaction.setData('url', req.originalUrl || req.url); diff --git a/packages/vue/src/router.ts b/packages/vue/src/router.ts index 70117e960fe2..9d28a4990e6e 100644 --- a/packages/vue/src/router.ts +++ b/packages/vue/src/router.ts @@ -1,4 +1,5 @@ import { WINDOW, captureException } from '@sentry/browser'; +import { spanSetMetadata } from '@sentry/core'; import type { Transaction, TransactionContext, TransactionSource } from '@sentry/types'; import { getActiveTransaction } from './tracing'; @@ -112,7 +113,7 @@ export function vueRouterInstrumentation( if (pageloadTransaction) { if (pageloadTransaction.metadata.source !== 'custom') { pageloadTransaction.updateName(transactionName); - pageloadTransaction.setMetadata({ source: transactionSource }); + spanSetMetadata(pageloadTransaction, { source: transactionSource }); } pageloadTransaction.setData('params', data.params); pageloadTransaction.setData('query', data.query); diff --git a/packages/vue/test/router.test.ts b/packages/vue/test/router.test.ts index 2e937e02f154..c9ca05b4c0b7 100644 --- a/packages/vue/test/router.test.ts +++ b/packages/vue/test/router.test.ts @@ -1,4 +1,5 @@ import * as SentryBrowser from '@sentry/browser'; +import * as SentryCore from '@sentry/core'; import type { Transaction } from '@sentry/types'; import { vueRouterInstrumentation } from '../src'; @@ -128,7 +129,6 @@ describe('vueRouterInstrumentation()', () => { const mockedTxn = { updateName: jest.fn(), setData: jest.fn(), - setMetadata: jest.fn(), metadata: {}, }; const customMockStartTxn = { ...mockStartTransaction }.mockImplementation(_ => { @@ -136,6 +136,8 @@ describe('vueRouterInstrumentation()', () => { }); jest.spyOn(vueTracing, 'getActiveTransaction').mockImplementation(() => mockedTxn as unknown as Transaction); + const mockSpanSetMetadata = jest.spyOn(SentryCore, 'spanSetMetadata'); + // create instrumentation const instrument = vueRouterInstrumentation(mockVueRouter); @@ -165,7 +167,7 @@ describe('vueRouterInstrumentation()', () => { expect(mockVueRouter.beforeEach).toHaveBeenCalledTimes(1); expect(mockedTxn.updateName).toHaveBeenCalledWith(transactionName); - expect(mockedTxn.setMetadata).toHaveBeenCalledWith({ source: transactionSource }); + expect(mockSpanSetMetadata).toHaveBeenCalledWith(expect.anything(), { source: transactionSource }); expect(mockedTxn.setData).toHaveBeenNthCalledWith(1, 'params', to.params); expect(mockedTxn.setData).toHaveBeenNthCalledWith(2, 'query', to.query);