From 12962523aef647827262e96bd2769653a3b9ad41 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 17 Jul 2024 11:56:51 +0000 Subject: [PATCH 1/7] feat(node): Send client reports --- packages/browser/src/client.ts | 28 +--------------------------- packages/core/src/baseclient.ts | 28 ++++++++++++++++++++++++++++ packages/node/src/sdk/client.ts | 33 ++++++++++++++++++++++++++++++--- 3 files changed, 59 insertions(+), 30 deletions(-) diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index c301df98f7f6..177d787a438d 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -12,7 +12,7 @@ import type { SeverityLevel, UserFeedback, } from '@sentry/types'; -import { createClientReportEnvelope, dsnToString, getSDKSource, logger } from '@sentry/utils'; +import { getSDKSource, logger } from '@sentry/utils'; import { DEBUG_BUILD } from './debug-build'; import { eventFromException, eventFromMessage } from './eventbuilder'; @@ -118,30 +118,4 @@ export class BrowserClient extends BaseClient { event.platform = event.platform || 'javascript'; return super._prepareEvent(event, hint, scope); } - - /** - * Sends client reports as an envelope. - */ - private _flushOutcomes(): void { - const outcomes = this._clearOutcomes(); - - if (outcomes.length === 0) { - DEBUG_BUILD && logger.log('No outcomes to send'); - return; - } - - // This is really the only place where we want to check for a DSN and only send outcomes then - if (!this._dsn) { - DEBUG_BUILD && logger.log('No dsn provided, will not send outcomes'); - return; - } - - DEBUG_BUILD && logger.log('Sending outcomes:', outcomes); - - const envelope = createClientReportEnvelope(outcomes, this._options.tunnel && dsnToString(this._dsn)); - - // sendEnvelope should not throw - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.sendEnvelope(envelope); - } } diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 9c4b43bd9c9d..6dcfcf83720a 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -36,7 +36,9 @@ import { addItemToEnvelope, checkOrSetAlreadyCaught, createAttachmentEnvelopeItem, + createClientReportEnvelope, dropUndefinedKeys, + dsnToString, isParameterizedString, isPlainObject, isPrimitive, @@ -871,6 +873,32 @@ export abstract class BaseClient implements Client { }); } + /** + * Sends client reports as an envelope. + */ + protected _flushOutcomes(): void { + const outcomes = this._clearOutcomes(); + + if (outcomes.length === 0) { + DEBUG_BUILD && logger.log('No outcomes to send'); + return; + } + + // This is really the only place where we want to check for a DSN and only send outcomes then + if (!this._dsn) { + DEBUG_BUILD && logger.log('No dsn provided, will not send outcomes'); + return; + } + + DEBUG_BUILD && logger.log('Sending outcomes:', outcomes); + + const envelope = createClientReportEnvelope(outcomes, this._options.tunnel && dsnToString(this._dsn)); + + // sendEnvelope should not throw + // eslint-disable-next-line @typescript-eslint/no-floating-promises + this.sendEnvelope(envelope); + } + /** * @inheritDoc */ diff --git a/packages/node/src/sdk/client.ts b/packages/node/src/sdk/client.ts index cf1cb3c2023a..c7547cb4714a 100644 --- a/packages/node/src/sdk/client.ts +++ b/packages/node/src/sdk/client.ts @@ -6,12 +6,16 @@ import type { ServerRuntimeClientOptions } from '@sentry/core'; import { SDK_VERSION, ServerRuntimeClient, applySdkMetadata } from '@sentry/core'; import { logger } from '@sentry/utils'; import { isMainThread, threadId } from 'worker_threads'; +import { DEBUG_BUILD } from '../debug-build'; import type { NodeClientOptions } from '../types'; +const CLIENT_REPORT_FLUSH_INTERVAL_MS = 60_000; // 60s was chosen arbitrarily + /** A client for using Sentry with Node & OpenTelemetry. */ export class NodeClient extends ServerRuntimeClient { public traceProvider: BasicTracerProvider | undefined; private _tracer: Tracer | undefined; + private _clientReportInterval: NodeJS.Timeout | undefined; public constructor(options: NodeClientOptions) { const clientOptions: ServerRuntimeClientOptions = { @@ -28,6 +32,18 @@ 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, which may leak memory. In these situations, users are required to + // call `client.close()` in order to dispose of the client resource. + 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(); + } } /** Get the OTEL tracer. */ @@ -44,9 +60,8 @@ export class NodeClient extends ServerRuntimeClient { return tracer; } - /** - * @inheritDoc - */ + // Eslint ignore explanation: This is already documented in super. + // eslint-disable-next-line jsdoc/require-jsdoc public async flush(timeout?: number): Promise { const provider = this.traceProvider; const spanProcessor = provider?.activeSpanProcessor; @@ -55,6 +70,18 @@ export class NodeClient extends ServerRuntimeClient { await spanProcessor.forceFlush(); } + this._flushOutcomes(); + return super.flush(timeout); } + + // Eslint ignore explanation: This is already documented in super. + // eslint-disable-next-line jsdoc/require-jsdoc + public close(timeout?: number | undefined): PromiseLike { + if (this._clientReportInterval) { + clearInterval(this._clientReportInterval); + } + + return super.close(timeout); + } } From fadc347018248907b4f13115d7e22a0216268e74 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 17 Jul 2024 11:58:55 +0000 Subject: [PATCH 2/7] moar comment --- packages/node/src/sdk/client.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/node/src/sdk/client.ts b/packages/node/src/sdk/client.ts index c7547cb4714a..31bf3a1119c0 100644 --- a/packages/node/src/sdk/client.ts +++ b/packages/node/src/sdk/client.ts @@ -37,6 +37,10 @@ export class NodeClient extends ServerRuntimeClient { // There is one mild concern here, being that if users periodically and unboundedly create new clients, we will // create more and more intervals, which may leak memory. In these situations, users are required to // call `client.close()` in order to dispose of the client resource. + // 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(); From 2b03c5b0f0f11cc9856d8c0b1a662b59df4c89f4 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 17 Jul 2024 12:21:43 +0000 Subject: [PATCH 3/7] beforeExit --- packages/node/src/sdk/client.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/node/src/sdk/client.ts b/packages/node/src/sdk/client.ts index 31bf3a1119c0..b3b1098d7c6f 100644 --- a/packages/node/src/sdk/client.ts +++ b/packages/node/src/sdk/client.ts @@ -16,6 +16,7 @@ export class NodeClient extends ServerRuntimeClient { public traceProvider: BasicTracerProvider | undefined; private _tracer: Tracer | undefined; private _clientReportInterval: NodeJS.Timeout | undefined; + private _clientReportOnExitFlushListener: (() => undefined) | undefined; public constructor(options: NodeClientOptions) { const clientOptions: ServerRuntimeClientOptions = { @@ -35,8 +36,8 @@ export class NodeClient extends ServerRuntimeClient { 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, which may leak memory. In these situations, users are required to - // call `client.close()` in order to dispose of the client resource. + // 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 @@ -47,6 +48,12 @@ export class NodeClient extends ServerRuntimeClient { }, 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); } } @@ -86,6 +93,10 @@ export class NodeClient extends ServerRuntimeClient { clearInterval(this._clientReportInterval); } + if (this._clientReportOnExitFlushListener) { + process.off('beforeExit', this._clientReportOnExitFlushListener); + } + return super.close(timeout); } } From b3008b608bb481cf4dd5aac7f0778772462cc4d8 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 22 Jul 2024 09:00:18 +0000 Subject: [PATCH 4/7] Extract into method and only start tracking in Sentry.init by default --- packages/core/src/baseclient.ts | 2 ++ packages/node/src/sdk/client.ts | 56 +++++++++++++++++++-------------- packages/node/src/sdk/index.ts | 2 ++ 3 files changed, 37 insertions(+), 23 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..99824ffa7504 100644 --- a/packages/node/src/sdk/client.ts +++ b/packages/node/src/sdk/client.ts @@ -16,7 +16,7 @@ export class NodeClient extends ServerRuntimeClient { public traceProvider: BasicTracerProvider | undefined; private _tracer: Tracer | undefined; private _clientReportInterval: NodeJS.Timeout | undefined; - private _clientReportOnExitFlushListener: (() => undefined) | undefined; + private _clientReportOnExitFlushListener: (() => void) | undefined; public constructor(options: NodeClientOptions) { const clientOptions: ServerRuntimeClientOptions = { @@ -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) { From 5c9b8ddf39ae9e1b4e46c7305ca3ad2ef5319885 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 22 Jul 2024 09:07:57 +0000 Subject: [PATCH 5/7] Disable client reports for node unit tests --- packages/node/test/helpers/mockSdkInit.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/node/test/helpers/mockSdkInit.ts b/packages/node/test/helpers/mockSdkInit.ts index 0e1d23cfc73c..16dc16253a01 100644 --- a/packages/node/test/helpers/mockSdkInit.ts +++ b/packages/node/test/helpers/mockSdkInit.ts @@ -17,7 +17,14 @@ export function resetGlobals(): void { export function mockSdkInit(options?: Partial) { resetGlobals(); - init({ dsn: PUBLIC_DSN, defaultIntegrations: false, ...options }); + init({ + dsn: PUBLIC_DSN, + defaultIntegrations: false, + ...options, + // We are disabling client reports because we would be acquiring resources with every init call and that would leak + // memory every time we call init in the tests + sendClientReports: false, + }); } export function cleanupOtel(_provider?: BasicTracerProvider): void { From c5a477076ed5b07d93f93b00b165beb1c8a2fe52 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 22 Jul 2024 09:18:48 +0000 Subject: [PATCH 6/7] fix --- packages/node/src/sdk/client.ts | 4 +++- packages/node/test/helpers/mockSdkInit.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/node/src/sdk/client.ts b/packages/node/src/sdk/client.ts index 99824ffa7504..b459ee14fffc 100644 --- a/packages/node/src/sdk/client.ts +++ b/packages/node/src/sdk/client.ts @@ -59,7 +59,9 @@ export class NodeClient extends ServerRuntimeClient { await spanProcessor.forceFlush(); } - this._flushOutcomes(); + if (this.getOptions().sendClientReports !== false) { + this._flushOutcomes(); + } return super.flush(timeout); } diff --git a/packages/node/test/helpers/mockSdkInit.ts b/packages/node/test/helpers/mockSdkInit.ts index 16dc16253a01..e482dac6ed08 100644 --- a/packages/node/test/helpers/mockSdkInit.ts +++ b/packages/node/test/helpers/mockSdkInit.ts @@ -20,10 +20,10 @@ export function mockSdkInit(options?: Partial) { init({ dsn: PUBLIC_DSN, defaultIntegrations: false, - ...options, // We are disabling client reports because we would be acquiring resources with every init call and that would leak // memory every time we call init in the tests sendClientReports: false, + ...options, }); } From 3f51221beb606fe2d516035cb7190e7294ed0bc5 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Mon, 22 Jul 2024 12:10:12 +0000 Subject: [PATCH 7/7] tests --- .../drop-reasons/before-send/scenario.ts | 23 +++++++++++++ .../drop-reasons/before-send/test.ts | 32 +++++++++++++++++++ .../drop-reasons/event-processors/scenario.ts | 24 ++++++++++++++ .../drop-reasons/event-processors/test.ts | 32 +++++++++++++++++++ .../client-reports/periodic-send/scenario.ts | 13 ++++++++ .../client-reports/periodic-send/test.ts | 21 ++++++++++++ .../node-integration-tests/utils/runner.ts | 21 ++++++++++++ packages/node/src/sdk/client.ts | 18 +++++++---- packages/node/src/sdk/index.ts | 1 + packages/node/src/types.ts | 5 +++ packages/types/src/options.ts | 3 +- 11 files changed, 184 insertions(+), 9 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send/test.ts create mode 100644 dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processors/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processors/test.ts create mode 100644 dev-packages/node-integration-tests/suites/client-reports/periodic-send/scenario.ts create mode 100644 dev-packages/node-integration-tests/suites/client-reports/periodic-send/test.ts diff --git a/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send/scenario.ts b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send/scenario.ts new file mode 100644 index 000000000000..ab22aa289892 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send/scenario.ts @@ -0,0 +1,23 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +(async () => { + Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: loggingTransport, + beforeSend(event) { + return !event.type ? null : event; + }, + }); + + Sentry.captureException(new Error('this should get dropped by the event processor')); + + await Sentry.flush(); + + Sentry.captureException(new Error('this should get dropped by the event processor')); + Sentry.captureException(new Error('this should get dropped by the event processor')); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Sentry.flush(); +})(); diff --git a/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send/test.ts b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send/test.ts new file mode 100644 index 000000000000..363b8f268cd2 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/before-send/test.ts @@ -0,0 +1,32 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should record client report for beforeSend', done => { + createRunner(__dirname, 'scenario.ts') + .expect({ + client_report: { + discarded_events: [ + { + category: 'error', + quantity: 1, + reason: 'before_send', + }, + ], + }, + }) + .expect({ + client_report: { + discarded_events: [ + { + category: 'error', + quantity: 2, + reason: 'before_send', + }, + ], + }, + }) + .start(done); +}); diff --git a/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processors/scenario.ts b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processors/scenario.ts new file mode 100644 index 000000000000..2b188f8d71f3 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processors/scenario.ts @@ -0,0 +1,24 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +(async () => { + Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: loggingTransport, + }); + + Sentry.addEventProcessor(event => { + return !event.type ? null : event; + }); + + Sentry.captureException(new Error('this should get dropped by the event processor')); + + await Sentry.flush(); + + Sentry.captureException(new Error('this should get dropped by the event processor')); + Sentry.captureException(new Error('this should get dropped by the event processor')); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + Sentry.flush(); +})(); diff --git a/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processors/test.ts b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processors/test.ts new file mode 100644 index 000000000000..803f1c09bafe --- /dev/null +++ b/dev-packages/node-integration-tests/suites/client-reports/drop-reasons/event-processors/test.ts @@ -0,0 +1,32 @@ +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should record client report for event processors', done => { + createRunner(__dirname, 'scenario.ts') + .expect({ + client_report: { + discarded_events: [ + { + category: 'error', + quantity: 1, + reason: 'event_processor', + }, + ], + }, + }) + .expect({ + client_report: { + discarded_events: [ + { + category: 'error', + quantity: 2, + reason: 'event_processor', + }, + ], + }, + }) + .start(done); +}); diff --git a/dev-packages/node-integration-tests/suites/client-reports/periodic-send/scenario.ts b/dev-packages/node-integration-tests/suites/client-reports/periodic-send/scenario.ts new file mode 100644 index 000000000000..ff14911469ca --- /dev/null +++ b/dev-packages/node-integration-tests/suites/client-reports/periodic-send/scenario.ts @@ -0,0 +1,13 @@ +import { loggingTransport } from '@sentry-internal/node-integration-tests'; +import * as Sentry from '@sentry/node'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + transport: loggingTransport, + clientReportFlushInterval: 5000, + beforeSend(event) { + return !event.type ? null : event; + }, +}); + +Sentry.captureException(new Error('this should get dropped by before send')); diff --git a/dev-packages/node-integration-tests/suites/client-reports/periodic-send/test.ts b/dev-packages/node-integration-tests/suites/client-reports/periodic-send/test.ts new file mode 100644 index 000000000000..0364f3ea01f0 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/client-reports/periodic-send/test.ts @@ -0,0 +1,21 @@ +import { cleanupChildProcesses, createRunner } from '../../../utils/runner'; + +afterAll(() => { + cleanupChildProcesses(); +}); + +test('should flush client reports automatically after the timeout interval', done => { + createRunner(__dirname, 'scenario.ts') + .expect({ + client_report: { + discarded_events: [ + { + category: 'error', + quantity: 1, + reason: 'before_send', + }, + ], + }, + }) + .start(done); +}); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 6e663cd13d75..ae0451f0d576 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -3,6 +3,7 @@ import { spawn, spawnSync } from 'child_process'; import { join } from 'path'; import { SDK_VERSION } from '@sentry/node'; import type { + ClientReport, Envelope, EnvelopeItemType, Event, @@ -46,6 +47,12 @@ export function assertSentryCheckIn(actual: SerializedCheckIn, expected: Partial }); } +export function assertSentryClientReport(actual: ClientReport, expected: Partial): void { + expect(actual).toMatchObject({ + ...expected, + }); +} + export function assertEnvelopeHeader(actual: Envelope[0], expected: Partial): void { expect(actual).toEqual({ event_id: expect.any(String), @@ -148,6 +155,9 @@ type Expected = } | { check_in: Partial | ((event: SerializedCheckIn) => void); + } + | { + client_report: Partial | ((event: ClientReport) => void); }; type ExpectedEnvelopeHeader = @@ -332,6 +342,17 @@ export function createRunner(...paths: string[]) { expectCallbackCalled(); } + + if ('client_report' in expected) { + const clientReport = item[1] as ClientReport; + if (typeof expected.client_report === 'function') { + expected.client_report(clientReport); + } else { + assertSentryClientReport(clientReport, expected.client_report); + } + + expectCallbackCalled(); + } } catch (e) { complete(e as Error); } diff --git a/packages/node/src/sdk/client.ts b/packages/node/src/sdk/client.ts index b459ee14fffc..877b363d3b2a 100644 --- a/packages/node/src/sdk/client.ts +++ b/packages/node/src/sdk/client.ts @@ -9,7 +9,7 @@ import { isMainThread, threadId } from 'worker_threads'; import { DEBUG_BUILD } from '../debug-build'; import type { NodeClientOptions } from '../types'; -const CLIENT_REPORT_FLUSH_INTERVAL_MS = 60_000; // 60s was chosen arbitrarily +const DEFAULT_CLIENT_REPORT_FLUSH_INTERVAL_MS = 60_000; // 60s was chosen arbitrarily /** A client for using Sentry with Node & OpenTelemetry. */ export class NodeClient extends ServerRuntimeClient { @@ -59,7 +59,7 @@ export class NodeClient extends ServerRuntimeClient { await spanProcessor.forceFlush(); } - if (this.getOptions().sendClientReports !== false) { + if (this.getOptions().sendClientReports) { this._flushOutcomes(); } @@ -96,15 +96,19 @@ export class NodeClient extends ServerRuntimeClient { // 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) { + const clientOptions = this.getOptions(); + if (clientOptions.sendClientReports) { this._clientReportOnExitFlushListener = () => { this._flushOutcomes(); }; - this._clientReportInterval = setInterval(() => { - DEBUG_BUILD && logger.log('Flushing client reports based on interval.'); - this._flushOutcomes(); - }, CLIENT_REPORT_FLUSH_INTERVAL_MS) + this._clientReportInterval = setInterval( + () => { + DEBUG_BUILD && logger.log('Flushing client reports based on interval.'); + this._flushOutcomes(); + }, + clientOptions.clientReportFlushInterval ?? DEFAULT_CLIENT_REPORT_FLUSH_INTERVAL_MS, + ) // Unref is critical for not preventing the process from exiting because the interval is active. .unref(); diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index fcbf48c274d1..65a6f6768096 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -230,6 +230,7 @@ function getClientOptions( transport: makeNodeTransport, dsn: process.env.SENTRY_DSN, environment: process.env.SENTRY_ENVIRONMENT, + sendClientReports: true, }); const overwriteOptions = dropUndefinedKeys({ diff --git a/packages/node/src/types.ts b/packages/node/src/types.ts index 9cf3047e6c0a..dbdbdd1b1178 100644 --- a/packages/node/src/types.ts +++ b/packages/node/src/types.ts @@ -109,6 +109,11 @@ export interface BaseNodeOptions { */ registerEsmLoaderHooks?: boolean | EsmLoaderHookOptions; + /** + * Configures in which interval client reports will be flushed. Defaults to `60_000` (milliseconds). + */ + clientReportFlushInterval?: number; + /** Callback that is executed when a fatal global error occurs. */ onFatalError?(this: void, error: Error): void; } diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index d6c407d60bd0..82123c01a380 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -31,8 +31,7 @@ export interface ClientOptions