From 342777700d6a76c5ae0ce8675d390a58ba9ef476 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 29 Jul 2024 09:40:26 +0200 Subject: [PATCH 1/2] fix(node): Improve OTEL validation logic --- packages/node/src/sdk/index.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index 65a6f6768096..cab3ac8274d1 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -195,7 +195,12 @@ export function validateOpenTelemetrySetup(): void { const setup = openTelemetrySetupCheck(); - const required = ['SentrySpanProcessor', 'SentryContextManager', 'SentryPropagator'] as const; + const required: ReturnType = ['SentryContextManager', 'SentryPropagator']; + + if (hasTracingEnabled()) { + required.push('SentrySpanProcessor'); + } + for (const k of required) { if (!setup.includes(k)) { logger.error( @@ -206,7 +211,7 @@ export function validateOpenTelemetrySetup(): void { if (!setup.includes('SentrySampler')) { logger.warn( - 'You have to set up the SentrySampler. Without this, the OpenTelemetry & Sentry integration may still work, but sample rates set for the Sentry SDK will not be respected.', + 'You have to set up the SentrySampler. Without this, the OpenTelemetry & Sentry integration may still work, but sample rates set for the Sentry SDK will not be respected. If you use a custom sampler, make sure to use `wrapSamplingDecision`.', ); } } From 1550bce70d1aeae84426ab41c8c830b63ef8a097 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Mon, 29 Jul 2024 10:21:12 +0200 Subject: [PATCH 2/2] add test --- packages/node/test/sdk/init.test.ts | 66 ++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/packages/node/test/sdk/init.test.ts b/packages/node/test/sdk/init.test.ts index 49246f4284ae..1f904ca528f9 100644 --- a/packages/node/test/sdk/init.test.ts +++ b/packages/node/test/sdk/init.test.ts @@ -1,8 +1,10 @@ import type { Integration } from '@sentry/types'; +import { logger } from '@sentry/utils'; +import * as SentryOpentelemetry from '@sentry/opentelemetry'; import { getClient, getIsolationScope } from '../../src/'; import * as auto from '../../src/integrations/tracing'; -import { init } from '../../src/sdk'; +import { init, validateOpenTelemetrySetup } from '../../src/sdk'; import { NodeClient } from '../../src/sdk/client'; import { cleanupOtel } from '../helpers/mockSdkInit'; @@ -193,3 +195,65 @@ describe('init()', () => { }); }); }); + +describe('validateOpenTelemetrySetup', () => { + afterEach(() => { + global.__SENTRY__ = {}; + cleanupOtel(); + jest.clearAllMocks(); + }); + + it('works with correct setup', () => { + const errorSpy = jest.spyOn(logger, 'error').mockImplementation(() => {}); + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {}); + + jest.spyOn(SentryOpentelemetry, 'openTelemetrySetupCheck').mockImplementation(() => { + return ['SentryContextManager', 'SentryPropagator', 'SentrySampler']; + }); + + validateOpenTelemetrySetup(); + + expect(errorSpy).toHaveBeenCalledTimes(0); + expect(warnSpy).toHaveBeenCalledTimes(0); + }); + + it('works with missing setup, without tracing', () => { + const errorSpy = jest.spyOn(logger, 'error').mockImplementation(() => {}); + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {}); + + jest.spyOn(SentryOpentelemetry, 'openTelemetrySetupCheck').mockImplementation(() => { + return []; + }); + + validateOpenTelemetrySetup(); + + // Without tracing, this is expected only twice + expect(errorSpy).toHaveBeenCalledTimes(2); + expect(warnSpy).toHaveBeenCalledTimes(1); + + expect(errorSpy).toBeCalledWith(expect.stringContaining('You have to set up the SentryContextManager.')); + expect(errorSpy).toBeCalledWith(expect.stringContaining('You have to set up the SentryPropagator.')); + expect(warnSpy).toBeCalledWith(expect.stringContaining('You have to set up the SentrySampler.')); + }); + + it('works with missing setup, with tracing', () => { + const errorSpy = jest.spyOn(logger, 'error').mockImplementation(() => {}); + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {}); + + jest.spyOn(SentryOpentelemetry, 'openTelemetrySetupCheck').mockImplementation(() => { + return []; + }); + + init({ dsn: PUBLIC_DSN, skipOpenTelemetrySetup: true, tracesSampleRate: 1 }); + + validateOpenTelemetrySetup(); + + expect(errorSpy).toHaveBeenCalledTimes(3); + expect(warnSpy).toHaveBeenCalledTimes(1); + + expect(errorSpy).toBeCalledWith(expect.stringContaining('You have to set up the SentryContextManager.')); + expect(errorSpy).toBeCalledWith(expect.stringContaining('You have to set up the SentryPropagator.')); + expect(errorSpy).toBeCalledWith(expect.stringContaining('You have to set up the SentrySpanProcessor.')); + expect(warnSpy).toBeCalledWith(expect.stringContaining('You have to set up the SentrySampler.')); + }); +});