From b97328e7634e01a009f69d0f6f7fd1cbe22782a4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 24 Jun 2024 10:26:23 +0000 Subject: [PATCH] Revert "Revert "feat(node): Allow to configure `maxSpanWaitDuration`" (#12511)" This reverts commit ff0d9c15c33fbccf00143f2e09da907c6fbd1f73. --- packages/node/src/sdk/initOtel.ts | 6 +- packages/node/src/types.ts | 11 ++++ .../test/integration/transactions.test.ts | 59 +++++++++++++++++++ packages/opentelemetry/src/spanExporter.ts | 7 ++- packages/opentelemetry/src/spanProcessor.ts | 4 +- 5 files changed, 82 insertions(+), 5 deletions(-) diff --git a/packages/node/src/sdk/initOtel.ts b/packages/node/src/sdk/initOtel.ts index 652c3826fa04..47c8879ae3e9 100644 --- a/packages/node/src/sdk/initOtel.ts +++ b/packages/node/src/sdk/initOtel.ts @@ -113,7 +113,11 @@ export function setupOtel(client: NodeClient): BasicTracerProvider { }), forceFlushTimeoutMillis: 500, }); - provider.addSpanProcessor(new SentrySpanProcessor()); + provider.addSpanProcessor( + new SentrySpanProcessor({ + timeout: client.getOptions().maxSpanWaitDuration, + }), + ); // Initialize the provider provider.register({ diff --git a/packages/node/src/types.ts b/packages/node/src/types.ts index d78e1761fd79..2c00302e2e64 100644 --- a/packages/node/src/types.ts +++ b/packages/node/src/types.ts @@ -74,6 +74,17 @@ export interface BaseNodeOptions { */ skipOpenTelemetrySetup?: boolean; + /** + * The max. duration in seconds that the SDK will wait for parent spans to be finished before discarding a span. + * The SDK will automatically clean up spans that have no finished parent after this duration. + * This is necessary to prevent memory leaks in case of parent spans that are never finished or otherwise dropped/missing. + * However, if you have very long-running spans in your application, a shorter duration might cause spans to be discarded too early. + * In this case, you can increase this duration to a value that fits your expected data. + * + * Defaults to 300 seconds (5 minutes). + */ + maxSpanWaitDuration?: number; + /** Callback that is executed when a fatal global error occurs. */ onFatalError?(this: void, error: Error): void; } diff --git a/packages/node/test/integration/transactions.test.ts b/packages/node/test/integration/transactions.test.ts index 75f5ef85a519..e3c9203ddaf5 100644 --- a/packages/node/test/integration/transactions.test.ts +++ b/packages/node/test/integration/transactions.test.ts @@ -633,4 +633,63 @@ describe('Integration | Transactions', () => { ]), ); }); + + it('allows to configure `maxSpanWaitDuration` to capture long running spans', async () => { + const transactions: TransactionEvent[] = []; + const beforeSendTransaction = jest.fn(event => { + transactions.push(event); + return null; + }); + + const now = Date.now(); + jest.useFakeTimers(); + jest.setSystemTime(now); + + const logs: unknown[] = []; + jest.spyOn(logger, 'log').mockImplementation(msg => logs.push(msg)); + + mockSdkInit({ + enableTracing: true, + beforeSendTransaction, + maxSpanWaitDuration: 100 * 60, + }); + + Sentry.startSpanManual({ name: 'test name' }, rootSpan => { + const subSpan = Sentry.startInactiveSpan({ name: 'inner span 1' }); + subSpan.end(); + + Sentry.startSpanManual({ name: 'inner span 2' }, innerSpan => { + // Child span ends after 10 min + setTimeout( + () => { + innerSpan.end(); + }, + 10 * 60 * 1_000, + ); + }); + + // root span ends after 99 min + setTimeout( + () => { + rootSpan.end(); + }, + 99 * 10 * 1_000, + ); + }); + + // Now wait for 100 mins + jest.advanceTimersByTime(100 * 60 * 1_000); + + expect(beforeSendTransaction).toHaveBeenCalledTimes(1); + expect(transactions).toHaveLength(1); + const transaction = transactions[0]!; + + expect(transaction.transaction).toEqual('test name'); + const spans = transaction.spans || []; + + expect(spans).toHaveLength(2); + + expect(spans[0]!.description).toEqual('inner span 1'); + expect(spans[1]!.description).toEqual('inner span 2'); + }); }); diff --git a/packages/opentelemetry/src/spanExporter.ts b/packages/opentelemetry/src/spanExporter.ts index 94e12bc019a8..d139044a713f 100644 --- a/packages/opentelemetry/src/spanExporter.ts +++ b/packages/opentelemetry/src/spanExporter.ts @@ -33,6 +33,7 @@ import { parseSpanDescription } from './utils/parseSpanDescription'; type SpanNodeCompleted = SpanNode & { span: ReadableSpan }; const MAX_SPAN_COUNT = 1000; +const DEFAULT_TIMEOUT = 300; // 5 min /** * A Sentry-specific exporter that converts OpenTelemetry Spans to Sentry Spans & Transactions. @@ -40,9 +41,11 @@ const MAX_SPAN_COUNT = 1000; export class SentrySpanExporter { private _flushTimeout: ReturnType | undefined; private _finishedSpans: ReadableSpan[]; + private _timeout: number; - public constructor() { + public constructor(options?: { timeout?: number }) { this._finishedSpans = []; + this._timeout = options?.timeout || DEFAULT_TIMEOUT; } /** Export a single span. */ @@ -103,7 +106,7 @@ export class SentrySpanExporter { */ private _cleanupOldSpans(spans = this._finishedSpans): void { this._finishedSpans = spans.filter(span => { - const shouldDrop = shouldCleanupSpan(span, 5 * 60); + const shouldDrop = shouldCleanupSpan(span, this._timeout); DEBUG_BUILD && shouldDrop && logger.log( diff --git a/packages/opentelemetry/src/spanProcessor.ts b/packages/opentelemetry/src/spanProcessor.ts index e3f604cab64b..71dab1359b94 100644 --- a/packages/opentelemetry/src/spanProcessor.ts +++ b/packages/opentelemetry/src/spanProcessor.ts @@ -65,9 +65,9 @@ function onSpanEnd(span: Span): void { export class SentrySpanProcessor implements SpanProcessorInterface { private _exporter: SentrySpanExporter; - public constructor() { + public constructor(options?: { timeout?: number }) { setIsSetup('SentrySpanProcessor'); - this._exporter = new SentrySpanExporter(); + this._exporter = new SentrySpanExporter(options); } /**