Skip to content

feat(core): Deprecate Transaction.getDynamicSamplingContext in favour of getDynamicSamplingContextFromSpan #10094

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
3 changes: 3 additions & 0 deletions dev-packages/rollup-utils/plugins/bundlePlugins.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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',
],
},
},
Expand Down
8 changes: 6 additions & 2 deletions packages/astro/src/server/meta.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { getDynamicSamplingContextFromClient, spanToTraceHeader } from '@sentry/core';
import {
getDynamicSamplingContextFromClient,
getDynamicSamplingContextFromSpan,
spanToTraceHeader,
} from '@sentry/core';
import type { Client, Scope, Span } from '@sentry/types';
import {
TRACEPARENT_REGEXP,
Expand Down Expand Up @@ -33,7 +37,7 @@ export function getTracingMetaTags(
const sentryTrace = span ? spanToTraceHeader(span) : generateSentryTraceHeader(traceId, undefined, sampled);

const dynamicSamplingContext = transaction
? transaction.getDynamicSamplingContext()
? getDynamicSamplingContextFromSpan(transaction)
: dsc
? dsc
: client
Expand Down
4 changes: 4 additions & 0 deletions packages/astro/test/server/meta.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
8 changes: 6 additions & 2 deletions packages/core/src/server-runtime-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<BaseTransportOptions> {
Expand Down Expand Up @@ -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)];
}

Expand Down
64 changes: 62 additions & 2 deletions packages/core/src/tracing/dynamicSamplingContext.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import type { Client, DynamicSamplingContext, Scope } 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_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes';
import { spanIsSampled, spanToJSON } from '../utils/spanUtils';

/**
* 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,
Expand All @@ -30,3 +33,60 @@ export function getDynamicSamplingContextFromClient(

return dsc;
}

/**
* A Span with a frozen dynamic sampling context.
*/
type TransactionWithV7FrozenDsc = Transaction & { _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<Partial<DynamicSamplingContext>> {
const client = getClient();
if (!client) {
return {};
}

// passing emit=false here to only emit later once the DSC is actually populated
const dsc = getDynamicSamplingContextFromClient(spanToJSON(span).trace_id || '', client, getCurrentScope());

// 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 as TransactionWithV7FrozenDsc | undefined;
if (!txn) {
return dsc;
}

// 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 && txn._frozenDynamicSamplingContext;
if (v7FrozenDsc) {
return v7FrozenDsc;
}

const maybeSampleRate = txn.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined;
if (maybeSampleRate != null) {
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.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SOURCE] as TransactionSource | undefined;
const jsonSpan = spanToJSON(txn);

// after JSON conversion, txn.name becomes jsonSpan.description
if (source && source !== 'url') {
dsc.transaction = jsonSpan.description;
}

dsc.sampled = String(spanIsSampled(txn));

client.emit && client.emit('createDsc', dsc);

return dsc;
}
2 changes: 1 addition & 1 deletion packages/core/src/tracing/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,5 @@ export {
startSpanManual,
continueTrace,
} from './trace';
export { getDynamicSamplingContextFromClient } from './dynamicSamplingContext';
export { getDynamicSamplingContextFromClient, getDynamicSamplingContextFromSpan } from './dynamicSamplingContext';
export { setMeasurement } from './measurement';
43 changes: 6 additions & 37 deletions packages/core/src/tracing/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -35,6 +35,7 @@ export class Transaction extends SpanClass implements TransactionInterface {

private _trimEnd?: boolean;

// DO NOT yet remove this property, it is used in a hack for v7 backwards compatibility.
private _frozenDynamicSamplingContext: Readonly<Partial<DynamicSamplingContext>> | undefined;

private _metadata: Partial<TransactionMetadata>;
Expand Down Expand Up @@ -227,43 +228,11 @@ export class Transaction extends SpanClass implements TransactionInterface {
* @inheritdoc
*
* @experimental
*
* @deprecated Use top-level `getDynamicSamplingContextFromSpan` instead.
*/
public getDynamicSamplingContext(): Readonly<Partial<DynamicSamplingContext>> {
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);
}

/**
Expand Down Expand Up @@ -340,7 +309,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
type: 'transaction',
sdkProcessingMetadata: {
...metadata,
dynamicSamplingContext: this.getDynamicSamplingContext(),
dynamicSamplingContext: getDynamicSamplingContextFromSpan(this),
},
...(source && {
transaction_info: {
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/utils/applyScopeDataToEvent.ts
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand Down Expand Up @@ -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;
Expand Down
85 changes: 85 additions & 0 deletions packages/core/test/lib/tracing/dynamicSamplingContext.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import type { TransactionSource } from '@sentry/types';
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: 1.0, release: '1.0.1' });
const client = new TestClient(options);
hub = new Hub(client);
hub.bindClient(client);
makeMain(hub);
addTracingExtensions();
});

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 = 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.
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',
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',
},
});

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 }),
},
});

// 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');
});
});
});
10 changes: 8 additions & 2 deletions packages/nextjs/src/common/wrapAppGetInitialPropsWithSentry.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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);
}
Expand Down
10 changes: 8 additions & 2 deletions packages/nextjs/src/common/wrapErrorGetInitialPropsWithSentry.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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);
}

Expand Down
10 changes: 8 additions & 2 deletions packages/nextjs/src/common/wrapGetInitialPropsWithSentry.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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);
}

Expand Down
Loading