From 45e844d1044d998dd59ef6b2e91f56cd9d0b0321 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 8 Jan 2024 12:53:45 +0100 Subject: [PATCH 01/10] feat(core): Deprecate `Transaction.getDynamicSamplingContext` in favour of `getDynamicSamplingContextFromSpan` --- MIGRATION.md | 1 + .../rollup-utils/plugins/bundlePlugins.mjs | 3 + packages/astro/src/server/meta.ts | 3 +- packages/core/src/server-runtime-client.ts | 8 ++- .../src/tracing/dynamicSamplingContext.ts | 64 ++++++++++++++++++- packages/core/src/tracing/index.ts | 2 +- packages/core/src/tracing/transaction.ts | 42 ++---------- .../core/src/utils/applyScopeDataToEvent.ts | 3 +- .../wrapAppGetInitialPropsWithSentry.ts | 10 ++- .../wrapErrorGetInitialPropsWithSentry.ts | 10 ++- .../common/wrapGetInitialPropsWithSentry.ts | 10 ++- .../wrapGetServerSidePropsWithSentry.ts | 10 ++- packages/node/src/integrations/hapi/index.ts | 3 +- packages/node/src/integrations/http.ts | 3 +- .../node/src/integrations/undici/index.ts | 3 +- packages/opentelemetry-node/src/propagator.ts | 4 +- packages/remix/src/utils/instrumentServer.ts | 3 +- packages/sveltekit/src/server/handle.ts | 4 +- .../tracing-internal/src/browser/request.ts | 3 +- packages/tracing-internal/src/common/fetch.ts | 3 +- packages/types/src/transaction.ts | 6 +- 21 files changed, 135 insertions(+), 63 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index c7587fc10c76..dfdf95c6549f 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -98,6 +98,7 @@ In v8, the Span class is heavily reworked. The following properties & methods ar * `span.traceId`: Use `span.spanContext().traceId` instead. * `span.name`: Use `spanToJSON(span).description` instead. * `span.description`: Use `spanToJSON(span).description` instead. +* `span.getDynamicSamplingContext`: Use `getDynamicSamplingContextFromSpan` utility function instead. * `transaction.setMetadata()`: Use attributes instead, or set data on the scope. * `transaction.metadata`: Use attributes instead, or set data on the scope. diff --git a/dev-packages/rollup-utils/plugins/bundlePlugins.mjs b/dev-packages/rollup-utils/plugins/bundlePlugins.mjs index 66f8e8c78228..9aef97dc828f 100644 --- a/dev-packages/rollup-utils/plugins/bundlePlugins.mjs +++ b/dev-packages/rollup-utils/plugins/bundlePlugins.mjs @@ -135,6 +135,9 @@ export function makeTerserPlugin() { // These are used by instrument.ts in utils for identifying HTML elements & events '_sentryCaptured', '_sentryId', + // For v7 backwards-compatibility we need to access txn._frozenDynamicSamplingContext + // TODO (v8): Remove this reserved word + '_frozenDynamicSamplingContext', ], }, }, diff --git a/packages/astro/src/server/meta.ts b/packages/astro/src/server/meta.ts index bdc0d89d4f80..a76c09c598d9 100644 --- a/packages/astro/src/server/meta.ts +++ b/packages/astro/src/server/meta.ts @@ -1,4 +1,5 @@ import { getDynamicSamplingContextFromClient, spanToTraceHeader } from '@sentry/core'; +import { getDynamicSamplingContextFromSpan } from '@sentry/core/src/tracing/dynamicSamplingContext'; import type { Client, Scope, Span } from '@sentry/types'; import { TRACEPARENT_REGEXP, @@ -33,7 +34,7 @@ export function getTracingMetaTags( const sentryTrace = span ? spanToTraceHeader(span) : generateSentryTraceHeader(traceId, undefined, sampled); const dynamicSamplingContext = transaction - ? transaction.getDynamicSamplingContext() + ? getDynamicSamplingContextFromSpan(transaction) : dsc ? dsc : client diff --git a/packages/core/src/server-runtime-client.ts b/packages/core/src/server-runtime-client.ts index b21e162c5b1c..00f23be2dc0a 100644 --- a/packages/core/src/server-runtime-client.ts +++ b/packages/core/src/server-runtime-client.ts @@ -21,7 +21,11 @@ import { getClient } from './exports'; import { MetricsAggregator } from './metrics/aggregator'; import type { Scope } from './scope'; import { SessionFlusher } from './sessionflusher'; -import { addTracingExtensions, getDynamicSamplingContextFromClient } from './tracing'; +import { + addTracingExtensions, + getDynamicSamplingContextFromClient, + getDynamicSamplingContextFromSpan, +} from './tracing'; import { spanToTraceContext } from './utils/spanUtils'; export interface ServerRuntimeClientOptions extends ClientOptions { @@ -258,7 +262,7 @@ export class ServerRuntimeClient< // eslint-disable-next-line deprecation/deprecation const span = scope.getSpan(); if (span) { - const samplingContext = span.transaction ? span.transaction.getDynamicSamplingContext() : undefined; + const samplingContext = span.transaction ? getDynamicSamplingContextFromSpan(span) : undefined; return [samplingContext, spanToTraceContext(span)]; } diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index f8e8cd107c87..783a1e5ecf14 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -1,17 +1,19 @@ -import type { Client, DynamicSamplingContext, Scope } from '@sentry/types'; +import type { Client, DynamicSamplingContext, Scope, Span } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; import { DEFAULT_ENVIRONMENT } from '../constants'; +import { getClient, getCurrentScope } from '../exports'; /** * Creates a dynamic sampling context from a client. * - * Dispatchs the `createDsc` lifecycle hook as a side effect. + * Dispatches the `createDsc` lifecycle hook as a side effect. */ export function getDynamicSamplingContextFromClient( trace_id: string, client: Client, scope?: Scope, + emitHook: boolean = true, ): DynamicSamplingContext { const options = client.getOptions(); @@ -26,6 +28,64 @@ export function getDynamicSamplingContextFromClient( trace_id, }) as DynamicSamplingContext; + if (emitHook) { + client.emit && client.emit('createDsc', dsc); + } + + return dsc; +} + +/** + * A Span with a frozen dynamic sampling context. + */ +type TransactionWithV7FrozenDsc = Span & { _frozenDynamicSamplingContext?: DynamicSamplingContext }; + +/** + * Creates a dynamic sampling context from a span (and client and scope) + * + * @param span the span from which a few values like the root span name and sample rate are extracted. + * + * @returns a dynamic sampling context + */ +export function getDynamicSamplingContextFromSpan(span: Span): Readonly> { + // TODO (v8): Remove v7FrozenDsc as a Transaction will no longer have _frozenDynamicSamplingContext + // For now we need to avoid breaking users who directly created a txn with a DSC, where this field is still set. + // @see Transaction class constructor + const v7FrozenDsc = (span as TransactionWithV7FrozenDsc)._frozenDynamicSamplingContext; + if (v7FrozenDsc) { + return v7FrozenDsc; + } + + const client = getClient(); + if (!client) { + return {}; + } + + // passing emit=false here to only emit later once the DSC is actually populated + const dsc = getDynamicSamplingContextFromClient(span.traceId, client, getCurrentScope(), false); + const txn = span.transaction; + if (!txn) { + return dsc; + } + + const maybeSampleRate = txn.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 = txn.metadata.source; + if (source && source !== 'url') { + dsc.transaction = txn.name; + } + + // TODO: Switch to `spanIsSampled` once we have it + // eslint-disable-next-line deprecation/deprecation + if (txn.sampled !== undefined) { + // eslint-disable-next-line deprecation/deprecation + dsc.sampled = String(txn.sampled); + } + client.emit && client.emit('createDsc', dsc); return dsc; diff --git a/packages/core/src/tracing/index.ts b/packages/core/src/tracing/index.ts index 759a42cbdfe0..ecdc5f595095 100644 --- a/packages/core/src/tracing/index.ts +++ b/packages/core/src/tracing/index.ts @@ -19,5 +19,5 @@ export { startSpanManual, continueTrace, } from './trace'; -export { getDynamicSamplingContextFromClient } from './dynamicSamplingContext'; +export { getDynamicSamplingContextFromClient, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext'; export { setMeasurement } from './measurement'; diff --git a/packages/core/src/tracing/transaction.ts b/packages/core/src/tracing/transaction.ts index 7d13038de6e0..5a26e282064f 100644 --- a/packages/core/src/tracing/transaction.ts +++ b/packages/core/src/tracing/transaction.ts @@ -17,7 +17,7 @@ import type { Hub } from '../hub'; import { getCurrentHub } from '../hub'; import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import { spanTimeInputToSeconds, spanToTraceContext } from '../utils/spanUtils'; -import { getDynamicSamplingContextFromClient } from './dynamicSamplingContext'; +import { getDynamicSamplingContextFromSpan } from './dynamicSamplingContext'; import { Span as SpanClass, SpanRecorder } from './span'; /** JSDoc */ @@ -227,43 +227,11 @@ export class Transaction extends SpanClass implements TransactionInterface { * @inheritdoc * * @experimental + * + * @deprecated Use top-level `getDynamicSamplingContextFromSpan` instead. */ public getDynamicSamplingContext(): Readonly> { - if (this._frozenDynamicSamplingContext) { - return this._frozenDynamicSamplingContext; - } - - const hub = this._hub || getCurrentHub(); - const client = hub.getClient(); - - if (!client) return {}; - - const { _traceId: traceId, _sampled: sampled } = this; - - const scope = hub.getScope(); - const dsc = getDynamicSamplingContextFromClient(traceId, client, scope); - - // eslint-disable-next-line deprecation/deprecation - const maybeSampleRate = this.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 - // eslint-disable-next-line deprecation/deprecation - const source = this.metadata.source; - if (source && source !== 'url') { - dsc.transaction = this._name; - } - - if (sampled !== undefined) { - dsc.sampled = String(sampled); - } - - // Uncomment if we want to make DSC immutable - // this._frozenDynamicSamplingContext = dsc; - - return dsc; + return getDynamicSamplingContextFromSpan(this); } /** @@ -340,7 +308,7 @@ export class Transaction extends SpanClass implements TransactionInterface { type: 'transaction', sdkProcessingMetadata: { ...metadata, - dynamicSamplingContext: this.getDynamicSamplingContext(), + dynamicSamplingContext: getDynamicSamplingContextFromSpan(this), }, ...(source && { transaction_info: { diff --git a/packages/core/src/utils/applyScopeDataToEvent.ts b/packages/core/src/utils/applyScopeDataToEvent.ts index ef9796680cb9..f834776ac9ba 100644 --- a/packages/core/src/utils/applyScopeDataToEvent.ts +++ b/packages/core/src/utils/applyScopeDataToEvent.ts @@ -1,5 +1,6 @@ import type { Breadcrumb, Event, PropagationContext, ScopeData, Span } from '@sentry/types'; import { arrayify } from '@sentry/utils'; +import { getDynamicSamplingContextFromSpan } from '../tracing/dynamicSamplingContext'; import { spanToJSON, spanToTraceContext } from './spanUtils'; /** @@ -176,7 +177,7 @@ function applySpanToEvent(event: Event, span: Span): void { const transaction = span.transaction; if (transaction) { event.sdkProcessingMetadata = { - dynamicSamplingContext: transaction.getDynamicSamplingContext(), + dynamicSamplingContext: getDynamicSamplingContextFromSpan(span), ...event.sdkProcessingMetadata, }; const transactionName = spanToJSON(transaction).description; diff --git a/packages/nextjs/src/common/wrapAppGetInitialPropsWithSentry.ts b/packages/nextjs/src/common/wrapAppGetInitialPropsWithSentry.ts index c965cacb3c32..df18b2ad952d 100644 --- a/packages/nextjs/src/common/wrapAppGetInitialPropsWithSentry.ts +++ b/packages/nextjs/src/common/wrapAppGetInitialPropsWithSentry.ts @@ -1,4 +1,10 @@ -import { addTracingExtensions, getClient, getCurrentScope, spanToTraceHeader } from '@sentry/core'; +import { + addTracingExtensions, + getClient, + getCurrentScope, + getDynamicSamplingContextFromSpan, + spanToTraceHeader, +} from '@sentry/core'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import type App from 'next/app'; @@ -66,7 +72,7 @@ export function wrapAppGetInitialPropsWithSentry(origAppGetInitialProps: AppGetI if (requestTransaction) { appGetInitialProps.pageProps._sentryTraceData = spanToTraceHeader(requestTransaction); - const dynamicSamplingContext = requestTransaction.getDynamicSamplingContext(); + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(requestTransaction); appGetInitialProps.pageProps._sentryBaggage = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); } diff --git a/packages/nextjs/src/common/wrapErrorGetInitialPropsWithSentry.ts b/packages/nextjs/src/common/wrapErrorGetInitialPropsWithSentry.ts index 413c350ef14f..44a171d8e6d5 100644 --- a/packages/nextjs/src/common/wrapErrorGetInitialPropsWithSentry.ts +++ b/packages/nextjs/src/common/wrapErrorGetInitialPropsWithSentry.ts @@ -1,4 +1,10 @@ -import { addTracingExtensions, getClient, getCurrentScope, spanToTraceHeader } from '@sentry/core'; +import { + addTracingExtensions, + getClient, + getCurrentScope, + getDynamicSamplingContextFromSpan, + spanToTraceHeader, +} from '@sentry/core'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import type { NextPageContext } from 'next'; import type { ErrorProps } from 'next/error'; @@ -58,7 +64,7 @@ export function wrapErrorGetInitialPropsWithSentry( if (requestTransaction) { errorGetInitialProps._sentryTraceData = spanToTraceHeader(requestTransaction); - const dynamicSamplingContext = requestTransaction.getDynamicSamplingContext(); + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(requestTransaction); errorGetInitialProps._sentryBaggage = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); } diff --git a/packages/nextjs/src/common/wrapGetInitialPropsWithSentry.ts b/packages/nextjs/src/common/wrapGetInitialPropsWithSentry.ts index 8263f00c3dcb..1a6743765cd6 100644 --- a/packages/nextjs/src/common/wrapGetInitialPropsWithSentry.ts +++ b/packages/nextjs/src/common/wrapGetInitialPropsWithSentry.ts @@ -1,4 +1,10 @@ -import { addTracingExtensions, getClient, getCurrentScope, spanToTraceHeader } from '@sentry/core'; +import { + addTracingExtensions, + getClient, + getCurrentScope, + getDynamicSamplingContextFromSpan, + spanToTraceHeader, +} from '@sentry/core'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import type { NextPage } from 'next'; @@ -54,7 +60,7 @@ export function wrapGetInitialPropsWithSentry(origGetInitialProps: GetInitialPro if (requestTransaction) { initialProps._sentryTraceData = spanToTraceHeader(requestTransaction); - const dynamicSamplingContext = requestTransaction.getDynamicSamplingContext(); + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(requestTransaction); initialProps._sentryBaggage = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); } diff --git a/packages/nextjs/src/common/wrapGetServerSidePropsWithSentry.ts b/packages/nextjs/src/common/wrapGetServerSidePropsWithSentry.ts index fff92d31f49b..691570f87683 100644 --- a/packages/nextjs/src/common/wrapGetServerSidePropsWithSentry.ts +++ b/packages/nextjs/src/common/wrapGetServerSidePropsWithSentry.ts @@ -1,4 +1,10 @@ -import { addTracingExtensions, getClient, getCurrentScope, spanToTraceHeader } from '@sentry/core'; +import { + addTracingExtensions, + getClient, + getCurrentScope, + getDynamicSamplingContextFromSpan, + spanToTraceHeader, +} from '@sentry/core'; import { dynamicSamplingContextToSentryBaggageHeader } from '@sentry/utils'; import type { GetServerSideProps } from 'next'; @@ -51,7 +57,7 @@ export function wrapGetServerSidePropsWithSentry( if (requestTransaction) { serverSideProps.props._sentryTraceData = spanToTraceHeader(requestTransaction); - const dynamicSamplingContext = requestTransaction.getDynamicSamplingContext(); + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(requestTransaction); serverSideProps.props._sentryBaggage = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); } } diff --git a/packages/node/src/integrations/hapi/index.ts b/packages/node/src/integrations/hapi/index.ts index 3776cf449ce8..42335f7c4ce5 100644 --- a/packages/node/src/integrations/hapi/index.ts +++ b/packages/node/src/integrations/hapi/index.ts @@ -5,6 +5,7 @@ import { convertIntegrationFnToClass, getActiveTransaction, getCurrentScope, + getDynamicSamplingContextFromSpan, spanToTraceHeader, startTransaction, } from '@sentry/core'; @@ -101,7 +102,7 @@ export const hapiTracingPlugin = { response.header('sentry-trace', spanToTraceHeader(transaction)); const dynamicSamplingContext = dynamicSamplingContextToSentryBaggageHeader( - transaction.getDynamicSamplingContext(), + getDynamicSamplingContextFromSpan(transaction), ); if (dynamicSamplingContext) { diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index 658f914a8155..7c2627b17123 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -8,6 +8,7 @@ import { getCurrentHub, getCurrentScope, getDynamicSamplingContextFromClient, + getDynamicSamplingContextFromSpan, isSentryRequestUrl, spanToJSON, spanToTraceHeader, @@ -271,7 +272,7 @@ function _createWrappedRequestMethodFactory( if (shouldAttachTraceData(rawRequestUrl)) { if (requestSpan) { const sentryTraceHeader = spanToTraceHeader(requestSpan); - const dynamicSamplingContext = requestSpan?.transaction?.getDynamicSamplingContext(); + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(requestSpan); addHeadersToRequestOptions(requestOptions, requestUrl, sentryTraceHeader, dynamicSamplingContext); } else { const client = getClient(); diff --git a/packages/node/src/integrations/undici/index.ts b/packages/node/src/integrations/undici/index.ts index 25ef141d6b83..f4aec53aa30f 100644 --- a/packages/node/src/integrations/undici/index.ts +++ b/packages/node/src/integrations/undici/index.ts @@ -5,6 +5,7 @@ import { getCurrentHub, getCurrentScope, getDynamicSamplingContextFromClient, + getDynamicSamplingContextFromSpan, isSentryRequestUrl, spanToTraceHeader, } from '@sentry/core'; @@ -181,7 +182,7 @@ export class Undici implements Integration { if (shouldAttachTraceData(stringUrl)) { if (span) { - const dynamicSamplingContext = span?.transaction?.getDynamicSamplingContext(); + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(span); const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); setHeadersOnRequest(request, spanToTraceHeader(span), sentryBaggageHeader); diff --git a/packages/opentelemetry-node/src/propagator.ts b/packages/opentelemetry-node/src/propagator.ts index ce0f295ce720..9052ede7e966 100644 --- a/packages/opentelemetry-node/src/propagator.ts +++ b/packages/opentelemetry-node/src/propagator.ts @@ -1,7 +1,7 @@ import type { Baggage, Context, TextMapGetter, TextMapSetter } from '@opentelemetry/api'; import { TraceFlags, isSpanContextValid, propagation, trace } from '@opentelemetry/api'; import { W3CBaggagePropagator, isTracingSuppressed } from '@opentelemetry/core'; -import { spanToTraceHeader } from '@sentry/core'; +import { getDynamicSamplingContextFromSpan, spanToTraceHeader } from '@sentry/core'; import { SENTRY_BAGGAGE_KEY_PREFIX, baggageHeaderToDynamicSamplingContext, @@ -36,7 +36,7 @@ export class SentryPropagator extends W3CBaggagePropagator { setter.set(carrier, SENTRY_TRACE_HEADER, spanToTraceHeader(span)); if (span.transaction) { - const dynamicSamplingContext = span.transaction.getDynamicSamplingContext(); + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(span); baggage = Object.entries(dynamicSamplingContext).reduce((b, [dscKey, dscValue]) => { if (dscValue) { return b.setEntry(`${SENTRY_BAGGAGE_KEY_PREFIX}${dscKey}`, { value: dscValue }); diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 4b1109549d47..f5202d64570a 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -4,6 +4,7 @@ import { getActiveTransaction, getClient, getCurrentScope, + getDynamicSamplingContextFromSpan, hasTracingEnabled, runWithAsyncContext, spanToJSON, @@ -307,7 +308,7 @@ function getTraceAndBaggage(): { const span = getActiveSpan(); if (span && transaction) { - const dynamicSamplingContext = transaction.getDynamicSamplingContext(); + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(transaction); return { sentryTrace: spanToTraceHeader(span), diff --git a/packages/sveltekit/src/server/handle.ts b/packages/sveltekit/src/server/handle.ts index cf195551ef61..1bb0c485168e 100644 --- a/packages/sveltekit/src/server/handle.ts +++ b/packages/sveltekit/src/server/handle.ts @@ -1,4 +1,4 @@ -import { getActiveSpan, getCurrentScope, spanToTraceHeader } from '@sentry/core'; +import { getActiveSpan, getCurrentScope, getDynamicSamplingContextFromSpan, spanToTraceHeader } from '@sentry/core'; import { getActiveTransaction, runWithAsyncContext, startSpan } from '@sentry/core'; import { captureException } from '@sentry/node'; /* eslint-disable @sentry-internal/sdk/no-optional-chaining */ @@ -100,7 +100,7 @@ export function addSentryCodeToPage(options: SentryHandleOptions): NonNullable diff --git a/packages/tracing-internal/src/browser/request.ts b/packages/tracing-internal/src/browser/request.ts index d7c4c7892189..79d91a7d0785 100644 --- a/packages/tracing-internal/src/browser/request.ts +++ b/packages/tracing-internal/src/browser/request.ts @@ -4,6 +4,7 @@ import { getClient, getCurrentScope, getDynamicSamplingContextFromClient, + getDynamicSamplingContextFromSpan, hasTracingEnabled, spanToTraceHeader, } from '@sentry/core'; @@ -297,7 +298,7 @@ export function xhrCallback( if (xhr.setRequestHeader && shouldAttachHeaders(sentryXhrData.url)) { if (span) { const transaction = span && span.transaction; - const dynamicSamplingContext = transaction && transaction.getDynamicSamplingContext(); + const dynamicSamplingContext = transaction && getDynamicSamplingContextFromSpan(transaction); const sentryBaggageHeader = dynamicSamplingContextToSentryBaggageHeader(dynamicSamplingContext); setHeaderOnXhr(xhr, spanToTraceHeader(span), sentryBaggageHeader); } else { diff --git a/packages/tracing-internal/src/common/fetch.ts b/packages/tracing-internal/src/common/fetch.ts index cc04885e4436..f41e5129f4ee 100644 --- a/packages/tracing-internal/src/common/fetch.ts +++ b/packages/tracing-internal/src/common/fetch.ts @@ -3,6 +3,7 @@ import { getClient, getCurrentScope, getDynamicSamplingContextFromClient, + getDynamicSamplingContextFromSpan, hasTracingEnabled, spanToTraceHeader, } from '@sentry/core'; @@ -139,7 +140,7 @@ export function addTracingHeadersToFetchRequest( const sentryTraceHeader = span ? spanToTraceHeader(span) : generateSentryTraceHeader(traceId, undefined, sampled); const dynamicSamplingContext = transaction - ? transaction.getDynamicSamplingContext() + ? getDynamicSamplingContextFromSpan(transaction) : dsc ? dsc : getDynamicSamplingContextFromClient(traceId, client, scope); diff --git a/packages/types/src/transaction.ts b/packages/types/src/transaction.ts index d07c5ce435c1..710c7034f4b2 100644 --- a/packages/types/src/transaction.ts +++ b/packages/types/src/transaction.ts @@ -136,7 +136,11 @@ export interface Transaction extends TransactionContext, Omit): void; - /** Return the current Dynamic Sampling Context of this transaction */ + /** + * Return the current Dynamic Sampling Context of this transaction + * + * @deprecated Use top-level `getDynamicSamplingContextFromSpan` instead. + */ getDynamicSamplingContext(): Partial; } From 5788a5499ad4916caf15e49618311f2943d5f293 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 8 Jan 2024 14:40:08 +0100 Subject: [PATCH 02/10] fix v7 frozenDSC obtain mechanism --- packages/core/src/tracing/dynamicSamplingContext.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 783a1e5ecf14..9ba90224208f 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -48,10 +48,14 @@ type TransactionWithV7FrozenDsc = Span & { _frozenDynamicSamplingContext?: Dynam * @returns a dynamic sampling context */ export function getDynamicSamplingContextFromSpan(span: Span): Readonly> { + // As long as we use `Transaction`s internally, this should be fine. + // TODO: We need to replace this with a `getRootSpan(span)` function though + const txn = span.transaction; + // TODO (v8): Remove v7FrozenDsc as a Transaction will no longer have _frozenDynamicSamplingContext // For now we need to avoid breaking users who directly created a txn with a DSC, where this field is still set. // @see Transaction class constructor - const v7FrozenDsc = (span as TransactionWithV7FrozenDsc)._frozenDynamicSamplingContext; + const v7FrozenDsc = (txn as TransactionWithV7FrozenDsc)._frozenDynamicSamplingContext; if (v7FrozenDsc) { return v7FrozenDsc; } @@ -63,7 +67,6 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly Date: Mon, 8 Jan 2024 17:14:44 +0100 Subject: [PATCH 03/10] fix tests, add tests --- packages/astro/src/server/meta.ts | 7 +- packages/astro/test/server/meta.test.ts | 4 + .../src/tracing/dynamicSamplingContext.ts | 29 +++---- .../tracing/dynamicSamplingContext.test.ts | 77 +++++++++++++++++++ 4 files changed, 101 insertions(+), 16 deletions(-) create mode 100644 packages/core/test/lib/tracing/dynamicSamplingContext.test.ts diff --git a/packages/astro/src/server/meta.ts b/packages/astro/src/server/meta.ts index a76c09c598d9..1a908acc9ceb 100644 --- a/packages/astro/src/server/meta.ts +++ b/packages/astro/src/server/meta.ts @@ -1,5 +1,8 @@ -import { getDynamicSamplingContextFromClient, spanToTraceHeader } from '@sentry/core'; -import { getDynamicSamplingContextFromSpan } from '@sentry/core/src/tracing/dynamicSamplingContext'; +import { + getDynamicSamplingContextFromClient, + getDynamicSamplingContextFromSpan, + spanToTraceHeader, +} from '@sentry/core'; import type { Client, Scope, Span } from '@sentry/types'; import { TRACEPARENT_REGEXP, diff --git a/packages/astro/test/server/meta.test.ts b/packages/astro/test/server/meta.test.ts index 1aa9e9405be5..f235ad34d7ca 100644 --- a/packages/astro/test/server/meta.test.ts +++ b/packages/astro/test/server/meta.test.ts @@ -32,6 +32,10 @@ const mockedScope = { describe('getTracingMetaTags', () => { it('returns the tracing tags from the span, if it is provided', () => { { + vi.spyOn(SentryCore, 'getDynamicSamplingContextFromSpan').mockReturnValueOnce({ + environment: 'production', + }); + const tags = getTracingMetaTags(mockedSpan, mockedScope, mockedClient); expect(tags).toEqual({ diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 9ba90224208f..19820c6950a7 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -1,4 +1,4 @@ -import type { Client, DynamicSamplingContext, Scope, Span } from '@sentry/types'; +import type { Client, DynamicSamplingContext, Scope, Span, Transaction } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; import { DEFAULT_ENVIRONMENT } from '../constants'; @@ -38,7 +38,7 @@ export function getDynamicSamplingContextFromClient( /** * A Span with a frozen dynamic sampling context. */ -type TransactionWithV7FrozenDsc = Span & { _frozenDynamicSamplingContext?: DynamicSamplingContext }; +type TransactionWithV7FrozenDsc = Transaction & { _frozenDynamicSamplingContext?: DynamicSamplingContext }; /** * Creates a dynamic sampling context from a span (and client and scope) @@ -48,18 +48,6 @@ type TransactionWithV7FrozenDsc = Span & { _frozenDynamicSamplingContext?: Dynam * @returns a dynamic sampling context */ export function getDynamicSamplingContextFromSpan(span: Span): Readonly> { - // As long as we use `Transaction`s internally, this should be fine. - // TODO: We need to replace this with a `getRootSpan(span)` function though - const txn = span.transaction; - - // TODO (v8): Remove v7FrozenDsc as a Transaction will no longer have _frozenDynamicSamplingContext - // For now we need to avoid breaking users who directly created a txn with a DSC, where this field is still set. - // @see Transaction class constructor - const v7FrozenDsc = (txn as TransactionWithV7FrozenDsc)._frozenDynamicSamplingContext; - if (v7FrozenDsc) { - return v7FrozenDsc; - } - const client = getClient(); if (!client) { return {}; @@ -67,10 +55,23 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly { + beforeEach(() => { + const options = getDefaultTestClientOptions({ tracesSampleRate: 0.0, release: '1.0.1' }); + const client = new TestClient(options); + const hub = new Hub(client); + makeMain(hub); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + test('returns the DSC provided during transaction creation', () => { + const transaction = new Transaction({ + name: 'tx', + metadata: { dynamicSamplingContext: { environment: 'myEnv' } }, + }); + + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(transaction); + + expect(dynamicSamplingContext).toStrictEqual({ environment: 'myEnv' }); + }); + + test('returns a new DSC, if no DSC was provided during transaction creation', () => { + const transaction = new Transaction({ + name: 'tx', + metadata: { + sampleRate: 0.56, + }, + }); + + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(transaction); + + expect(dynamicSamplingContext).toStrictEqual({ + release: '1.0.1', + environment: 'production', + sample_rate: '0.56', + trace_id: expect.any(String), + transaction: 'tx', + }); + }); + + describe('Including transaction name in DSC', () => { + test('is not included if transaction source is url', () => { + const transaction = new Transaction({ + name: 'tx', + metadata: { + source: 'url', + }, + }); + + const dsc = getDynamicSamplingContextFromSpan(transaction); + expect(dsc.transaction).toBeUndefined(); + }); + + test.each([ + ['is included if transaction source is parameterized route/url', 'route'], + ['is included if transaction source is a custom name', 'custom'], + ])('%s', (_: string, source) => { + const transaction = new Transaction({ + name: 'tx', + metadata: { + ...(source && { source: source as TransactionSource }), + }, + }); + + const dsc = getDynamicSamplingContextFromSpan(transaction); + + expect(dsc.transaction).toEqual('tx'); + }); + }); +}); From c9c1592fef134096e211d2e9bb99eb4f0f340f2d Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 8 Jan 2024 17:52:05 +0100 Subject: [PATCH 04/10] apply new APIs (spanIsSampled, spanToJson) after rebase --- .../src/tracing/dynamicSamplingContext.ts | 20 +++++++++---------- packages/core/src/tracing/transaction.ts | 1 + .../tracing/dynamicSamplingContext.test.ts | 2 ++ 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 19820c6950a7..8a91c5422d08 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -3,6 +3,7 @@ import { dropUndefinedKeys } from '@sentry/utils'; import { DEFAULT_ENVIRONMENT } from '../constants'; import { getClient, getCurrentScope } from '../exports'; +import { spanIsSampled, spanToJSON } from '../utils/spanUtils'; /** * Creates a dynamic sampling context from a client. @@ -54,16 +55,15 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly> | undefined; private _metadata: Partial; diff --git a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts index d3cb5599443e..1e1dabd7c71d 100644 --- a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts +++ b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts @@ -29,6 +29,7 @@ describe('getDynamicSamplingContextFromSpan', () => { test('returns a new DSC, if no DSC was provided during transaction creation', () => { const transaction = new Transaction({ name: 'tx', + sampled: true, metadata: { sampleRate: 0.56, }, @@ -39,6 +40,7 @@ describe('getDynamicSamplingContextFromSpan', () => { expect(dynamicSamplingContext).toStrictEqual({ release: '1.0.1', environment: 'production', + sampled: 'true', sample_rate: '0.56', trace_id: expect.any(String), transaction: 'tx', From 14f2cff9d99f701be0567ed05b097a6b89f29fe8 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 8 Jan 2024 18:08:46 +0100 Subject: [PATCH 05/10] fix tracing tests --- packages/tracing/test/span.test.ts | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/tracing/test/span.test.ts b/packages/tracing/test/span.test.ts index d47919d7acdb..e2ebf335ecb5 100644 --- a/packages/tracing/test/span.test.ts +++ b/packages/tracing/test/span.test.ts @@ -571,24 +571,19 @@ describe('Span', () => { hub, ); - const hubSpy = jest.spyOn(hub.getClient()!, 'getOptions'); - const dynamicSamplingContext = transaction.getDynamicSamplingContext(); - expect(hubSpy).not.toHaveBeenCalled(); expect(dynamicSamplingContext).toStrictEqual({ environment: 'myEnv' }); }); test('should return new DSC, if no DSC was provided during transaction creation', () => { - const transaction = new Transaction( - { - name: 'tx', - metadata: { - sampleRate: 0.56, - }, + const transaction = new Transaction({ + name: 'tx', + metadata: { + sampleRate: 0.56, }, - hub, - ); + sampled: true, + }); const getOptionsSpy = jest.spyOn(hub.getClient()!, 'getOptions'); @@ -598,6 +593,7 @@ describe('Span', () => { expect(dynamicSamplingContext).toStrictEqual({ release: '1.0.1', environment: 'production', + sampled: 'true', sample_rate: '0.56', trace_id: expect.any(String), transaction: 'tx', From 9c76fc9831b162c75dfa99f5e9c087d19e5cd93c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 9 Jan 2024 12:26:54 +0100 Subject: [PATCH 06/10] use txn.attributes instead of txn.metadata.source --- packages/core/src/tracing/dynamicSamplingContext.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 8a91c5422d08..c610285a69c7 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -1,8 +1,9 @@ -import type { Client, DynamicSamplingContext, Scope, Span, Transaction } from '@sentry/types'; +import type { Client, DynamicSamplingContext, Scope, Span, Transaction, TransactionSource } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; import { DEFAULT_ENVIRONMENT } from '../constants'; import { getClient, getCurrentScope } from '../exports'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import { spanIsSampled, spanToJSON } from '../utils/spanUtils'; /** @@ -78,7 +79,7 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly Date: Tue, 9 Jan 2024 14:19:03 +0100 Subject: [PATCH 07/10] sample rate attribute && remove emit --- packages/core/src/tracing/dynamicSamplingContext.ts | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index c610285a69c7..489e1374387e 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -3,7 +3,7 @@ import { dropUndefinedKeys } from '@sentry/utils'; import { DEFAULT_ENVIRONMENT } from '../constants'; import { getClient, getCurrentScope } from '../exports'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; +import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import { spanIsSampled, spanToJSON } from '../utils/spanUtils'; /** @@ -15,7 +15,6 @@ export function getDynamicSamplingContextFromClient( trace_id: string, client: Client, scope?: Scope, - emitHook: boolean = true, ): DynamicSamplingContext { const options = client.getOptions(); @@ -30,9 +29,7 @@ export function getDynamicSamplingContextFromClient( trace_id, }) as DynamicSamplingContext; - if (emitHook) { - client.emit && client.emit('createDsc', dsc); - } + client.emit && client.emit('createDsc', dsc); return dsc; } @@ -56,7 +53,7 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly Date: Tue, 9 Jan 2024 18:18:12 +0100 Subject: [PATCH 08/10] fix tests once again --- .../tracing/dynamicSamplingContext.test.ts | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts index 1e1dabd7c71d..944ac2ff6528 100644 --- a/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts +++ b/packages/core/test/lib/tracing/dynamicSamplingContext.test.ts @@ -1,14 +1,18 @@ import type { TransactionSource } from '@sentry/types'; -import { Hub, makeMain } from '../../../src'; -import { Transaction, getDynamicSamplingContextFromSpan } from '../../../src/tracing'; +import { Hub, SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, makeMain } from '../../../src'; +import { Transaction, getDynamicSamplingContextFromSpan, startInactiveSpan } from '../../../src/tracing'; +import { addTracingExtensions } from '../../../src/tracing'; import { TestClient, getDefaultTestClientOptions } from '../../mocks/client'; describe('getDynamicSamplingContextFromSpan', () => { + let hub: Hub; beforeEach(() => { - const options = getDefaultTestClientOptions({ tracesSampleRate: 0.0, release: '1.0.1' }); + const options = getDefaultTestClientOptions({ tracesSampleRate: 1.0, release: '1.0.1' }); const client = new TestClient(options); - const hub = new Hub(client); + hub = new Hub(client); + hub.bindClient(client); makeMain(hub); + addTracingExtensions(); }); afterEach(() => { @@ -27,15 +31,14 @@ describe('getDynamicSamplingContextFromSpan', () => { }); test('returns a new DSC, if no DSC was provided during transaction creation', () => { - const transaction = new Transaction({ - name: 'tx', - sampled: true, - metadata: { - sampleRate: 0.56, - }, - }); + const transaction = startInactiveSpan({ name: 'tx' }); - const dynamicSamplingContext = getDynamicSamplingContextFromSpan(transaction); + // Only setting the attribute manually because we can't "fake" a + // sample rate or txn name source anymore like we used to. + transaction?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, 0.56); + transaction?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); + + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(transaction!); expect(dynamicSamplingContext).toStrictEqual({ release: '1.0.1', @@ -71,6 +74,9 @@ describe('getDynamicSamplingContextFromSpan', () => { }, }); + // Only setting the attribute manually because we're directly calling new Transaction() + transaction?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, source); + const dsc = getDynamicSamplingContextFromSpan(transaction); expect(dsc.transaction).toEqual('tx'); From 74b9799ed9ee1dc1febdbb7710c6bb21a4e9f6a2 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 10 Jan 2024 11:04:14 +0100 Subject: [PATCH 09/10] use txn.metadata instead of txn.attributes :( --- .../src/tracing/dynamicSamplingContext.ts | 6 ++- .../tracing/dynamicSamplingContext.test.ts | 45 +++++++++++++++++-- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 489e1374387e..2d5a49566eeb 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -70,13 +70,15 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly { expect(dynamicSamplingContext).toStrictEqual({ environment: 'myEnv' }); }); - test('returns a new DSC, if no DSC was provided during transaction creation', () => { + test('returns a new DSC, if no DSC was provided during transaction creation (via attributes)', () => { const transaction = startInactiveSpan({ name: 'tx' }); - // Only setting the attribute manually because we can't "fake" a - // sample rate or txn name source anymore like we used to. + // Setting the attribute should overwrite the computed values transaction?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, 0.56); transaction?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route'); @@ -50,12 +49,52 @@ describe('getDynamicSamplingContextFromSpan', () => { }); }); + test('returns a new DSC, if no DSC was provided during transaction creation (via deprecated metadata)', () => { + const transaction = startInactiveSpan({ + name: 'tx', + }); + + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(transaction!); + + expect(dynamicSamplingContext).toStrictEqual({ + release: '1.0.1', + environment: 'production', + sampled: 'true', + sample_rate: '1', + trace_id: expect.any(String), + transaction: 'tx', + }); + }); + + test('returns a new DSC, if no DSC was provided during transaction creation (via new Txn and deprecated metadata)', () => { + const transaction = new Transaction({ + name: 'tx', + metadata: { + sampleRate: 0.56, + source: 'route', + }, + sampled: true, + }); + + const dynamicSamplingContext = getDynamicSamplingContextFromSpan(transaction!); + + expect(dynamicSamplingContext).toStrictEqual({ + release: '1.0.1', + environment: 'production', + sampled: 'true', + sample_rate: '0.56', + trace_id: expect.any(String), + transaction: 'tx', + }); + }); + describe('Including transaction name in DSC', () => { test('is not included if transaction source is url', () => { const transaction = new Transaction({ name: 'tx', metadata: { source: 'url', + sampleRate: 0.56, }, }); From 6962539490ab38e09a7e1fae91ec1ce39d0a7246 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 10 Jan 2024 11:21:51 +0100 Subject: [PATCH 10/10] lint --- packages/core/src/tracing/dynamicSamplingContext.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index 2d5a49566eeb..318c232eb43b 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -1,9 +1,8 @@ -import type { Client, DynamicSamplingContext, Scope, Span, Transaction, TransactionSource } from '@sentry/types'; +import type { Client, DynamicSamplingContext, Scope, Span, Transaction } from '@sentry/types'; import { dropUndefinedKeys } from '@sentry/utils'; import { DEFAULT_ENVIRONMENT } from '../constants'; import { getClient, getCurrentScope } from '../exports'; -import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import { spanIsSampled, spanToJSON } from '../utils/spanUtils'; /**