Skip to content

Commit 5fd5033

Browse files
authored
fix(node-experimental): Sample in OTEL Sampler (#9203)
In order for sampling propagation to work, we need to sample in OTEL, not in Sentry. As the sentry spans are only created later. Note that this _does not_ fix propagation yet in node-experimental, once this is merged I still need to adjust the propagator (or rather, fork it) from opentelemetry-node to work without sentry spans.
1 parent a173fa2 commit 5fd5033

15 files changed

+561
-119
lines changed

packages/node-experimental/src/constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,4 @@ export const OTEL_ATTR_BREADCRUMB_LEVEL = 'sentry.breadcrumb.level';
1313
export const OTEL_ATTR_BREADCRUMB_EVENT_ID = 'sentry.breadcrumb.event_id';
1414
export const OTEL_ATTR_BREADCRUMB_CATEGORY = 'sentry.breadcrumb.category';
1515
export const OTEL_ATTR_BREADCRUMB_DATA = 'sentry.breadcrumb.data';
16+
export const OTEL_ATTR_SENTRY_SAMPLE_RATE = 'sentry.sample_rate';
Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
/* eslint-disable no-bitwise */
2+
import type { Attributes, Context, SpanContext } from '@opentelemetry/api';
3+
import { isSpanContextValid, trace, TraceFlags } from '@opentelemetry/api';
4+
import type { Sampler, SamplingResult } from '@opentelemetry/sdk-trace-base';
5+
import { SamplingDecision } from '@opentelemetry/sdk-trace-base';
6+
import { hasTracingEnabled } from '@sentry/core';
7+
import { _INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY } from '@sentry/opentelemetry-node';
8+
import type { Client, ClientOptions, SamplingContext, TraceparentData } from '@sentry/types';
9+
import { isNaN, logger } from '@sentry/utils';
10+
11+
import { OTEL_ATTR_PARENT_SAMPLED, OTEL_ATTR_SENTRY_SAMPLE_RATE } from '../constants';
12+
13+
/**
14+
* A custom OTEL sampler that uses Sentry sampling rates to make it's decision
15+
*/
16+
export class SentrySampler implements Sampler {
17+
private _client: Client;
18+
19+
public constructor(client: Client) {
20+
this._client = client;
21+
}
22+
23+
/** @inheritDoc */
24+
public shouldSample(
25+
context: Context,
26+
traceId: string,
27+
spanName: string,
28+
_spanKind: unknown,
29+
_attributes: unknown,
30+
_links: unknown,
31+
): SamplingResult {
32+
const options = this._client.getOptions();
33+
34+
if (!hasTracingEnabled(options)) {
35+
return { decision: SamplingDecision.NOT_RECORD };
36+
}
37+
38+
const parentContext = trace.getSpanContext(context);
39+
40+
let parentSampled: boolean | undefined = undefined;
41+
42+
// Only inherit sample rate if `traceId` is the same
43+
// Note for testing: `isSpanContextValid()` checks the format of the traceId/spanId, so we need to pass valid ones
44+
if (parentContext && isSpanContextValid(parentContext) && parentContext.traceId === traceId) {
45+
if (parentContext.isRemote) {
46+
parentSampled = getParentRemoteSampled(parentContext, context);
47+
__DEBUG_BUILD__ &&
48+
logger.log(`[Tracing] Inheriting remote parent's sampled decision for ${spanName}: ${parentSampled}`);
49+
} else {
50+
parentSampled = Boolean(parentContext.traceFlags & TraceFlags.SAMPLED);
51+
__DEBUG_BUILD__ &&
52+
logger.log(`[Tracing] Inheriting parent's sampled decision for ${spanName}: ${parentSampled}`);
53+
}
54+
}
55+
56+
const sampleRate = getSampleRate(options, {
57+
transactionContext: {
58+
name: spanName,
59+
parentSampled,
60+
},
61+
parentSampled,
62+
});
63+
64+
const attributes: Attributes = {
65+
[OTEL_ATTR_SENTRY_SAMPLE_RATE]: Number(sampleRate),
66+
};
67+
68+
if (typeof parentSampled === 'boolean') {
69+
attributes[OTEL_ATTR_PARENT_SAMPLED] = parentSampled;
70+
}
71+
72+
// Since this is coming from the user (or from a function provided by the user), who knows what we might get. (The
73+
// only valid values are booleans or numbers between 0 and 1.)
74+
if (!isValidSampleRate(sampleRate)) {
75+
__DEBUG_BUILD__ && logger.warn('[Tracing] Discarding span because of invalid sample rate.');
76+
77+
return {
78+
decision: SamplingDecision.NOT_RECORD,
79+
attributes,
80+
};
81+
}
82+
83+
// if the function returned 0 (or false), or if `tracesSampleRate` is 0, it's a sign the transaction should be dropped
84+
if (!sampleRate) {
85+
__DEBUG_BUILD__ &&
86+
logger.log(
87+
`[Tracing] Discarding span because ${
88+
typeof options.tracesSampler === 'function'
89+
? 'tracesSampler returned 0 or false'
90+
: 'a negative sampling decision was inherited or tracesSampleRate is set to 0'
91+
}`,
92+
);
93+
94+
return {
95+
decision: SamplingDecision.NOT_RECORD,
96+
attributes,
97+
};
98+
}
99+
100+
// Now we roll the dice. Math.random is inclusive of 0, but not of 1, so strict < is safe here. In case sampleRate is
101+
// a boolean, the < comparison will cause it to be automatically cast to 1 if it's true and 0 if it's false.
102+
const isSampled = Math.random() < (sampleRate as number | boolean);
103+
104+
// if we're not going to keep it, we're done
105+
if (!isSampled) {
106+
__DEBUG_BUILD__ &&
107+
logger.log(
108+
`[Tracing] Discarding span because it's not included in the random sample (sampling rate = ${Number(
109+
sampleRate,
110+
)})`,
111+
);
112+
113+
return {
114+
decision: SamplingDecision.NOT_RECORD,
115+
attributes,
116+
};
117+
}
118+
119+
return {
120+
decision: SamplingDecision.RECORD_AND_SAMPLED,
121+
attributes,
122+
};
123+
}
124+
125+
/** Returns the sampler name or short description with the configuration. */
126+
public toString(): string {
127+
return 'SentrySampler';
128+
}
129+
}
130+
131+
function getSampleRate(
132+
options: Pick<ClientOptions, 'tracesSampleRate' | 'tracesSampler' | 'enableTracing'>,
133+
samplingContext: SamplingContext,
134+
): number | boolean {
135+
if (typeof options.tracesSampler === 'function') {
136+
return options.tracesSampler(samplingContext);
137+
}
138+
139+
if (samplingContext.parentSampled !== undefined) {
140+
return samplingContext.parentSampled;
141+
}
142+
143+
if (typeof options.tracesSampleRate !== 'undefined') {
144+
return options.tracesSampleRate;
145+
}
146+
147+
// When `enableTracing === true`, we use a sample rate of 100%
148+
if (options.enableTracing) {
149+
return 1;
150+
}
151+
152+
return 0;
153+
}
154+
155+
/**
156+
* Checks the given sample rate to make sure it is valid type and value (a boolean, or a number between 0 and 1).
157+
*/
158+
function isValidSampleRate(rate: unknown): boolean {
159+
// we need to check NaN explicitly because it's of type 'number' and therefore wouldn't get caught by this typecheck
160+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
161+
if (isNaN(rate) || !(typeof rate === 'number' || typeof rate === 'boolean')) {
162+
__DEBUG_BUILD__ &&
163+
logger.warn(
164+
`[Tracing] Given sample rate is invalid. Sample rate must be a boolean or a number between 0 and 1. Got ${JSON.stringify(
165+
rate,
166+
)} of type ${JSON.stringify(typeof rate)}.`,
167+
);
168+
return false;
169+
}
170+
171+
// in case sampleRate is a boolean, it will get automatically cast to 1 if it's true and 0 if it's false
172+
if (rate < 0 || rate > 1) {
173+
__DEBUG_BUILD__ &&
174+
logger.warn(`[Tracing] Given sample rate is invalid. Sample rate must be between 0 and 1. Got ${rate}.`);
175+
return false;
176+
}
177+
return true;
178+
}
179+
180+
function getTraceParentData(parentContext: Context): TraceparentData | undefined {
181+
return parentContext.getValue(_INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY) as TraceparentData | undefined;
182+
}
183+
184+
function getParentRemoteSampled(spanContext: SpanContext, context: Context): boolean | undefined {
185+
const traceId = spanContext.traceId;
186+
const traceparentData = getTraceParentData(context);
187+
188+
// Only inherit sample rate if `traceId` is the same
189+
return traceparentData && traceId === traceparentData.traceId ? traceparentData.parentSampled : undefined;
190+
}

packages/node-experimental/src/opentelemetry/spanExporter.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,13 @@ import { mapOtelStatus, parseOtelSpanDescription } from '@sentry/opentelemetry-n
99
import type { DynamicSamplingContext, Span as SentrySpan, SpanOrigin, TransactionSource } from '@sentry/types';
1010
import { logger } from '@sentry/utils';
1111

12-
import { OTEL_ATTR_OP, OTEL_ATTR_ORIGIN, OTEL_ATTR_PARENT_SAMPLED, OTEL_ATTR_SOURCE } from '../constants';
12+
import {
13+
OTEL_ATTR_OP,
14+
OTEL_ATTR_ORIGIN,
15+
OTEL_ATTR_PARENT_SAMPLED,
16+
OTEL_ATTR_SENTRY_SAMPLE_RATE,
17+
OTEL_ATTR_SOURCE,
18+
} from '../constants';
1319
import { getCurrentHub } from '../sdk/hub';
1420
import { NodeExperimentalScope } from '../sdk/scope';
1521
import type { NodeExperimentalTransaction } from '../sdk/transaction';
@@ -172,11 +178,13 @@ function createTransactionForOtelSpan(span: ReadableSpan): NodeExperimentalTrans
172178
metadata: {
173179
dynamicSamplingContext,
174180
source,
181+
sampleRate: span.attributes[OTEL_ATTR_SENTRY_SAMPLE_RATE] as number | undefined,
175182
...metadata,
176183
},
177184
data: removeSentryAttributes(data),
178185
origin,
179186
tags,
187+
sampled: true,
180188
}) as NodeExperimentalTransaction;
181189

182190
transaction.setContext('otel', {
@@ -270,6 +278,7 @@ function removeSentryAttributes(data: Record<string, unknown>): Record<string, u
270278
delete cleanedData[OTEL_ATTR_ORIGIN];
271279
delete cleanedData[OTEL_ATTR_OP];
272280
delete cleanedData[OTEL_ATTR_SOURCE];
281+
delete cleanedData[OTEL_ATTR_SENTRY_SAMPLE_RATE];
273282
/* eslint-enable @typescript-eslint/no-dynamic-delete */
274283

275284
return cleanedData;

packages/node-experimental/src/opentelemetry/spanProcessor.ts

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,11 @@ import { ROOT_CONTEXT, SpanKind, trace } from '@opentelemetry/api';
33
import type { Span, SpanProcessor as SpanProcessorInterface } from '@opentelemetry/sdk-trace-base';
44
import { BatchSpanProcessor } from '@opentelemetry/sdk-trace-base';
55
import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
6-
import {
7-
_INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY,
8-
maybeCaptureExceptionForTimedEvent,
9-
} from '@sentry/opentelemetry-node';
10-
import type { Hub, TraceparentData } from '@sentry/types';
6+
import { maybeCaptureExceptionForTimedEvent } from '@sentry/opentelemetry-node';
7+
import type { Hub } from '@sentry/types';
8+
import { logger } from '@sentry/utils';
119

12-
import { OTEL_ATTR_PARENT_SAMPLED, OTEL_CONTEXT_HUB_KEY } from '../constants';
10+
import { OTEL_CONTEXT_HUB_KEY } from '../constants';
1311
import { Http } from '../integrations';
1412
import type { NodeExperimentalClient } from '../sdk/client';
1513
import { getCurrentHub } from '../sdk/hub';
@@ -51,17 +49,15 @@ export class SentrySpanProcessor extends BatchSpanProcessor implements SpanProce
5149
setSpanHub(span, actualHub);
5250
}
5351

54-
// We need to set this here based on the parent context
55-
const parentSampled = getParentSampled(span, parentContext);
56-
if (typeof parentSampled === 'boolean') {
57-
span.setAttribute(OTEL_ATTR_PARENT_SAMPLED, parentSampled);
58-
}
52+
__DEBUG_BUILD__ && logger.log(`[Tracing] Starting span "${span.name}" (${span.spanContext().spanId})`);
5953

6054
return super.onStart(span, parentContext);
6155
}
6256

6357
/** @inheritDoc */
6458
public onEnd(span: Span): void {
59+
__DEBUG_BUILD__ && logger.log(`[Tracing] Finishing span "${span.name}" (${span.spanContext().spanId})`);
60+
6561
if (!shouldCaptureSentrySpan(span)) {
6662
// Prevent this being called to super.onEnd(), which would pass this to the span exporter
6763
return;
@@ -77,19 +73,6 @@ export class SentrySpanProcessor extends BatchSpanProcessor implements SpanProce
7773
}
7874
}
7975

80-
function getTraceParentData(parentContext: Context): TraceparentData | undefined {
81-
return parentContext.getValue(_INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY) as TraceparentData | undefined;
82-
}
83-
84-
function getParentSampled(span: Span, parentContext: Context): boolean | undefined {
85-
const spanContext = span.spanContext();
86-
const traceId = spanContext.traceId;
87-
const traceparentData = getTraceParentData(parentContext);
88-
89-
// Only inherit sample rate if `traceId` is the same
90-
return traceparentData && traceId === traceparentData.traceId ? traceparentData.parentSampled : undefined;
91-
}
92-
9376
function shouldCaptureSentrySpan(span: Span): boolean {
9477
const client = getCurrentHub().getClient<NodeExperimentalClient>();
9578
const httpIntegration = client ? client.getIntegration(Http) : undefined;

packages/node-experimental/src/sdk/initOtel.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { diag, DiagLogLevel } from '@opentelemetry/api';
22
import { Resource } from '@opentelemetry/resources';
3-
import { AlwaysOnSampler, BasicTracerProvider } from '@opentelemetry/sdk-trace-base';
3+
import { BasicTracerProvider } from '@opentelemetry/sdk-trace-base';
44
import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions';
55
import { SDK_VERSION } from '@sentry/core';
66
import { SentryPropagator } from '@sentry/opentelemetry-node';
77
import { logger } from '@sentry/utils';
88

9+
import { SentrySampler } from '../opentelemetry/sampler';
910
import { SentrySpanProcessor } from '../opentelemetry/spanProcessor';
1011
import type { NodeExperimentalClient } from '../types';
1112
import { setupEventContextTrace } from '../utils/setupEventContextTrace';
@@ -19,7 +20,15 @@ import { getCurrentHub } from './hub';
1920
export function initOtel(): void {
2021
const client = getCurrentHub().getClient<NodeExperimentalClient>();
2122

22-
if (client?.getOptions().debug) {
23+
if (!client) {
24+
__DEBUG_BUILD__ &&
25+
logger.warn(
26+
'No client available, skipping OpenTelemetry setup. This probably means that `Sentry.init()` was not called before `initOtel()`.',
27+
);
28+
return;
29+
}
30+
31+
if (client.getOptions().debug) {
2332
const otelLogger = new Proxy(logger as typeof logger & { verbose: (typeof logger)['debug'] }, {
2433
get(target, prop, receiver) {
2534
const actualProp = prop === 'verbose' ? 'debug' : prop;
@@ -30,21 +39,17 @@ export function initOtel(): void {
3039
diag.setLogger(otelLogger, DiagLogLevel.DEBUG);
3140
}
3241

33-
if (client) {
34-
setupEventContextTrace(client);
35-
}
42+
setupEventContextTrace(client);
3643

37-
const provider = setupOtel();
38-
if (client) {
39-
client.traceProvider = provider;
40-
}
44+
const provider = setupOtel(client);
45+
client.traceProvider = provider;
4146
}
4247

4348
/** Just exported for tests. */
44-
export function setupOtel(): BasicTracerProvider {
49+
export function setupOtel(client: NodeExperimentalClient): BasicTracerProvider {
4550
// Create and configure NodeTracerProvider
4651
const provider = new BasicTracerProvider({
47-
sampler: new AlwaysOnSampler(),
52+
sampler: new SentrySampler(client),
4853
resource: new Resource({
4954
[SemanticResourceAttributes.SERVICE_NAME]: 'node-experimental',
5055
[SemanticResourceAttributes.SERVICE_NAMESPACE]: 'sentry',

packages/node-experimental/src/sdk/transaction.ts

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,21 @@
11
import type { Hub } from '@sentry/core';
2-
import { sampleTransaction, Transaction } from '@sentry/core';
3-
import type {
4-
ClientOptions,
5-
CustomSamplingContext,
6-
Hub as HubInterface,
7-
Scope,
8-
TransactionContext,
9-
} from '@sentry/types';
2+
import { Transaction } from '@sentry/core';
3+
import type { ClientOptions, Hub as HubInterface, Scope, TransactionContext } from '@sentry/types';
104
import { uuid4 } from '@sentry/utils';
115

126
/**
137
* This is a fork of core's tracing/hubextensions.ts _startTransaction,
148
* with some OTEL specifics.
159
*/
16-
export function startTransaction(
17-
hub: HubInterface,
18-
transactionContext: TransactionContext,
19-
customSamplingContext?: CustomSamplingContext,
20-
): Transaction {
10+
export function startTransaction(hub: HubInterface, transactionContext: TransactionContext): Transaction {
2111
const client = hub.getClient();
2212
const options: Partial<ClientOptions> = (client && client.getOptions()) || {};
2313

24-
let transaction = new NodeExperimentalTransaction(transactionContext, hub as Hub);
25-
transaction = sampleTransaction(transaction, options, {
26-
parentSampled: transactionContext.parentSampled,
27-
transactionContext,
28-
...customSamplingContext,
29-
});
30-
if (transaction.sampled) {
31-
transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));
32-
}
14+
const transaction = new NodeExperimentalTransaction(transactionContext, hub as Hub);
15+
// Since we do not do sampling here, we assume that this is _always_ sampled
16+
// Any sampling decision happens in OpenTelemetry's sampler
17+
transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number));
18+
3319
if (client && client.emit) {
3420
client.emit('startTransaction', transaction);
3521
}

0 commit comments

Comments
 (0)