From 06a1d93dd80d1b752e8f50d2e4863111659f7c55 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 17 Jan 2025 11:10:14 +0000 Subject: [PATCH 1/3] Introduce failing test --- .../suites/sample-rand-propagation/server.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js b/dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js index 89ad5ef12f21..d7b54cb25c4f 100644 --- a/dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js +++ b/dev-packages/node-integration-tests/suites/sample-rand-propagation/server.js @@ -4,7 +4,7 @@ const Sentry = require('@sentry/node'); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', transport: loggingTransport, - tracesSampleRate: 1, + tracesSampleRate: 0.00000001, // It's important that this is not 1, so that we also check logic for NonRecordingSpans, which is usually the edge-case }); // express must be required after Sentry is initialized From 76aebf068638388aa942614d476490d77ed33fac Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Fri, 17 Jan 2025 12:00:09 +0000 Subject: [PATCH 2/3] fix --- .../src/tracing/dynamicSamplingContext.ts | 11 ++++++-- packages/opentelemetry/src/constants.ts | 1 + packages/opentelemetry/src/sampler.ts | 26 ++++++++++++++----- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index ba362c49795a..aba46d565c30 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -1,7 +1,7 @@ import type { Client } from '../client'; import { DEFAULT_ENVIRONMENT } from '../constants'; import { getClient } from '../currentScopes'; -import type { Scope } from '../scope'; +import { Scope } from '../scope'; import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import type { DynamicSamplingContext, Span } from '../types-hoist'; import { @@ -117,7 +117,14 @@ export function getDynamicSamplingContextFromSpan(span: Span): Readonly Date: Fri, 17 Jan 2025 12:17:23 +0000 Subject: [PATCH 3/3] tests n stuff --- .../error-active-span-unsampled/test.ts | 1 + .../core/src/tracing/dynamicSamplingContext.ts | 2 +- packages/opentelemetry/test/sampler.test.ts | 15 +++++++++------ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts index 8ac2dd53a089..6ba4aaa74b32 100644 --- a/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/envelope-header/error-active-span-unsampled/test.ts @@ -11,6 +11,7 @@ test('envelope header for error event during active unsampled span is correct', environment: 'production', release: '1.0', sampled: 'false', + sample_rand: expect.any(String), }, }, }) diff --git a/packages/core/src/tracing/dynamicSamplingContext.ts b/packages/core/src/tracing/dynamicSamplingContext.ts index aba46d565c30..5da5883f95ce 100644 --- a/packages/core/src/tracing/dynamicSamplingContext.ts +++ b/packages/core/src/tracing/dynamicSamplingContext.ts @@ -1,7 +1,7 @@ import type { Client } from '../client'; import { DEFAULT_ENVIRONMENT } from '../constants'; import { getClient } from '../currentScopes'; -import { Scope } from '../scope'; +import type { Scope } from '../scope'; import { SEMANTIC_ATTRIBUTE_SENTRY_SAMPLE_RATE, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '../semanticAttributes'; import type { DynamicSamplingContext, Span } from '../types-hoist'; import { diff --git a/packages/opentelemetry/test/sampler.test.ts b/packages/opentelemetry/test/sampler.test.ts index 9f6545bc14a3..fc38037a7ff2 100644 --- a/packages/opentelemetry/test/sampler.test.ts +++ b/packages/opentelemetry/test/sampler.test.ts @@ -26,11 +26,14 @@ describe('SentrySampler', () => { const links = undefined; const actual = sampler.shouldSample(ctx, traceId, spanName, spanKind, spanAttributes, links); - expect(actual).toEqual({ - decision: SamplingDecision.NOT_RECORD, - attributes: { 'sentry.sample_rate': 0 }, - traceState: new TraceState().set('sentry.sampled_not_recording', '1'), - }); + expect(actual).toEqual( + expect.objectContaining({ + decision: SamplingDecision.NOT_RECORD, + attributes: { 'sentry.sample_rate': 0 }, + }), + ); + expect(actual.traceState?.get('sentry.sampled_not_recording')).toBe('1'); + expect(actual.traceState?.get('sentry.sample_rand')).toEqual(expect.any(String)); expect(spyOnDroppedEvent).toHaveBeenCalledTimes(1); expect(spyOnDroppedEvent).toHaveBeenCalledWith('sample_rate', 'transaction'); @@ -81,7 +84,7 @@ describe('SentrySampler', () => { expect(actual).toEqual({ decision: SamplingDecision.RECORD_AND_SAMPLED, attributes: { 'sentry.sample_rate': 1 }, - traceState: new TraceState(), + traceState: expect.any(TraceState), }); expect(spyOnDroppedEvent).toHaveBeenCalledTimes(0);