Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(node): Send client reports #12951

Merged
merged 7 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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://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();
})();
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://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();
})();
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://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'));
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
Loading