diff --git a/packages/node-experimental/src/constants.ts b/packages/node-experimental/src/constants.ts index 930574157d73..c41660be0fa2 100644 --- a/packages/node-experimental/src/constants.ts +++ b/packages/node-experimental/src/constants.ts @@ -13,3 +13,4 @@ export const OTEL_ATTR_BREADCRUMB_LEVEL = 'sentry.breadcrumb.level'; export const OTEL_ATTR_BREADCRUMB_EVENT_ID = 'sentry.breadcrumb.event_id'; export const OTEL_ATTR_BREADCRUMB_CATEGORY = 'sentry.breadcrumb.category'; export const OTEL_ATTR_BREADCRUMB_DATA = 'sentry.breadcrumb.data'; +export const OTEL_ATTR_SENTRY_SAMPLE_RATE = 'sentry.sample_rate'; diff --git a/packages/node-experimental/src/opentelemetry/sampler.ts b/packages/node-experimental/src/opentelemetry/sampler.ts new file mode 100644 index 000000000000..327294fbf272 --- /dev/null +++ b/packages/node-experimental/src/opentelemetry/sampler.ts @@ -0,0 +1,190 @@ +/* eslint-disable no-bitwise */ +import type { Attributes, Context, SpanContext } from '@opentelemetry/api'; +import { isSpanContextValid, trace, TraceFlags } 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 { _INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY } from '@sentry/opentelemetry-node'; +import type { Client, ClientOptions, SamplingContext, TraceparentData } from '@sentry/types'; +import { isNaN, logger } from '@sentry/utils'; + +import { OTEL_ATTR_PARENT_SAMPLED, OTEL_ATTR_SENTRY_SAMPLE_RATE } from '../constants'; + +/** + * A custom OTEL sampler that uses Sentry sampling rates to make it's decision + */ +export class SentrySampler implements Sampler { + private _client: Client; + + public constructor(client: Client) { + this._client = client; + } + + /** @inheritDoc */ + public shouldSample( + context: Context, + traceId: string, + spanName: string, + _spanKind: unknown, + _attributes: unknown, + _links: unknown, + ): SamplingResult { + const options = this._client.getOptions(); + + if (!hasTracingEnabled(options)) { + return { decision: SamplingDecision.NOT_RECORD }; + } + + const parentContext = trace.getSpanContext(context); + + let parentSampled: boolean | undefined = undefined; + + // Only inherit sample rate if `traceId` is the same + // Note for testing: `isSpanContextValid()` checks the format of the traceId/spanId, so we need to pass valid ones + if (parentContext && isSpanContextValid(parentContext) && parentContext.traceId === traceId) { + if (parentContext.isRemote) { + parentSampled = getParentRemoteSampled(parentContext, context); + __DEBUG_BUILD__ && + logger.log(`[Tracing] Inheriting remote parent's sampled decision for ${spanName}: ${parentSampled}`); + } else { + parentSampled = Boolean(parentContext.traceFlags & TraceFlags.SAMPLED); + __DEBUG_BUILD__ && + logger.log(`[Tracing] Inheriting parent's sampled decision for ${spanName}: ${parentSampled}`); + } + } + + const sampleRate = getSampleRate(options, { + transactionContext: { + name: spanName, + parentSampled, + }, + parentSampled, + }); + + const attributes: Attributes = { + [OTEL_ATTR_SENTRY_SAMPLE_RATE]: Number(sampleRate), + }; + + if (typeof parentSampled === 'boolean') { + attributes[OTEL_ATTR_PARENT_SAMPLED] = parentSampled; + } + + // Since this is coming from the user (or from a function provided by the user), who knows what we might get. (The + // only valid values are booleans or numbers between 0 and 1.) + if (!isValidSampleRate(sampleRate)) { + __DEBUG_BUILD__ && logger.warn('[Tracing] Discarding span because of invalid sample rate.'); + + return { + decision: SamplingDecision.NOT_RECORD, + attributes, + }; + } + + // if the function returned 0 (or false), or if `tracesSampleRate` is 0, it's a sign the transaction should be dropped + if (!sampleRate) { + __DEBUG_BUILD__ && + logger.log( + `[Tracing] Discarding span because ${ + typeof options.tracesSampler === 'function' + ? 'tracesSampler returned 0 or false' + : 'a negative sampling decision was inherited or tracesSampleRate is set to 0' + }`, + ); + + return { + decision: SamplingDecision.NOT_RECORD, + attributes, + }; + } + + // Now we roll the dice. Math.random is inclusive of 0, but not of 1, so strict < is safe here. In case sampleRate is + // a boolean, the < comparison will cause it to be automatically cast to 1 if it's true and 0 if it's false. + const isSampled = Math.random() < (sampleRate as number | boolean); + + // if we're not going to keep it, we're done + if (!isSampled) { + __DEBUG_BUILD__ && + logger.log( + `[Tracing] Discarding span because it's not included in the random sample (sampling rate = ${Number( + sampleRate, + )})`, + ); + + return { + decision: SamplingDecision.NOT_RECORD, + attributes, + }; + } + + return { + decision: SamplingDecision.RECORD_AND_SAMPLED, + attributes, + }; + } + + /** Returns the sampler name or short description with the configuration. */ + public toString(): string { + return 'SentrySampler'; + } +} + +function getSampleRate( + options: Pick, + samplingContext: SamplingContext, +): number | boolean { + if (typeof options.tracesSampler === 'function') { + return options.tracesSampler(samplingContext); + } + + if (samplingContext.parentSampled !== undefined) { + return samplingContext.parentSampled; + } + + if (typeof options.tracesSampleRate !== 'undefined') { + return options.tracesSampleRate; + } + + // When `enableTracing === true`, we use a sample rate of 100% + if (options.enableTracing) { + return 1; + } + + return 0; +} + +/** + * Checks the given sample rate to make sure it is valid type and value (a boolean, or a number between 0 and 1). + */ +function isValidSampleRate(rate: unknown): boolean { + // we need to check NaN explicitly because it's of type 'number' and therefore wouldn't get caught by this typecheck + // eslint-disable-next-line @typescript-eslint/no-explicit-any + if (isNaN(rate) || !(typeof rate === 'number' || typeof rate === 'boolean')) { + __DEBUG_BUILD__ && + logger.warn( + `[Tracing] Given sample rate is invalid. Sample rate must be a boolean or a number between 0 and 1. Got ${JSON.stringify( + rate, + )} of type ${JSON.stringify(typeof rate)}.`, + ); + return false; + } + + // in case sampleRate is a boolean, it will get automatically cast to 1 if it's true and 0 if it's false + if (rate < 0 || rate > 1) { + __DEBUG_BUILD__ && + logger.warn(`[Tracing] Given sample rate is invalid. Sample rate must be between 0 and 1. Got ${rate}.`); + return false; + } + return true; +} + +function getTraceParentData(parentContext: Context): TraceparentData | undefined { + return parentContext.getValue(_INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY) as TraceparentData | undefined; +} + +function getParentRemoteSampled(spanContext: SpanContext, context: Context): boolean | undefined { + const traceId = spanContext.traceId; + const traceparentData = getTraceParentData(context); + + // Only inherit sample rate if `traceId` is the same + return traceparentData && traceId === traceparentData.traceId ? traceparentData.parentSampled : undefined; +} diff --git a/packages/node-experimental/src/opentelemetry/spanExporter.ts b/packages/node-experimental/src/opentelemetry/spanExporter.ts index 90af46e8672d..e242f74d6104 100644 --- a/packages/node-experimental/src/opentelemetry/spanExporter.ts +++ b/packages/node-experimental/src/opentelemetry/spanExporter.ts @@ -9,7 +9,13 @@ import { mapOtelStatus, parseOtelSpanDescription } from '@sentry/opentelemetry-n import type { DynamicSamplingContext, Span as SentrySpan, SpanOrigin, TransactionSource } from '@sentry/types'; import { logger } from '@sentry/utils'; -import { OTEL_ATTR_OP, OTEL_ATTR_ORIGIN, OTEL_ATTR_PARENT_SAMPLED, OTEL_ATTR_SOURCE } from '../constants'; +import { + OTEL_ATTR_OP, + OTEL_ATTR_ORIGIN, + OTEL_ATTR_PARENT_SAMPLED, + OTEL_ATTR_SENTRY_SAMPLE_RATE, + OTEL_ATTR_SOURCE, +} from '../constants'; import { getCurrentHub } from '../sdk/hub'; import { NodeExperimentalScope } from '../sdk/scope'; import type { NodeExperimentalTransaction } from '../sdk/transaction'; @@ -172,11 +178,13 @@ function createTransactionForOtelSpan(span: ReadableSpan): NodeExperimentalTrans metadata: { dynamicSamplingContext, source, + sampleRate: span.attributes[OTEL_ATTR_SENTRY_SAMPLE_RATE] as number | undefined, ...metadata, }, data: removeSentryAttributes(data), origin, tags, + sampled: true, }) as NodeExperimentalTransaction; transaction.setContext('otel', { @@ -270,6 +278,7 @@ function removeSentryAttributes(data: Record): Record(); const httpIntegration = client ? client.getIntegration(Http) : undefined; diff --git a/packages/node-experimental/src/sdk/initOtel.ts b/packages/node-experimental/src/sdk/initOtel.ts index 855a443889bb..134926f19c35 100644 --- a/packages/node-experimental/src/sdk/initOtel.ts +++ b/packages/node-experimental/src/sdk/initOtel.ts @@ -1,11 +1,12 @@ import { diag, DiagLogLevel } from '@opentelemetry/api'; import { Resource } from '@opentelemetry/resources'; -import { AlwaysOnSampler, BasicTracerProvider } from '@opentelemetry/sdk-trace-base'; +import { BasicTracerProvider } from '@opentelemetry/sdk-trace-base'; import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'; import { SDK_VERSION } from '@sentry/core'; import { SentryPropagator } from '@sentry/opentelemetry-node'; import { logger } from '@sentry/utils'; +import { SentrySampler } from '../opentelemetry/sampler'; import { SentrySpanProcessor } from '../opentelemetry/spanProcessor'; import type { NodeExperimentalClient } from '../types'; import { setupEventContextTrace } from '../utils/setupEventContextTrace'; @@ -19,7 +20,15 @@ import { getCurrentHub } from './hub'; export function initOtel(): void { const client = getCurrentHub().getClient(); - if (client?.getOptions().debug) { + if (!client) { + __DEBUG_BUILD__ && + logger.warn( + 'No client available, skipping OpenTelemetry setup. This probably means that `Sentry.init()` was not called before `initOtel()`.', + ); + return; + } + + if (client.getOptions().debug) { const otelLogger = new Proxy(logger as typeof logger & { verbose: (typeof logger)['debug'] }, { get(target, prop, receiver) { const actualProp = prop === 'verbose' ? 'debug' : prop; @@ -30,21 +39,17 @@ export function initOtel(): void { diag.setLogger(otelLogger, DiagLogLevel.DEBUG); } - if (client) { - setupEventContextTrace(client); - } + setupEventContextTrace(client); - const provider = setupOtel(); - if (client) { - client.traceProvider = provider; - } + const provider = setupOtel(client); + client.traceProvider = provider; } /** Just exported for tests. */ -export function setupOtel(): BasicTracerProvider { +export function setupOtel(client: NodeExperimentalClient): BasicTracerProvider { // Create and configure NodeTracerProvider const provider = new BasicTracerProvider({ - sampler: new AlwaysOnSampler(), + sampler: new SentrySampler(client), resource: new Resource({ [SemanticResourceAttributes.SERVICE_NAME]: 'node-experimental', [SemanticResourceAttributes.SERVICE_NAMESPACE]: 'sentry', diff --git a/packages/node-experimental/src/sdk/transaction.ts b/packages/node-experimental/src/sdk/transaction.ts index c301dd6e9521..382d9a926f5d 100644 --- a/packages/node-experimental/src/sdk/transaction.ts +++ b/packages/node-experimental/src/sdk/transaction.ts @@ -1,35 +1,21 @@ import type { Hub } from '@sentry/core'; -import { sampleTransaction, Transaction } from '@sentry/core'; -import type { - ClientOptions, - CustomSamplingContext, - Hub as HubInterface, - Scope, - TransactionContext, -} from '@sentry/types'; +import { Transaction } from '@sentry/core'; +import type { ClientOptions, Hub as HubInterface, Scope, TransactionContext } from '@sentry/types'; import { uuid4 } from '@sentry/utils'; /** * This is a fork of core's tracing/hubextensions.ts _startTransaction, * with some OTEL specifics. */ -export function startTransaction( - hub: HubInterface, - transactionContext: TransactionContext, - customSamplingContext?: CustomSamplingContext, -): Transaction { +export function startTransaction(hub: HubInterface, transactionContext: TransactionContext): Transaction { const client = hub.getClient(); const options: Partial = (client && client.getOptions()) || {}; - let transaction = new NodeExperimentalTransaction(transactionContext, hub as Hub); - transaction = sampleTransaction(transaction, options, { - parentSampled: transactionContext.parentSampled, - transactionContext, - ...customSamplingContext, - }); - if (transaction.sampled) { - transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number)); - } + const transaction = new NodeExperimentalTransaction(transactionContext, hub as Hub); + // Since we do not do sampling here, we assume that this is _always_ sampled + // Any sampling decision happens in OpenTelemetry's sampler + transaction.initSpanRecorder(options._experiments && (options._experiments.maxSpans as number)); + if (client && client.emit) { client.emit('startTransaction', transaction); } diff --git a/packages/node-experimental/test/helpers/getDefaultNodePreviewClientOptions.ts b/packages/node-experimental/test/helpers/getDefaultNodePreviewClientOptions.ts index c5f2b8b5cf82..00778e78582a 100644 --- a/packages/node-experimental/test/helpers/getDefaultNodePreviewClientOptions.ts +++ b/packages/node-experimental/test/helpers/getDefaultNodePreviewClientOptions.ts @@ -7,6 +7,7 @@ export function getDefaultNodeExperimentalClientOptions( options: Partial = {}, ): NodeExperimentalClientOptions { return { + tracesSampleRate: 1, integrations: [], transport: () => createTransport({ recordDroppedEvent: () => undefined }, _ => resolvedSyncPromise({})), stackParser: () => [], diff --git a/packages/node-experimental/test/integration/transactions.test.ts b/packages/node-experimental/test/integration/transactions.test.ts index 00ec85700316..7979b771c440 100644 --- a/packages/node-experimental/test/integration/transactions.test.ts +++ b/packages/node-experimental/test/integration/transactions.test.ts @@ -541,7 +541,7 @@ describe('Integration | Transactions', () => { ); }); - it('does not creates spans for http requests if disabled in http integration xxx', async () => { + it('does not creates spans for http requests if disabled in http integration', async () => { const beforeSendTransaction = jest.fn(() => null); mockSdkInit({ enableTracing: true, beforeSendTransaction }); diff --git a/packages/node-experimental/test/sdk/client.test.ts b/packages/node-experimental/test/sdk/client.test.ts index 03ee60ecbf0b..b7db215a4cd8 100644 --- a/packages/node-experimental/test/sdk/client.test.ts +++ b/packages/node-experimental/test/sdk/client.test.ts @@ -29,6 +29,7 @@ describe('NodeExperimentalClient', () => { platform: 'node', runtime: { name: 'node', version: expect.any(String) }, serverName: expect.any(String), + tracesSampleRate: 1, }); }); diff --git a/packages/node-experimental/test/sdk/otelAsyncContextStrategy.test.ts b/packages/node-experimental/test/sdk/otelAsyncContextStrategy.test.ts index 346683bf45f3..518d61000fee 100644 --- a/packages/node-experimental/test/sdk/otelAsyncContextStrategy.test.ts +++ b/packages/node-experimental/test/sdk/otelAsyncContextStrategy.test.ts @@ -2,16 +2,20 @@ import type { BasicTracerProvider } from '@opentelemetry/sdk-trace-base'; import type { Hub } from '@sentry/core'; import { runWithAsyncContext, setAsyncContextStrategy } from '@sentry/core'; +import { NodeExperimentalClient } from '../../src/sdk/client'; import { getCurrentHub } from '../../src/sdk/hub'; import { setupOtel } from '../../src/sdk/initOtel'; import { setOtelContextAsyncContextStrategy } from '../../src/sdk/otelAsyncContextStrategy'; +import { getDefaultNodeExperimentalClientOptions } from '../helpers/getDefaultNodePreviewClientOptions'; import { cleanupOtel } from '../helpers/mockSdkInit'; describe('otelAsyncContextStrategy', () => { let provider: BasicTracerProvider | undefined; beforeEach(() => { - provider = setupOtel(); + const options = getDefaultNodeExperimentalClientOptions(); + const client = new NodeExperimentalClient(options); + provider = setupOtel(client); setOtelContextAsyncContextStrategy(); }); diff --git a/packages/node-experimental/test/sdk/trace.test.ts b/packages/node-experimental/test/sdk/trace.test.ts index 76ecb28a5f9d..e5f5a1aa7ac7 100644 --- a/packages/node-experimental/test/sdk/trace.test.ts +++ b/packages/node-experimental/test/sdk/trace.test.ts @@ -1,7 +1,9 @@ +import { context, trace, TraceFlags } from '@opentelemetry/api'; import type { Span } from '@opentelemetry/sdk-trace-base'; +import { _INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY } from '@sentry/opentelemetry-node'; import * as Sentry from '../../src'; -import { OTEL_ATTR_OP, OTEL_ATTR_ORIGIN, OTEL_ATTR_SOURCE } from '../../src/constants'; +import { OTEL_ATTR_OP, OTEL_ATTR_ORIGIN, OTEL_ATTR_SENTRY_SAMPLE_RATE, OTEL_ATTR_SOURCE } from '../../src/constants'; import { getSpanMetadata } from '../../src/opentelemetry/spanData'; import { getActiveSpan } from '../../src/utils/getActiveSpan'; import { cleanupOtel, mockSdkInit } from '../helpers/mockSdkInit'; @@ -142,7 +144,9 @@ describe('trace', () => { }, span => { expect(span).toBeDefined(); - expect(span?.attributes).toEqual({}); + expect(span?.attributes).toEqual({ + [OTEL_ATTR_SENTRY_SAMPLE_RATE]: 1, + }); expect(getSpanMetadata(span!)).toEqual(undefined); }, @@ -162,6 +166,7 @@ describe('trace', () => { [OTEL_ATTR_SOURCE]: 'task', [OTEL_ATTR_ORIGIN]: 'auto.test.origin', [OTEL_ATTR_OP]: 'my-op', + [OTEL_ATTR_SENTRY_SAMPLE_RATE]: 1, }); expect(getSpanMetadata(span!)).toEqual({ requestPath: 'test-path' }); @@ -210,7 +215,9 @@ describe('trace', () => { }); expect(span).toBeDefined(); - expect(span?.attributes).toEqual({}); + expect(span?.attributes).toEqual({ + [OTEL_ATTR_SENTRY_SAMPLE_RATE]: 1, + }); expect(getSpanMetadata(span!)).toEqual(undefined); @@ -224,6 +231,7 @@ describe('trace', () => { expect(span2).toBeDefined(); expect(span2?.attributes).toEqual({ + [OTEL_ATTR_SENTRY_SAMPLE_RATE]: 1, [OTEL_ATTR_SOURCE]: 'task', [OTEL_ATTR_ORIGIN]: 'auto.test.origin', [OTEL_ATTR_OP]: 'my-op', @@ -259,3 +267,299 @@ describe('trace (tracing disabled)', () => { expect(span).toBeUndefined(); }); }); + +describe('trace (sampling)', () => { + afterEach(() => { + cleanupOtel(); + jest.clearAllMocks(); + }); + + it('samples with a tracesSampleRate, when Math.random() > tracesSampleRate', () => { + jest.spyOn(Math, 'random').mockImplementation(() => 0.6); + + mockSdkInit({ tracesSampleRate: 0.5 }); + + Sentry.startSpan({ name: 'outer' }, outerSpan => { + expect(outerSpan).toBeUndefined(); + + Sentry.startSpan({ name: 'inner' }, innerSpan => { + expect(innerSpan).toBeUndefined(); + }); + }); + }); + + it('samples with a tracesSampleRate, when Math.random() < tracesSampleRate', () => { + jest.spyOn(Math, 'random').mockImplementation(() => 0.4); + + mockSdkInit({ tracesSampleRate: 0.5 }); + + Sentry.startSpan({ name: 'outer' }, outerSpan => { + expect(outerSpan).toBeDefined(); + expect(outerSpan?.isRecording()).toBe(true); + // All fields are empty for NonRecordingSpan + expect(outerSpan?.name).toBe('outer'); + + Sentry.startSpan({ name: 'inner' }, innerSpan => { + expect(innerSpan).toBeDefined(); + expect(innerSpan?.isRecording()).toBe(true); + expect(innerSpan?.name).toBe('inner'); + }); + }); + }); + + it('positive parent sampling takes precedence over tracesSampleRate', () => { + jest.spyOn(Math, 'random').mockImplementation(() => 0.6); + + mockSdkInit({ tracesSampleRate: 1 }); + + // This will def. be sampled because of the tracesSampleRate + Sentry.startSpan({ name: 'outer' }, outerSpan => { + expect(outerSpan).toBeDefined(); + expect(outerSpan?.isRecording()).toBe(true); + expect(outerSpan?.name).toBe('outer'); + + // Now let's mutate the tracesSampleRate so that the next entry _should_ not be sampled + // but it will because of parent sampling + const client = Sentry.getCurrentHub().getClient(); + client!.getOptions().tracesSampleRate = 0.5; + + Sentry.startSpan({ name: 'inner' }, innerSpan => { + expect(innerSpan).toBeDefined(); + expect(innerSpan?.isRecording()).toBe(true); + expect(innerSpan?.name).toBe('inner'); + }); + }); + }); + + it('negative parent sampling takes precedence over tracesSampleRate', () => { + jest.spyOn(Math, 'random').mockImplementation(() => 0.6); + + mockSdkInit({ tracesSampleRate: 0.5 }); + + // This will def. be sampled because of the tracesSampleRate + Sentry.startSpan({ name: 'outer' }, outerSpan => { + expect(outerSpan).toBeUndefined(); + + // Now let's mutate the tracesSampleRate so that the next entry _should_ not be sampled + // but it will because of parent sampling + const client = Sentry.getCurrentHub().getClient(); + client!.getOptions().tracesSampleRate = 1; + + Sentry.startSpan({ name: 'inner' }, innerSpan => { + expect(innerSpan).toBeUndefined(); + }); + }); + }); + + it('positive remote parent sampling takes precedence over tracesSampleRate', () => { + jest.spyOn(Math, 'random').mockImplementation(() => 0.6); + + mockSdkInit({ tracesSampleRate: 0.5 }); + + const traceId = 'd4cda95b652f4a1592b449d5929fda1b'; + const parentSpanId = '6e0c63257de34c92'; + + const spanContext = { + traceId, + spanId: parentSpanId, + sampled: true, + isRemote: true, + traceFlags: TraceFlags.SAMPLED, + }; + + const traceParentData = { + traceId, + parentSpanId, + parentSampled: true, + }; + + // We simulate the correct context we'd normally get from the SentryPropagator + context.with( + trace.setSpanContext( + context.active().setValue(_INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY, traceParentData), + spanContext, + ), + () => { + // This will def. be sampled because of the tracesSampleRate + Sentry.startSpan({ name: 'outer' }, outerSpan => { + expect(outerSpan).toBeDefined(); + expect(outerSpan?.isRecording()).toBe(true); + expect(outerSpan?.name).toBe('outer'); + }); + }, + ); + }); + + it('negative remote parent sampling takes precedence over tracesSampleRate', () => { + jest.spyOn(Math, 'random').mockImplementation(() => 0.6); + + mockSdkInit({ tracesSampleRate: 0.5 }); + + const traceId = 'd4cda95b652f4a1592b449d5929fda1b'; + const parentSpanId = '6e0c63257de34c92'; + + const spanContext = { + traceId, + spanId: parentSpanId, + sampled: false, + isRemote: true, + traceFlags: TraceFlags.NONE, + }; + + const traceParentData = { + traceId, + parentSpanId, + parentSampled: false, + }; + + // We simulate the correct context we'd normally get from the SentryPropagator + context.with( + trace.setSpanContext( + context.active().setValue(_INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY, traceParentData), + spanContext, + ), + () => { + // This will def. be sampled because of the tracesSampleRate + Sentry.startSpan({ name: 'outer' }, outerSpan => { + expect(outerSpan).toBeUndefined(); + }); + }, + ); + }); + + it('samples with a tracesSampler returning a boolean', () => { + let tracesSamplerResponse: boolean = true; + + const tracesSampler = jest.fn(() => { + return tracesSamplerResponse; + }); + + mockSdkInit({ tracesSampler }); + + Sentry.startSpan({ name: 'outer' }, outerSpan => { + expect(outerSpan).toBeDefined(); + }); + + expect(tracesSampler).toBeCalledTimes(1); + expect(tracesSampler).toHaveBeenLastCalledWith({ + parentSampled: undefined, + transactionContext: { name: 'outer', parentSampled: undefined }, + }); + + // Now return `false`, it should not sample + tracesSamplerResponse = false; + + Sentry.startSpan({ name: 'outer2' }, outerSpan => { + expect(outerSpan).toBeUndefined(); + + Sentry.startSpan({ name: 'inner2' }, outerSpan => { + expect(outerSpan).toBeUndefined(); + }); + }); + + expect(tracesSampler).toHaveBeenCalledTimes(3); + expect(tracesSampler).toHaveBeenLastCalledWith({ + parentSampled: false, + transactionContext: { name: 'inner2', parentSampled: false }, + }); + }); + + it('samples with a tracesSampler returning a number', () => { + jest.spyOn(Math, 'random').mockImplementation(() => 0.6); + + let tracesSamplerResponse: number = 1; + + const tracesSampler = jest.fn(() => { + return tracesSamplerResponse; + }); + + mockSdkInit({ tracesSampler }); + + Sentry.startSpan({ name: 'outer' }, outerSpan => { + expect(outerSpan).toBeDefined(); + }); + + expect(tracesSampler).toBeCalledTimes(1); + expect(tracesSampler).toHaveBeenLastCalledWith({ + parentSampled: undefined, + transactionContext: { name: 'outer', parentSampled: undefined }, + }); + + // Now return `0`, it should not sample + tracesSamplerResponse = 0; + + Sentry.startSpan({ name: 'outer2' }, outerSpan => { + expect(outerSpan).toBeUndefined(); + + Sentry.startSpan({ name: 'inner2' }, outerSpan => { + expect(outerSpan).toBeUndefined(); + }); + }); + + expect(tracesSampler).toHaveBeenCalledTimes(3); + expect(tracesSampler).toHaveBeenLastCalledWith({ + parentSampled: false, + transactionContext: { name: 'inner2', parentSampled: false }, + }); + + // Now return `0.4`, it should not sample + tracesSamplerResponse = 0.4; + + Sentry.startSpan({ name: 'outer3' }, outerSpan => { + expect(outerSpan).toBeUndefined(); + }); + + expect(tracesSampler).toHaveBeenCalledTimes(4); + expect(tracesSampler).toHaveBeenLastCalledWith({ + parentSampled: undefined, + transactionContext: { name: 'outer3', parentSampled: undefined }, + }); + }); + + it('samples with a tracesSampler even if parent is remotely sampled', () => { + const tracesSampler = jest.fn(() => { + return false; + }); + + mockSdkInit({ tracesSampler }); + const traceId = 'd4cda95b652f4a1592b449d5929fda1b'; + const parentSpanId = '6e0c63257de34c92'; + + const spanContext = { + traceId, + spanId: parentSpanId, + sampled: true, + isRemote: true, + traceFlags: TraceFlags.SAMPLED, + }; + + const traceParentData = { + traceId, + parentSpanId, + parentSampled: true, + }; + + // We simulate the correct context we'd normally get from the SentryPropagator + context.with( + trace.setSpanContext( + context.active().setValue(_INTERNAL_SENTRY_TRACE_PARENT_CONTEXT_KEY, traceParentData), + spanContext, + ), + () => { + // This will def. be sampled because of the tracesSampleRate + Sentry.startSpan({ name: 'outer' }, outerSpan => { + expect(outerSpan).toBeUndefined(); + }); + }, + ); + + expect(tracesSampler).toBeCalledTimes(1); + expect(tracesSampler).toHaveBeenLastCalledWith({ + parentSampled: true, + transactionContext: { + name: 'outer', + parentSampled: true, + }, + }); + }); +}); diff --git a/packages/node-experimental/test/sdk/transaction.test.ts b/packages/node-experimental/test/sdk/transaction.test.ts index bf27549c8017..132696655b09 100644 --- a/packages/node-experimental/test/sdk/transaction.test.ts +++ b/packages/node-experimental/test/sdk/transaction.test.ts @@ -139,37 +139,8 @@ describe('startTranscation', () => { jest.resetAllMocks(); }); - it('creates an unsampled NodeExperimentalTransaction by default', () => { - const client = new NodeExperimentalClient(getDefaultNodeExperimentalClientOptions()); - const mockEmit = jest.spyOn(client, 'emit').mockImplementation(() => {}); - const hub = getCurrentHub(); - hub.bindClient(client); - - const transaction = startTransaction(hub, { name: 'test' }); - - expect(transaction).toBeInstanceOf(NodeExperimentalTransaction); - expect(mockEmit).toBeCalledTimes(1); - expect(mockEmit).toBeCalledWith('startTransaction', transaction); - - expect(transaction.sampled).toBe(false); - expect(transaction.spanRecorder).toBeUndefined(); - expect(transaction.metadata).toEqual({ - source: 'custom', - spanMetadata: {}, - }); - - expect(transaction.toJSON()).toEqual( - expect.objectContaining({ - origin: 'manual', - span_id: expect.any(String), - start_timestamp: expect.any(Number), - trace_id: expect.any(String), - }), - ); - }); - - it('creates a sampled NodeExperimentalTransaction based on the tracesSampleRate', () => { - const client = new NodeExperimentalClient(getDefaultNodeExperimentalClientOptions({ tracesSampleRate: 1 })); + it('creates a NodeExperimentalTransaction', () => { + const client = new NodeExperimentalClient(getDefaultNodeExperimentalClientOptions({ tracesSampleRate: 0 })); const hub = getCurrentHub(); hub.bindClient(client); @@ -177,13 +148,12 @@ describe('startTranscation', () => { expect(transaction).toBeInstanceOf(NodeExperimentalTransaction); - expect(transaction.sampled).toBe(true); + expect(transaction.sampled).toBe(undefined); expect(transaction.spanRecorder).toBeDefined(); expect(transaction.spanRecorder?.spans).toHaveLength(1); expect(transaction.metadata).toEqual({ source: 'custom', spanMetadata: {}, - sampleRate: 1, }); expect(transaction.toJSON()).toEqual( @@ -210,8 +180,6 @@ describe('startTranscation', () => { expect(transaction).toBeInstanceOf(NodeExperimentalTransaction); - expect(transaction.sampled).toBe(false); - expect(transaction.spanRecorder).toBeUndefined(); expect(transaction.metadata).toEqual({ source: 'custom', spanMetadata: {}, @@ -226,20 +194,4 @@ describe('startTranscation', () => { }), ); }); - - it('inherits sampled based on parentSampled', () => { - const client = new NodeExperimentalClient(getDefaultNodeExperimentalClientOptions({ tracesSampleRate: 0 })); - const hub = getCurrentHub(); - hub.bindClient(client); - - const transaction = startTransaction(hub, { - name: 'test', - startTimestamp: 1234, - spanId: 'span1', - traceId: 'trace1', - parentSampled: true, - }); - - expect(transaction.sampled).toBe(true); - }); }); diff --git a/packages/node-experimental/test/utils/getActiveSpan.test.ts b/packages/node-experimental/test/utils/getActiveSpan.test.ts index 61b7d4f5d6c5..b97ced5bdbf8 100644 --- a/packages/node-experimental/test/utils/getActiveSpan.test.ts +++ b/packages/node-experimental/test/utils/getActiveSpan.test.ts @@ -1,15 +1,19 @@ import { trace } from '@opentelemetry/api'; import type { BasicTracerProvider } from '@opentelemetry/sdk-trace-base'; +import { NodeExperimentalClient } from '../../src/sdk/client'; import { setupOtel } from '../../src/sdk/initOtel'; import { getActiveSpan, getRootSpan } from '../../src/utils/getActiveSpan'; +import { getDefaultNodeExperimentalClientOptions } from '../helpers/getDefaultNodePreviewClientOptions'; import { cleanupOtel } from '../helpers/mockSdkInit'; describe('getActiveSpan', () => { let provider: BasicTracerProvider | undefined; beforeEach(() => { - provider = setupOtel(); + const options = getDefaultNodeExperimentalClientOptions(); + const client = new NodeExperimentalClient(options); + provider = setupOtel(client); }); afterEach(() => { @@ -93,7 +97,9 @@ describe('getRootSpan', () => { let provider: BasicTracerProvider | undefined; beforeEach(() => { - provider = setupOtel(); + const options = getDefaultNodeExperimentalClientOptions(); + const client = new NodeExperimentalClient(options); + provider = setupOtel(client); }); afterEach(async () => { diff --git a/packages/node-experimental/test/utils/setupEventContextTrace.test.ts b/packages/node-experimental/test/utils/setupEventContextTrace.test.ts index 390fa255a146..15d7f0976b9e 100644 --- a/packages/node-experimental/test/utils/setupEventContextTrace.test.ts +++ b/packages/node-experimental/test/utils/setupEventContextTrace.test.ts @@ -32,7 +32,7 @@ describe('setupEventContextTrace', () => { makeMain(hub); setupEventContextTrace(client); - provider = setupOtel(); + provider = setupOtel(client); }); afterEach(() => { diff --git a/packages/node-experimental/test/utils/spanTypes.test.ts b/packages/node-experimental/test/utils/spanTypes.test.ts index e4c2ca907ce9..fcd4703db9ce 100644 --- a/packages/node-experimental/test/utils/spanTypes.test.ts +++ b/packages/node-experimental/test/utils/spanTypes.test.ts @@ -31,7 +31,7 @@ describe('spanTypes', () => { it.each([ [{}, false], [{ kind: null }, false], - [{ kind: 'xxx' }, true], + [{ kind: 'TEST_KIND' }, true], ])('works with %p', (span, expected) => { const castSpan = span as unknown as Span; const actual = spanHasKind(castSpan); @@ -48,7 +48,7 @@ describe('spanTypes', () => { it.each([ [{}, false], [{ parentSpanId: null }, false], - [{ parentSpanId: 'xxx' }, true], + [{ parentSpanId: 'TEST_PARENT_ID' }, true], ])('works with %p', (span, expected) => { const castSpan = span as unknown as Span; const actual = spanHasParentId(castSpan);