Skip to content
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

feat(core): Pass name & attributes to tracesSampler #10426

Merged
merged 2 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ npx @sentry/migr8@latest
This will let you select which updates to run, and automatically update your code. Make sure to still review all code
changes!

## Deprecated `transactionContext` passed to `tracesSampler`

Instead of an `transactionContext` being passed to the `tracesSampler` callback, the callback will directly receive
`name` and `attributes` going forward. You can use these to make your sampling decisions, while `transactionContext`
will be removed in v8. Note that the `attributes` are only the attributes at span creation time, and some attributes may
only be set later during the span lifecycle (and thus not be available during sampling).

## Deprecate using `getClient()` to check if the SDK was initialized

In v8, `getClient()` will stop returning `undefined` if `Sentry.init()` was not called. For cases where this may be used
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/tracing/hubextensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,14 @@ The transaction will not be sampled. Please use the ${configInstrumenter} instru
// eslint-disable-next-line deprecation/deprecation
let transaction = new Transaction(transactionContext, this);
transaction = sampleTransaction(transaction, options, {
name: transactionContext.name,
parentSampled: transactionContext.parentSampled,
transactionContext,
attributes: {
// eslint-disable-next-line deprecation/deprecation
...transactionContext.data,
...transactionContext.attributes,
},
...customSamplingContext,
});
if (transaction.isRecording()) {
Expand Down Expand Up @@ -106,8 +112,14 @@ export function startIdleTransaction(
delayAutoFinishUntilSignal,
);
transaction = sampleTransaction(transaction, options, {
name: transactionContext.name,
parentSampled: transactionContext.parentSampled,
transactionContext,
attributes: {
// eslint-disable-next-line deprecation/deprecation
...transactionContext.data,
...transactionContext.attributes,
},
...customSamplingContext,
});
if (transaction.isRecording()) {
Expand Down
30 changes: 30 additions & 0 deletions packages/core/test/lib/tracing/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
addTracingExtensions,
getCurrentScope,
makeMain,
setCurrentClient,
spanToJSON,
withScope,
} from '../../../src';
Expand Down Expand Up @@ -357,6 +358,35 @@ describe('startSpan', () => {
expect(span).toBeDefined();
});
});

it('samples with a tracesSampler', () => {
const tracesSampler = jest.fn(() => {
return true;
});

const options = getDefaultTestClientOptions({ tracesSampler });
client = new TestClient(options);
setCurrentClient(client);

startSpan(
{ name: 'outer', attributes: { test1: 'aa', test2: 'aa' }, data: { test1: 'bb', test3: 'bb' } },
outerSpan => {
expect(outerSpan).toBeDefined();
},
);

expect(tracesSampler).toBeCalledTimes(1);
expect(tracesSampler).toHaveBeenLastCalledWith({
parentSampled: undefined,
name: 'outer',
attributes: {
test1: 'aa',
test2: 'aa',
test3: 'bb',
},
transactionContext: expect.objectContaining({ name: 'outer', parentSampled: undefined }),
});
});
});

describe('startSpanManual', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/opentelemetry-node/src/spanprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export class SentrySpanProcessor implements OtelSpanProcessor {
const transaction = getCurrentHub().startTransaction({
name: otelSpan.name,
...traceCtx,
attributes: otelSpan.attributes,
instrumenter: 'otel',
startTimestamp: convertOtelTimeToSeconds(otelSpan.startTime),
spanId: otelSpanId,
Expand Down
8 changes: 5 additions & 3 deletions packages/opentelemetry/src/sampler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { Attributes, Context, SpanContext } from '@opentelemetry/api';
import { TraceFlags, isSpanContextValid, trace } from '@opentelemetry/api';
import type { Sampler, SamplingResult } from '@opentelemetry/sdk-trace-base';
import { SamplingDecision } from '@opentelemetry/sdk-trace-base';
import { hasTracingEnabled } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, hasTracingEnabled } from '@sentry/core';
import type { Client, ClientOptions, SamplingContext } from '@sentry/types';
import { isNaN, logger } from '@sentry/utils';

Expand All @@ -27,7 +27,7 @@ export class SentrySampler implements Sampler {
traceId: string,
spanName: string,
_spanKind: unknown,
_attributes: unknown,
spanAttributes: unknown,
_links: unknown,
): SamplingResult {
const options = this._client.getOptions();
Expand All @@ -54,6 +54,8 @@ export class SentrySampler implements Sampler {
}

const sampleRate = getSampleRate(options, {
name: spanName,
attributes: spanAttributes,
transactionContext: {
name: spanName,
parentSampled,
Expand All @@ -62,7 +64,7 @@ export class SentrySampler implements Sampler {
});

const attributes: Attributes = {
[InternalSentrySemanticAttributes.SAMPLE_RATE]: Number(sampleRate),
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: Number(sampleRate),
};

if (typeof parentSampled === 'boolean') {
Expand Down
6 changes: 3 additions & 3 deletions packages/opentelemetry/src/spanExporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { ExportResult } from '@opentelemetry/core';
import { ExportResultCode } from '@opentelemetry/core';
import type { ReadableSpan, SpanExporter } from '@opentelemetry/sdk-trace-base';
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
import { flush, getCurrentScope } from '@sentry/core';
import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, flush, getCurrentScope } from '@sentry/core';
import type { Scope, Span as SentrySpan, SpanOrigin, TransactionSource } from '@sentry/types';
import { logger } from '@sentry/utils';

Expand Down Expand Up @@ -176,7 +176,7 @@ function createTransactionForOtelSpan(span: ReadableSpan): OpenTelemetryTransact
metadata: {
dynamicSamplingContext,
source,
sampleRate: span.attributes[InternalSentrySemanticAttributes.SAMPLE_RATE] as number | undefined,
sampleRate: span.attributes[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE] as number | undefined,
...metadata,
},
data: removeSentryAttributes(data),
Expand Down Expand Up @@ -267,7 +267,7 @@ function removeSentryAttributes(data: Record<string, unknown>): Record<string, u
delete cleanedData[InternalSentrySemanticAttributes.ORIGIN];
delete cleanedData[InternalSentrySemanticAttributes.OP];
delete cleanedData[InternalSentrySemanticAttributes.SOURCE];
delete cleanedData[InternalSentrySemanticAttributes.SAMPLE_RATE];
delete cleanedData[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE];
/* eslint-enable @typescript-eslint/no-dynamic-delete */

return cleanedData;
Expand Down
31 changes: 24 additions & 7 deletions packages/opentelemetry/test/trace.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { SpanKind } from '@opentelemetry/api';
import { TraceFlags, context, trace } from '@opentelemetry/api';
import type { ReadableSpan } from '@opentelemetry/sdk-trace-base';
import { Span as SpanClass } from '@opentelemetry/sdk-trace-base';
import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE } from '@sentry/core';
import type { PropagationContext } from '@sentry/types';

import { getClient } from '../src/custom/hub';
Expand Down Expand Up @@ -206,7 +207,7 @@ describe('trace', () => {
span => {
expect(span).toBeDefined();
expect(getSpanAttributes(span)).toEqual({
[InternalSentrySemanticAttributes.SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
});

expect(getSpanMetadata(span)).toEqual(undefined);
Expand All @@ -227,7 +228,7 @@ describe('trace', () => {
[InternalSentrySemanticAttributes.SOURCE]: 'task',
[InternalSentrySemanticAttributes.ORIGIN]: 'auto.test.origin',
[InternalSentrySemanticAttributes.OP]: 'my-op',
[InternalSentrySemanticAttributes.SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
});

expect(getSpanMetadata(span)).toEqual({ requestPath: 'test-path' });
Expand All @@ -253,7 +254,7 @@ describe('trace', () => {
expect(span).toBeDefined();
expect(getSpanName(span)).toEqual('outer');
expect(getSpanAttributes(span)).toEqual({
[InternalSentrySemanticAttributes.SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
test1: 'test 1',
test2: 2,
});
Expand Down Expand Up @@ -326,7 +327,7 @@ describe('trace', () => {

expect(span).toBeDefined();
expect(getSpanAttributes(span)).toEqual({
[InternalSentrySemanticAttributes.SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
});

expect(getSpanMetadata(span)).toEqual(undefined);
Expand All @@ -341,7 +342,7 @@ describe('trace', () => {

expect(span2).toBeDefined();
expect(getSpanAttributes(span2)).toEqual({
[InternalSentrySemanticAttributes.SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
[InternalSentrySemanticAttributes.SOURCE]: 'task',
[InternalSentrySemanticAttributes.ORIGIN]: 'auto.test.origin',
[InternalSentrySemanticAttributes.OP]: 'my-op',
Expand All @@ -366,7 +367,7 @@ describe('trace', () => {
expect(span).toBeDefined();
expect(getSpanName(span)).toEqual('outer');
expect(getSpanAttributes(span)).toEqual({
[InternalSentrySemanticAttributes.SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
test1: 'test 1',
test2: 2,
});
Expand Down Expand Up @@ -451,7 +452,7 @@ describe('trace', () => {
expect(span).toBeDefined();
expect(getSpanName(span)).toEqual('outer');
expect(getSpanAttributes(span)).toEqual({
[InternalSentrySemanticAttributes.SAMPLE_RATE]: 1,
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
test1: 'test 1',
test2: 2,
});
Expand Down Expand Up @@ -688,6 +689,10 @@ describe('trace (sampling)', () => {
expect(tracesSampler).toBeCalledTimes(1);
expect(tracesSampler).toHaveBeenLastCalledWith({
parentSampled: undefined,
name: 'outer',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
},
transactionContext: { name: 'outer', parentSampled: undefined },
});

Expand All @@ -705,6 +710,8 @@ describe('trace (sampling)', () => {
expect(tracesSampler).toHaveBeenCalledTimes(3);
expect(tracesSampler).toHaveBeenLastCalledWith({
parentSampled: false,
name: 'inner2',
attributes: {},
transactionContext: { name: 'inner2', parentSampled: false },
});
});
Expand All @@ -727,6 +734,10 @@ describe('trace (sampling)', () => {
expect(tracesSampler).toBeCalledTimes(1);
expect(tracesSampler).toHaveBeenLastCalledWith({
parentSampled: undefined,
name: 'outer',
attributes: {
[SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE]: 1,
},
transactionContext: { name: 'outer', parentSampled: undefined },
});

Expand All @@ -744,6 +755,8 @@ describe('trace (sampling)', () => {
expect(tracesSampler).toHaveBeenCalledTimes(3);
expect(tracesSampler).toHaveBeenLastCalledWith({
parentSampled: false,
name: 'inner2',
attributes: {},
transactionContext: { name: 'inner2', parentSampled: false },
});

Expand All @@ -757,6 +770,8 @@ describe('trace (sampling)', () => {
expect(tracesSampler).toHaveBeenCalledTimes(4);
expect(tracesSampler).toHaveBeenLastCalledWith({
parentSampled: undefined,
name: 'outer3',
attributes: {},
transactionContext: { name: 'outer3', parentSampled: undefined },
});
});
Expand Down Expand Up @@ -799,6 +814,8 @@ describe('trace (sampling)', () => {
expect(tracesSampler).toBeCalledTimes(1);
expect(tracesSampler).toHaveBeenLastCalledWith({
parentSampled: true,
name: 'outer',
attributes: {},
transactionContext: {
name: 'outer',
parentSampled: true,
Expand Down
Loading