Skip to content

Commit

Permalink
fix(node): Guard against invalid maxSpanWaitDuration values
Browse files Browse the repository at this point in the history
  • Loading branch information
mydea committed Dec 10, 2024
1 parent 4df242d commit 3a5bee0
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 1 deletion.
26 changes: 25 additions & 1 deletion packages/node/src/sdk/initOtel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
import { GLOBAL_OBJ, SDK_VERSION, consoleSandbox, logger } from '@sentry/core';
import { SentryPropagator, SentrySampler, SentrySpanProcessor } from '@sentry/opentelemetry';
import { createAddHookMessageChannel } from 'import-in-the-middle';
import { DEBUG_BUILD } from '../debug-build';
import { getOpenTelemetryInstrumentationToPreload } from '../integrations/tracing';
import { SentryContextManager } from '../otel/contextManager';
import type { EsmLoaderHookOptions } from '../types';
Expand All @@ -18,6 +19,9 @@ import type { NodeClient } from './client';

declare const __IMPORT_META_URL_REPLACEMENT__: string;

// About 277h - this must fit into new Array(len)!
const MAX_MAX_SPAN_WAIT_DURATION = 1_000_000;

/**
* Initialize OpenTelemetry for Node.
*/
Expand Down Expand Up @@ -138,7 +142,7 @@ export function setupOtel(client: NodeClient): BasicTracerProvider {
forceFlushTimeoutMillis: 500,
spanProcessors: [
new SentrySpanProcessor({
timeout: client.getOptions().maxSpanWaitDuration,
timeout: _getSpanProcessorTimeout(client.getOptions().maxSpanWaitDuration),
}),
],
});
Expand All @@ -152,6 +156,26 @@ export function setupOtel(client: NodeClient): BasicTracerProvider {
return provider;
}

/** Just exported for tests. */
export function _getSpanProcessorTimeout(maxSpanWaitDuration: number | undefined): number | undefined {
if (maxSpanWaitDuration == null) {
return undefined;
}

// We guard for a max. value here, because we create an array with this length
// So if this value is too large, this would fail
if (maxSpanWaitDuration > MAX_MAX_SPAN_WAIT_DURATION) {
DEBUG_BUILD &&
logger.warn(`\`maxSpanWaitDuration\` is too high, using the maximum value of ${MAX_MAX_SPAN_WAIT_DURATION}`);
return MAX_MAX_SPAN_WAIT_DURATION;
} else if (maxSpanWaitDuration <= 0 || Number.isNaN(maxSpanWaitDuration)) {
DEBUG_BUILD && logger.warn('`maxSpanWaitDuration` must be a positive number, using default value instead.');
return undefined;
}

return maxSpanWaitDuration;
}

/**
* Setup the OTEL logger to use our own logger.
*/
Expand Down
78 changes: 78 additions & 0 deletions packages/node/test/sdk/initOtel.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
import { logger } from '@sentry/core';
import { _getSpanProcessorTimeout } from '../../src/sdk/initOtel';

describe('_getSpanProcessorTimeout', () => {
beforeEach(() => {
jest.clearAllMocks();
});

it('works with undefined', () => {
const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
const timeout = _getSpanProcessorTimeout(undefined);
expect(timeout).toBe(undefined);
expect(loggerWarnSpy).not.toHaveBeenCalled();
});

it('works with positive number', () => {
const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
const timeout = _getSpanProcessorTimeout(10);
expect(timeout).toBe(10);
expect(loggerWarnSpy).not.toHaveBeenCalled();
});

it('works with 0', () => {
const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
const timeout = _getSpanProcessorTimeout(0);
expect(timeout).toBe(undefined);
expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
expect(loggerWarnSpy).toHaveBeenCalledWith(
'`maxSpanWaitDuration` must be a positive number, using default value instead.',
);
});

it('works with negative number', () => {
const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
const timeout = _getSpanProcessorTimeout(-10);
expect(timeout).toBe(undefined);
expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
expect(loggerWarnSpy).toHaveBeenCalledWith(
'`maxSpanWaitDuration` must be a positive number, using default value instead.',
);
});

it('works with -Infinity', () => {
const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
const timeout = _getSpanProcessorTimeout(-Infinity);
expect(timeout).toBe(undefined);
expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
expect(loggerWarnSpy).toHaveBeenCalledWith(
'`maxSpanWaitDuration` must be a positive number, using default value instead.',
);
});

it('works with Infinity', () => {
const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
const timeout = _getSpanProcessorTimeout(Infinity);
expect(timeout).toBe(1_000_000);
expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
expect(loggerWarnSpy).toHaveBeenCalledWith('`maxSpanWaitDuration` is too high, using the maximum value of 1000000');
});

it('works with large number', () => {
const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
const timeout = _getSpanProcessorTimeout(1_000_000_000);
expect(timeout).toBe(1_000_000);
expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
expect(loggerWarnSpy).toHaveBeenCalledWith('`maxSpanWaitDuration` is too high, using the maximum value of 1000000');
});

it('works with NaN', () => {
const loggerWarnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {});
const timeout = _getSpanProcessorTimeout(NaN);
expect(timeout).toBe(undefined);
expect(loggerWarnSpy).toHaveBeenCalledTimes(1);
expect(loggerWarnSpy).toHaveBeenCalledWith(
'`maxSpanWaitDuration` must be a positive number, using default value instead.',
);
});
});

0 comments on commit 3a5bee0

Please sign in to comment.