Skip to content

Commit

Permalink
Extract into method and only start tracking in Sentry.init by default
Browse files Browse the repository at this point in the history
  • Loading branch information
lforst committed Jul 22, 2024
1 parent 2b03c5b commit b3008b6
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 23 deletions.
2 changes: 2 additions & 0 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,8 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
* Sends client reports as an envelope.
*/
protected _flushOutcomes(): void {
DEBUG_BUILD && logger.log('Flushing outcomes...');

const outcomes = this._clearOutcomes();

if (outcomes.length === 0) {
Expand Down
56 changes: 33 additions & 23 deletions packages/node/src/sdk/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
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 = {
Expand All @@ -33,28 +33,6 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
);

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. */
Expand Down Expand Up @@ -99,4 +77,36 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {

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);
}
}
}
2 changes: 2 additions & 0 deletions packages/node/src/sdk/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,8 @@ function _init(
startSessionTracking();
}

client.startClientReportTracking();

updateScopeFromEnvVariables();

if (options.spotlight) {
Expand Down

0 comments on commit b3008b6

Please sign in to comment.