From 5ab3631269c229e0ffbbb0c802554e0bb374f5e4 Mon Sep 17 00:00:00 2001 From: Abhijeet Prasad Date: Thu, 4 Apr 2024 15:05:15 -0400 Subject: [PATCH] fix(core): unref timer to not block node exit (#11430) Add `unref` to make sure that the session flusher or metrics aggregator does not block nodejs exit. Especially important for serverless scenarios. --- .../suites/metrics/should-exit-forced.js | 19 +++++++++++++++++ .../suites/metrics/should-exit.js | 18 ++++++++++++++++ .../suites/metrics/test.ts | 21 +++++++++++++++++++ .../tracing/metric-summaries/scenario.js | 3 --- packages/core/src/metrics/aggregator.ts | 12 +++++++++-- packages/core/src/sessionflusher.ts | 11 ++++++++-- 6 files changed, 77 insertions(+), 7 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/metrics/should-exit-forced.js create mode 100644 dev-packages/node-integration-tests/suites/metrics/should-exit.js create mode 100644 dev-packages/node-integration-tests/suites/metrics/test.ts diff --git a/dev-packages/node-integration-tests/suites/metrics/should-exit-forced.js b/dev-packages/node-integration-tests/suites/metrics/should-exit-forced.js new file mode 100644 index 000000000000..2621828973ab --- /dev/null +++ b/dev-packages/node-integration-tests/suites/metrics/should-exit-forced.js @@ -0,0 +1,19 @@ +const Sentry = require('@sentry/node'); + +function configureSentry() { + Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + autoSessionTracking: false, + }); + + Sentry.metrics.increment('test'); +} + +async function main() { + configureSentry(); + await new Promise(resolve => setTimeout(resolve, 1000)); + process.exit(0); +} + +main(); diff --git a/dev-packages/node-integration-tests/suites/metrics/should-exit.js b/dev-packages/node-integration-tests/suites/metrics/should-exit.js new file mode 100644 index 000000000000..01a6f0194507 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/metrics/should-exit.js @@ -0,0 +1,18 @@ +const Sentry = require('@sentry/node'); + +function configureSentry() { + Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + autoSessionTracking: false, + }); + + Sentry.metrics.increment('test'); +} + +async function main() { + configureSentry(); + await new Promise(resolve => setTimeout(resolve, 1000)); +} + +main(); diff --git a/dev-packages/node-integration-tests/suites/metrics/test.ts b/dev-packages/node-integration-tests/suites/metrics/test.ts new file mode 100644 index 000000000000..2c3cc350eeba --- /dev/null +++ b/dev-packages/node-integration-tests/suites/metrics/test.ts @@ -0,0 +1,21 @@ +import { createRunner } from '../../utils/runner'; + +describe('metrics', () => { + test('should exit', done => { + const runner = createRunner(__dirname, 'should-exit.js').start(); + + setTimeout(() => { + expect(runner.childHasExited()).toBe(true); + done(); + }, 5_000); + }); + + test('should exit forced', done => { + const runner = createRunner(__dirname, 'should-exit-forced.js').start(); + + setTimeout(() => { + expect(runner.childHasExited()).toBe(true); + done(); + }, 5_000); + }); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js index ef68afb06576..dfeb13c17b9b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing/metric-summaries/scenario.js @@ -11,9 +11,6 @@ Sentry.init({ }, }); -// Stop the process from exiting before the transaction is sent -setInterval(() => {}, 1000); - Sentry.startSpan( { name: 'Test Transaction', diff --git a/packages/core/src/metrics/aggregator.ts b/packages/core/src/metrics/aggregator.ts index 2b331082ab3e..4bf66f099887 100644 --- a/packages/core/src/metrics/aggregator.ts +++ b/packages/core/src/metrics/aggregator.ts @@ -25,7 +25,9 @@ export class MetricsAggregator implements MetricsAggregatorBase { // that we store in memory. private _bucketsTotalWeight; - private readonly _interval: ReturnType; + // Cast to any so that it can use Node.js timeout + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private readonly _interval: any; // SDKs are required to shift the flush interval by random() * rollup_in_seconds. // That shift is determined once per startup to create jittering. @@ -42,7 +44,13 @@ export class MetricsAggregator implements MetricsAggregatorBase { public constructor(private readonly _client: Client) { this._buckets = new Map(); this._bucketsTotalWeight = 0; - this._interval = setInterval(() => this._flush(), DEFAULT_FLUSH_INTERVAL); + + this._interval = setInterval(() => this._flush(), DEFAULT_FLUSH_INTERVAL) as any; + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + if (this._interval.unref) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + this._interval.unref(); + } this._flushShift = Math.floor((Math.random() * DEFAULT_FLUSH_INTERVAL) / 1000); this._forceFlush = false; } diff --git a/packages/core/src/sessionflusher.ts b/packages/core/src/sessionflusher.ts index dac81b82336d..40679f99e7f0 100644 --- a/packages/core/src/sessionflusher.ts +++ b/packages/core/src/sessionflusher.ts @@ -20,7 +20,9 @@ export class SessionFlusher implements SessionFlusherLike { public readonly flushTimeout: number; private _pendingAggregates: Record; private _sessionAttrs: ReleaseHealthAttributes; - private _intervalId: ReturnType; + // Cast to any so that it can use Node.js timeout + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private _intervalId: any; private _isEnabled: boolean; private _client: Client; @@ -30,8 +32,13 @@ export class SessionFlusher implements SessionFlusherLike { this._pendingAggregates = {}; this._isEnabled = true; - // Call to setInterval, so that flush is called every 60 seconds + // Call to setInterval, so that flush is called every 60 seconds. this._intervalId = setInterval(() => this.flush(), this.flushTimeout * 1000); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + if (this._intervalId.unref) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access + this._intervalId.unref(); + } this._sessionAttrs = attrs; }