From cb11d4c6d5b31b9c9f568f07b1bc6d982d15e5dc Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 22 Jul 2024 08:52:38 +0000 Subject: [PATCH] Extract into method and only start tracking in Sentry.init by default --- packages/core/src/baseclient.ts | 2 ++ packages/node/src/sdk/client.ts | 54 +++++++++++++++++++-------------- packages/node/src/sdk/index.ts | 2 ++ 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 6dcfcf83720a..5122b66d3267 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -877,6 +877,8 @@ export abstract class BaseClient implements Client { * Sends client reports as an envelope. */ protected _flushOutcomes(): void { + DEBUG_BUILD && logger.log('Flushing outcomes...'); + const outcomes = this._clearOutcomes(); if (outcomes.length === 0) { diff --git a/packages/node/src/sdk/client.ts b/packages/node/src/sdk/client.ts index b3b1098d7c6f..ff3fc5587688 100644 --- a/packages/node/src/sdk/client.ts +++ b/packages/node/src/sdk/client.ts @@ -33,28 +33,6 @@ export class NodeClient extends ServerRuntimeClient { ); super(clientOptions); - - if (clientOptions.sendClientReports !== false) { - // There is one mild concern here, being that if users periodically and unboundedly create new clients, we will - // create more and more intervals and beforeExit listeners, which may leak memory. In these situations, users are - // required to call `client.close()` in order to dispose of the acquired resources. - // Users are already confronted with the same reality with the SessionFlusher at the time of writing this so the - // working theory is that this should be fine. - // Note: We have experimented with using `FinalizationRegisty` to clear the interval when the client is garbage - // collected, but it did not work, because the cleanup function never got called. - this._clientReportInterval = setInterval(() => { - DEBUG_BUILD && logger.log('Flushing client reports based on interval.'); - this._flushOutcomes(); - }, CLIENT_REPORT_FLUSH_INTERVAL_MS) - // Unref is critical, otherwise we stop the process from exiting by itself - .unref(); - - this._clientReportOnExitFlushListener = () => { - this._flushOutcomes(); - }; - - process.on('beforeExit', this._clientReportOnExitFlushListener); - } } /** Get the OTEL tracer. */ @@ -99,4 +77,36 @@ export class NodeClient extends ServerRuntimeClient { return super.close(timeout); } + + /** + * Will start tracking client reports for this client. + * + * NOTICE: This method will create an interval that is periodically called and attach a `process.on('beforeExit')` + * hook. To clean up these resources, call `.close()` when you no longer intend to use the client. Not doing so will + * result in a memory leak. + */ + // The reason client reports need to be manually activated with this method instead of just enabling them in a + // constructor, is that if users periodically and unboundedly create new clients, we will create more and more + // intervals and beforeExit listeners, thus leaking memory. In these situations, users are required to call + // `client.close()` in order to dispose of the acquired resources. + // We assume that calling this method in Sentry.init() is a sensible default, because calling Sentry.init() over and + // over again would also result in memory leaks. + // Note: We have experimented with using `FinalizationRegisty` to clear the interval when the client is garbage + // collected, but it did not work, because the cleanup function never got called. + public startClientReportTracking(): void { + if (this.getOptions().sendClientReports !== false) { + this._clientReportOnExitFlushListener = () => { + this._flushOutcomes(); + }; + + this._clientReportInterval = setInterval(() => { + DEBUG_BUILD && logger.log('Flushing client reports based on interval.'); + this._flushOutcomes(); + }, CLIENT_REPORT_FLUSH_INTERVAL_MS) + // Unref is critical for not preventing the process from exiting because the interval is active. + .unref(); + + process.on('beforeExit', this._clientReportOnExitFlushListener); + } + } } diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index 7dd145854993..fcbf48c274d1 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -154,6 +154,8 @@ function _init( startSessionTracking(); } + client.startClientReportTracking(); + updateScopeFromEnvVariables(); if (options.spotlight) {