Skip to content

Commit

Permalink
feat(core): Deprecate Transaction.getDynamicSamplingContext in favo…
Browse files Browse the repository at this point in the history
…ur of `getDynamicSamplingContextFromSpan` (#10094)

Deprecate `Transaction.getDynamicSamplingContext` and
introduce its direct replacement, a top-level utility function. Note
that this only is an intermediate step we should take to rework how
we generate and handle the DSC creation. More details in #10095
  • Loading branch information
Lms24 authored Jan 10, 2024
1 parent eb5bb3d commit 6faec42
Show file tree
Hide file tree
Showing 24 changed files with 276 additions and 75 deletions.
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.
* `span.tags`: Set tags on the surrounding scope instead, or use attributes.
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
65 changes: 63 additions & 2 deletions packages/core/src/tracing/dynamicSamplingContext.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import type { Client, DynamicSamplingContext, Scope } 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 { 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 +32,62 @@ 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;
}

// TODO (v8): Replace txn.metadata with txn.attributes[]
// We can't do this yet because attributes aren't always set yet.
// eslint-disable-next-line deprecation/deprecation
const { sampleRate: maybeSampleRate, source } = txn.metadata;
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 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 @@ -230,43 +231,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 @@ -344,7 +313,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
124 changes: 124 additions & 0 deletions packages/core/test/lib/tracing/dynamicSamplingContext.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
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 (via attributes)', () => {
const transaction = startInactiveSpan({ name: 'tx' });

// Setting the attribute should overwrite the computed values
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',
});
});

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

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
Loading

0 comments on commit 6faec42

Please sign in to comment.