Skip to content

Commit

Permalink
feat(node): Send client reports (#12951)
Browse files Browse the repository at this point in the history
  • Loading branch information
lforst authored Jul 22, 2024
1 parent 81e20c3 commit 849d1cf
Show file tree
Hide file tree
Showing 14 changed files with 275 additions and 33 deletions.
Original file line number Diff line number Diff line change
@@ -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://[email protected]/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();
})();
Original file line number Diff line number Diff line change
@@ -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);
});
Original file line number Diff line number Diff line change
@@ -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://[email protected]/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();
})();
Original file line number Diff line number Diff line change
@@ -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);
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { loggingTransport } from '@sentry-internal/node-integration-tests';
import * as Sentry from '@sentry/node';

Sentry.init({
dsn: 'https://[email protected]/1337',
transport: loggingTransport,
clientReportFlushInterval: 5000,
beforeSend(event) {
return !event.type ? null : event;
},
});

Sentry.captureException(new Error('this should get dropped by before send'));
Original file line number Diff line number Diff line change
@@ -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);
});
21 changes: 21 additions & 0 deletions dev-packages/node-integration-tests/utils/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -46,6 +47,12 @@ export function assertSentryCheckIn(actual: SerializedCheckIn, expected: Partial
});
}

export function assertSentryClientReport(actual: ClientReport, expected: Partial<ClientReport>): void {
expect(actual).toMatchObject({
...expected,
});
}

export function assertEnvelopeHeader(actual: Envelope[0], expected: Partial<Envelope[0]>): void {
expect(actual).toEqual({
event_id: expect.any(String),
Expand Down Expand Up @@ -148,6 +155,9 @@ type Expected =
}
| {
check_in: Partial<SerializedCheckIn> | ((event: SerializedCheckIn) => void);
}
| {
client_report: Partial<ClientReport> | ((event: ClientReport) => void);
};

type ExpectedEnvelopeHeader =
Expand Down Expand Up @@ -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);
}
Expand Down
28 changes: 1 addition & 27 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -118,30 +118,4 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
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);
}
}
30 changes: 30 additions & 0 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ import {
addItemToEnvelope,
checkOrSetAlreadyCaught,
createAttachmentEnvelopeItem,
createClientReportEnvelope,
dropUndefinedKeys,
dsnToString,
isParameterizedString,
isPlainObject,
isPrimitive,
Expand Down Expand Up @@ -871,6 +873,34 @@ 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) {
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
*/
Expand Down
64 changes: 61 additions & 3 deletions packages/node/src/sdk/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@ 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 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<NodeClientOptions> {
public traceProvider: BasicTracerProvider | undefined;
private _tracer: Tracer | undefined;
private _clientReportInterval: NodeJS.Timeout | undefined;
private _clientReportOnExitFlushListener: (() => void) | undefined;

public constructor(options: NodeClientOptions) {
const clientOptions: ServerRuntimeClientOptions = {
Expand Down Expand Up @@ -44,9 +49,8 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
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<boolean> {
const provider = this.traceProvider;
const spanProcessor = provider?.activeSpanProcessor;
Expand All @@ -55,6 +59,60 @@ export class NodeClient extends ServerRuntimeClient<NodeClientOptions> {
await spanProcessor.forceFlush();
}

if (this.getOptions().sendClientReports) {
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<boolean> {
if (this._clientReportInterval) {
clearInterval(this._clientReportInterval);
}

if (this._clientReportOnExitFlushListener) {
process.off('beforeExit', this._clientReportOnExitFlushListener);
}

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 {
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();
},
clientOptions.clientReportFlushInterval ?? DEFAULT_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);
}
}
}
3 changes: 3 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 Expand Up @@ -228,6 +230,7 @@ function getClientOptions(
transport: makeNodeTransport,
dsn: process.env.SENTRY_DSN,
environment: process.env.SENTRY_ENVIRONMENT,
sendClientReports: true,
});

const overwriteOptions = dropUndefinedKeys({
Expand Down
Loading

0 comments on commit 849d1cf

Please sign in to comment.