From a7878948755945e236115092a438c298cabb0343 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Fri, 3 Sep 2021 15:33:07 +0200 Subject: [PATCH] feat(browser): Client Report Support --- packages/browser/src/backend.ts | 1 + packages/browser/src/sdk.ts | 3 + packages/browser/src/transports/base.ts | 74 +++++++++++++++ packages/browser/src/transports/fetch.ts | 63 ++++++++----- packages/browser/src/transports/xhr.ts | 58 +++++++----- .../browser/test/unit/transports/base.test.ts | 89 +++++++++++++++++++ .../test/unit/transports/fetch.test.ts | 48 ++++++++++ .../browser/test/unit/transports/xhr.test.ts | 45 ++++++++++ packages/core/src/baseclient.ts | 16 +++- packages/core/test/lib/base.test.ts | 78 +++++++++++++++- packages/node/src/client.ts | 2 +- packages/node/src/transports/base/index.ts | 3 - packages/tracing/src/transaction.ts | 7 ++ packages/tracing/test/idletransaction.test.ts | 21 ++++- packages/types/src/client.ts | 4 + packages/types/src/index.ts | 2 +- packages/types/src/options.ts | 6 ++ packages/types/src/request.ts | 1 + packages/types/src/transport.ts | 17 ++++ 19 files changed, 480 insertions(+), 58 deletions(-) diff --git a/packages/browser/src/backend.ts b/packages/browser/src/backend.ts index 3780ca4556ba..88d0bba7bc75 100644 --- a/packages/browser/src/backend.ts +++ b/packages/browser/src/backend.ts @@ -62,6 +62,7 @@ export class BrowserBackend extends BaseBackend { ...this._options.transportOptions, dsn: this._options.dsn, tunnel: this._options.tunnel, + sendClientReports: this._options.sendClientReports, _metadata: this._options._metadata, }; diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts index 1cf6b5958edf..518052ac71ad 100644 --- a/packages/browser/src/sdk.ts +++ b/packages/browser/src/sdk.ts @@ -88,6 +88,9 @@ export function init(options: BrowserOptions = {}): void { if (options.autoSessionTracking === undefined) { options.autoSessionTracking = true; } + if (options.sendClientReports === undefined) { + options.sendClientReports = true; + } initAndBind(BrowserClient, options); diff --git a/packages/browser/src/transports/base.ts b/packages/browser/src/transports/base.ts index e23425fde9c1..876a9c009fcc 100644 --- a/packages/browser/src/transports/base.ts +++ b/packages/browser/src/transports/base.ts @@ -1,6 +1,7 @@ import { API } from '@sentry/core'; import { Event, + Outcome, Response as SentryResponse, SentryRequestType, Status, @@ -34,10 +35,20 @@ export abstract class BaseTransport implements Transport { /** Locks transport after receiving rate limits in a response */ protected readonly _rateLimits: Record = {}; + protected _outcomes: { [key: string]: number } = {}; + public constructor(public options: TransportOptions) { this._api = new API(options.dsn, options._metadata, options.tunnel); // eslint-disable-next-line deprecation/deprecation this.url = this._api.getStoreEndpointWithUrlEncodedAuth(); + + if (this.options.sendClientReports) { + document.addEventListener('visibilitychange', () => { + if (document.visibilityState === 'hidden') { + this._flushOutcomes(); + } + }); + } } /** @@ -54,6 +65,69 @@ export abstract class BaseTransport implements Transport { return this._buffer.drain(timeout); } + /** + * @inheritDoc + */ + public recordLostEvent(reason: Outcome, category: SentryRequestType): void { + if (!this.options.sendClientReports) { + return; + } + // We want to track each category (event, transaction, session) separately + // but still keep the distinction between different type of outcomes. + // We could use nested maps, but it's much easier to read and type this way. + // A correct type for map-based implementation if we want to go that route + // would be `Partial>>>` + const key = `${CATEGORY_MAPPING[category]}:${reason}`; + logger.log(`Adding outcome: ${key}`); + this._outcomes[key] = (this._outcomes[key] ?? 0) + 1; + } + + /** + * Send outcomes as an envelope + */ + protected _flushOutcomes(): void { + if (!this.options.sendClientReports) { + return; + } + + if (!navigator || typeof navigator.sendBeacon !== 'function') { + logger.warn('Beacon API not available, skipping sending outcomes.'); + return; + } + + const outcomes = this._outcomes; + this._outcomes = {}; + + // Nothing to send + if (!Object.keys(outcomes).length) { + logger.log('No outcomes to flush'); + return; + } + + logger.log(`Flushing outcomes:\n${JSON.stringify(outcomes, null, 2)}`); + + const url = this._api.getEnvelopeEndpointWithUrlEncodedAuth(); + // Envelope header is required to be at least an empty object + const envelopeHeader = JSON.stringify({}); + const itemHeaders = JSON.stringify({ + type: 'client_report', + }); + const item = JSON.stringify({ + timestamp: Date.now(), + discarded_events: Object.keys(outcomes).map(key => { + const [category, reason] = key.split(':'); + return { + reason, + category, + quantity: outcomes[key], + }; + }), + }); + const envelope = `${envelopeHeader}\n${itemHeaders}\n${item}`; + + navigator.sendBeacon(url, envelope); + } + /** * Handle Sentry repsonse for promise-based transports. */ diff --git a/packages/browser/src/transports/fetch.ts b/packages/browser/src/transports/fetch.ts index 5c3c6ebd2b8c..a324ae3862ed 100644 --- a/packages/browser/src/transports/fetch.ts +++ b/packages/browser/src/transports/fetch.ts @@ -1,6 +1,13 @@ import { eventToSentryRequest, sessionToSentryRequest } from '@sentry/core'; -import { Event, Response, SentryRequest, Session, TransportOptions } from '@sentry/types'; -import { getGlobalObject, isNativeFetch, logger, supportsReferrerPolicy, SyncPromise } from '@sentry/utils'; +import { Event, Outcome, Response, SentryRequest, Session, TransportOptions } from '@sentry/types'; +import { + getGlobalObject, + isNativeFetch, + logger, + SentryError, + supportsReferrerPolicy, + SyncPromise, +} from '@sentry/utils'; import { BaseTransport } from './base'; @@ -106,6 +113,8 @@ export class FetchTransport extends BaseTransport { */ private _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike { if (this._isRateLimited(sentryRequest.type)) { + this.recordLostEvent(Outcome.RateLimitBackoff, sentryRequest.type); + return Promise.reject({ event: originalPayload, type: sentryRequest.type, @@ -132,25 +141,35 @@ export class FetchTransport extends BaseTransport { options.headers = this.options.headers; } - return this._buffer.add( - () => - new SyncPromise((resolve, reject) => { - void this._fetch(sentryRequest.url, options) - .then(response => { - const headers = { - 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), - 'retry-after': response.headers.get('Retry-After'), - }; - this._handleResponse({ - requestType: sentryRequest.type, - response, - headers, - resolve, - reject, - }); - }) - .catch(reject); - }), - ); + return this._buffer + .add( + () => + new SyncPromise((resolve, reject) => { + void this._fetch(sentryRequest.url, options) + .then(response => { + const headers = { + 'x-sentry-rate-limits': response.headers.get('X-Sentry-Rate-Limits'), + 'retry-after': response.headers.get('Retry-After'), + }; + this._handleResponse({ + requestType: sentryRequest.type, + response, + headers, + resolve, + reject, + }); + }) + .catch(reject); + }), + ) + .then(undefined, reason => { + // It's either buffer rejection or any other xhr/fetch error, which are treated as NetworkError. + if (reason instanceof SentryError) { + this.recordLostEvent(Outcome.QueueOverflow, sentryRequest.type); + } else { + this.recordLostEvent(Outcome.NetworkError, sentryRequest.type); + } + throw reason; + }); } } diff --git a/packages/browser/src/transports/xhr.ts b/packages/browser/src/transports/xhr.ts index e7f4c735baa1..8d142deb4cb1 100644 --- a/packages/browser/src/transports/xhr.ts +++ b/packages/browser/src/transports/xhr.ts @@ -1,6 +1,6 @@ import { eventToSentryRequest, sessionToSentryRequest } from '@sentry/core'; -import { Event, Response, SentryRequest, Session } from '@sentry/types'; -import { SyncPromise } from '@sentry/utils'; +import { Event, Outcome, Response, SentryRequest, Session } from '@sentry/types'; +import { SentryError, SyncPromise } from '@sentry/utils'; import { BaseTransport } from './base'; @@ -26,6 +26,8 @@ export class XHRTransport extends BaseTransport { */ private _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike { if (this._isRateLimited(sentryRequest.type)) { + this.recordLostEvent(Outcome.RateLimitBackoff, sentryRequest.type); + return Promise.reject({ event: originalPayload, type: sentryRequest.type, @@ -36,29 +38,39 @@ export class XHRTransport extends BaseTransport { }); } - return this._buffer.add( - () => - new SyncPromise((resolve, reject) => { - const request = new XMLHttpRequest(); + return this._buffer + .add( + () => + new SyncPromise((resolve, reject) => { + const request = new XMLHttpRequest(); - request.onreadystatechange = (): void => { - if (request.readyState === 4) { - const headers = { - 'x-sentry-rate-limits': request.getResponseHeader('X-Sentry-Rate-Limits'), - 'retry-after': request.getResponseHeader('Retry-After'), - }; - this._handleResponse({ requestType: sentryRequest.type, response: request, headers, resolve, reject }); - } - }; + request.onreadystatechange = (): void => { + if (request.readyState === 4) { + const headers = { + 'x-sentry-rate-limits': request.getResponseHeader('X-Sentry-Rate-Limits'), + 'retry-after': request.getResponseHeader('Retry-After'), + }; + this._handleResponse({ requestType: sentryRequest.type, response: request, headers, resolve, reject }); + } + }; - request.open('POST', sentryRequest.url); - for (const header in this.options.headers) { - if (this.options.headers.hasOwnProperty(header)) { - request.setRequestHeader(header, this.options.headers[header]); + request.open('POST', sentryRequest.url); + for (const header in this.options.headers) { + if (this.options.headers.hasOwnProperty(header)) { + request.setRequestHeader(header, this.options.headers[header]); + } } - } - request.send(sentryRequest.body); - }), - ); + request.send(sentryRequest.body); + }), + ) + .then(undefined, reason => { + // It's either buffer rejection or any other xhr/fetch error, which are treated as NetworkError. + if (reason instanceof SentryError) { + this.recordLostEvent(Outcome.QueueOverflow, sentryRequest.type); + } else { + this.recordLostEvent(Outcome.NetworkError, sentryRequest.type); + } + throw reason; + }); } } diff --git a/packages/browser/test/unit/transports/base.test.ts b/packages/browser/test/unit/transports/base.test.ts index 22a38b514135..5ae61a017249 100644 --- a/packages/browser/test/unit/transports/base.test.ts +++ b/packages/browser/test/unit/transports/base.test.ts @@ -1,10 +1,99 @@ +import { Outcome } from '@sentry/types'; + import { BaseTransport } from '../../../src/transports/base'; const testDsn = 'https://123@sentry.io/42'; +const envelopeEndpoint = 'https://sentry.io/api/42/envelope/?sentry_key=123&sentry_version=7'; class SimpleTransport extends BaseTransport {} describe('BaseTransport', () => { + describe('Client Reports', () => { + const sendBeaconSpy = jest.fn(); + let visibilityState: string; + + beforeAll(() => { + navigator.sendBeacon = sendBeaconSpy; + Object.defineProperty(document, 'visibilityState', { + configurable: true, + get: function() { + return visibilityState; + }, + }); + jest.spyOn(Date, 'now').mockImplementation(() => 1337); + }); + + beforeEach(() => { + sendBeaconSpy.mockClear(); + }); + + it('attaches visibilitychange handler if sendClientReport is set to true', () => { + const eventListenerSpy = jest.spyOn(document, 'addEventListener'); + new SimpleTransport({ dsn: testDsn, sendClientReports: true }); + expect(eventListenerSpy.mock.calls[0][0]).toBe('visibilitychange'); + eventListenerSpy.mockRestore(); + }); + + it('doesnt attach visibilitychange handler if sendClientReport is set to false', () => { + const eventListenerSpy = jest.spyOn(document, 'addEventListener'); + new SimpleTransport({ dsn: testDsn, sendClientReports: false }); + expect(eventListenerSpy).not.toHaveBeenCalled(); + eventListenerSpy.mockRestore(); + }); + + it('sends beacon request when there are outcomes captured and visibility changed to `hidden`', () => { + const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true }); + + transport.recordLostEvent(Outcome.BeforeSend, 'event'); + + visibilityState = 'hidden'; + document.dispatchEvent(new Event('visibilitychange')); + + const outcomes = [{ reason: Outcome.BeforeSend, category: 'error', quantity: 1 }]; + + expect(sendBeaconSpy).toHaveBeenCalledWith( + envelopeEndpoint, + `{}\n{"type":"client_report"}\n{"timestamp":1337,"discarded_events":${JSON.stringify(outcomes)}}`, + ); + }); + + it('doesnt send beacon request when there are outcomes captured, but visibility state did not change to `hidden`', () => { + const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true }); + transport.recordLostEvent(Outcome.BeforeSend, 'event'); + + visibilityState = 'visible'; + document.dispatchEvent(new Event('visibilitychange')); + + expect(sendBeaconSpy).not.toHaveBeenCalled(); + }); + + it('correctly serializes request with different categories/reasons pairs', () => { + const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true }); + + transport.recordLostEvent(Outcome.BeforeSend, 'event'); + transport.recordLostEvent(Outcome.BeforeSend, 'event'); + transport.recordLostEvent(Outcome.SampleRate, 'transaction'); + transport.recordLostEvent(Outcome.NetworkError, 'session'); + transport.recordLostEvent(Outcome.NetworkError, 'session'); + transport.recordLostEvent(Outcome.RateLimitBackoff, 'event'); + + visibilityState = 'hidden'; + document.dispatchEvent(new Event('visibilitychange')); + + const outcomes = [ + { reason: Outcome.BeforeSend, category: 'error', quantity: 2 }, + { reason: Outcome.SampleRate, category: 'transaction', quantity: 1 }, + { reason: Outcome.NetworkError, category: 'session', quantity: 2 }, + { reason: Outcome.RateLimitBackoff, category: 'error', quantity: 1 }, + ]; + + expect(sendBeaconSpy).toHaveBeenCalledWith( + envelopeEndpoint, + `{}\n{"type":"client_report"}\n{"timestamp":1337,"discarded_events":${JSON.stringify(outcomes)}}`, + ); + }); + }); + it('doesnt provide sendEvent() implementation', () => { const transport = new SimpleTransport({ dsn: testDsn }); diff --git a/packages/browser/test/unit/transports/fetch.test.ts b/packages/browser/test/unit/transports/fetch.test.ts index 7f084cb7aaf4..a6f39cfc8c97 100644 --- a/packages/browser/test/unit/transports/fetch.test.ts +++ b/packages/browser/test/unit/transports/fetch.test.ts @@ -1,3 +1,6 @@ +import { Outcome } from '@sentry/types'; +import { SentryError } from '@sentry/utils'; + import { Event, Response, Status, Transports } from '../../../src'; const testDsn = 'https://123@sentry.io/42'; @@ -104,6 +107,32 @@ describe('FetchTransport', () => { } }); + it('should record dropped event when fetch fails', async () => { + const response = { status: 403, headers: new Headers() }; + + window.fetch.mockImplementation(() => Promise.reject(response)); + + const spy = jest.spyOn(transport, 'recordLostEvent'); + + try { + await transport.sendEvent(eventPayload); + } catch (_) { + expect(spy).toHaveBeenCalledWith(Outcome.NetworkError, 'event'); + } + }); + + it('should record dropped event when queue buffer overflows', async () => { + // @ts-ignore private method + jest.spyOn(transport._buffer, 'add').mockRejectedValue(new SentryError('Buffer Full')); + const spy = jest.spyOn(transport, 'recordLostEvent'); + + try { + await transport.sendEvent(transactionPayload); + } catch (_) { + expect(spy).toHaveBeenCalledWith(Outcome.QueueOverflow, 'transaction'); + } + }); + it('passes in headers', async () => { transport = new Transports.FetchTransport( { @@ -451,6 +480,25 @@ describe('FetchTransport', () => { expect(eventRes.status).toBe(Status.Success); expect(fetch).toHaveBeenCalledTimes(2); }); + + it('should record dropped event', async () => { + // @ts-ignore private method + jest.spyOn(transport, '_isRateLimited').mockReturnValue(true); + + const spy = jest.spyOn(transport, 'recordLostEvent'); + + try { + await transport.sendEvent(eventPayload); + } catch (_) { + expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'event'); + } + + try { + await transport.sendEvent(transactionPayload); + } catch (_) { + expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'transaction'); + } + }); }); }); }); diff --git a/packages/browser/test/unit/transports/xhr.test.ts b/packages/browser/test/unit/transports/xhr.test.ts index a41406998e20..9d4291edd092 100644 --- a/packages/browser/test/unit/transports/xhr.test.ts +++ b/packages/browser/test/unit/transports/xhr.test.ts @@ -1,3 +1,5 @@ +import { Outcome } from '@sentry/types'; +import { SentryError } from '@sentry/utils'; import { fakeServer, SinonFakeServer } from 'sinon'; import { Event, Response, Status, Transports } from '../../../src'; @@ -69,6 +71,30 @@ describe('XHRTransport', () => { } }); + it('should record dropped event when request fails', async () => { + server.respondWith('POST', storeUrl, [403, {}, '']); + + const spy = jest.spyOn(transport, 'recordLostEvent'); + + try { + await transport.sendEvent(eventPayload); + } catch (_) { + expect(spy).toHaveBeenCalledWith(Outcome.NetworkError, 'event'); + } + }); + + it('should record dropped event when queue buffer overflows', async () => { + // @ts-ignore private method + jest.spyOn(transport._buffer, 'add').mockRejectedValue(new SentryError('Buffer Full')); + const spy = jest.spyOn(transport, 'recordLostEvent'); + + try { + await transport.sendEvent(transactionPayload); + } catch (_) { + expect(spy).toHaveBeenCalledWith(Outcome.QueueOverflow, 'transaction'); + } + }); + it('passes in headers', async () => { transport = new Transports.XHRTransport({ dsn: testDsn, @@ -380,6 +406,25 @@ describe('XHRTransport', () => { expect(eventRes.status).toBe(Status.Success); expect(server.requests.length).toBe(2); }); + + it('should record dropped event', async () => { + // @ts-ignore private method + jest.spyOn(transport, '_isRateLimited').mockReturnValue(true); + + const spy = jest.spyOn(transport, 'recordLostEvent'); + + try { + await transport.sendEvent(eventPayload); + } catch (_) { + expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'event'); + } + + try { + await transport.sendEvent(transactionPayload); + } catch (_) { + expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'transaction'); + } + }); }); }); }); diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 83aeb475a390..46d5d78eb1ae 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -7,8 +7,10 @@ import { Integration, IntegrationClass, Options, + Outcome, SessionStatus, Severity, + Transport, } from '@sentry/types'; import { dateTimestampInSeconds, @@ -181,13 +183,19 @@ export abstract class BaseClient implement return this._options; } + /** + * @inheritDoc + */ + public getTransport(): Transport { + return this._getBackend().getTransport(); + } + /** * @inheritDoc */ public flush(timeout?: number): PromiseLike { return this._isClientDoneProcessing(timeout).then(clientFinished => { - return this._getBackend() - .getTransport() + return this.getTransport() .close(timeout) .then(transportFlushed => clientFinished && transportFlushed); }); @@ -498,6 +506,7 @@ export abstract class BaseClient implement protected _processEvent(event: Event, hint?: EventHint, scope?: Scope): PromiseLike { // eslint-disable-next-line @typescript-eslint/unbound-method const { beforeSend, sampleRate } = this.getOptions(); + const transport = this.getTransport(); if (!this._isEnabled()) { return SyncPromise.reject(new SentryError('SDK not enabled, will not capture event.')); @@ -508,6 +517,7 @@ export abstract class BaseClient implement // 0.0 === 0% events are sent // Sampling for transaction happens somewhere else if (!isTransaction && typeof sampleRate === 'number' && Math.random() > sampleRate) { + transport.recordLostEvent?.(Outcome.SampleRate, 'event'); return SyncPromise.reject( new SentryError( `Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`, @@ -518,6 +528,7 @@ export abstract class BaseClient implement return this._prepareEvent(event, scope, hint) .then(prepared => { if (prepared === null) { + transport.recordLostEvent?.(Outcome.EventProcessor, event.type || 'event'); throw new SentryError('An event processor returned null, will not send event.'); } @@ -531,6 +542,7 @@ export abstract class BaseClient implement }) .then(processedEvent => { if (processedEvent === null) { + transport.recordLostEvent?.(Outcome.BeforeSend, event.type || 'event'); throw new SentryError('`beforeSend` returned `null`, will not send event.'); } diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index db9fbeb09664..64218d8ff97e 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1,5 +1,5 @@ import { Hub, Scope, Session } from '@sentry/hub'; -import { Event, Severity, Span } from '@sentry/types'; +import { Event, Outcome, Severity, Span, Transport } from '@sentry/types'; import { logger, SentryError, SyncPromise } from '@sentry/utils'; import * as integrationModule from '../../src/integration'; @@ -88,6 +88,16 @@ describe('BaseClient', () => { }); }); + describe('getTransport()', () => { + test('returns the transport from backend', () => { + expect.assertions(2); + const options = { dsn: PUBLIC_DSN, transport: FakeTransport }; + const client = new TestClient(options); + expect(client.getTransport()).toBeInstanceOf(FakeTransport); + expect(TestBackend.instance!.getTransport()).toBe(client.getTransport()); + }); + }); + describe('getBreadcrumbs() / addBreadcrumb()', () => { test('adds a breadcrumb', () => { expect.assertions(1); @@ -797,6 +807,28 @@ describe('BaseClient', () => { expect((TestBackend.instance!.event! as any).data).toBe('someRandomThing'); }); + test('beforeSend records dropped events', () => { + expect.assertions(1); + const client = new TestClient({ + dsn: PUBLIC_DSN, + beforeSend() { + return null; + }, + }); + + const recordLostEventSpy = jest.fn(); + jest.spyOn(client, 'getTransport').mockImplementationOnce( + () => + (({ + recordLostEvent: recordLostEventSpy, + } as any) as Transport), + ); + + client.captureEvent({ message: 'hello' }, {}); + + expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.BeforeSend, 'event'); + }); + test('eventProcessor can drop the even when it returns null', () => { expect.assertions(3); const client = new TestClient({ dsn: PUBLIC_DSN }); @@ -810,6 +842,25 @@ describe('BaseClient', () => { expect(loggerErrorSpy).toBeCalledWith(new SentryError('An event processor returned null, will not send event.')); }); + test('eventProcessor records dropped events', () => { + expect.assertions(1); + const client = new TestClient({ dsn: PUBLIC_DSN }); + + const recordLostEventSpy = jest.fn(); + jest.spyOn(client, 'getTransport').mockImplementationOnce( + () => + (({ + recordLostEvent: recordLostEventSpy, + } as any) as Transport), + ); + + const scope = new Scope(); + scope.addEventProcessor(() => null); + client.captureEvent({ message: 'hello' }, {}, scope); + + expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.EventProcessor, 'event'); + }); + test('eventProcessor sends an event and logs when it crashes', () => { expect.assertions(3); const client = new TestClient({ dsn: PUBLIC_DSN }); @@ -834,6 +885,25 @@ describe('BaseClient', () => { ), ); }); + + test('records events dropped due to sampleRate', () => { + expect.assertions(1); + const client = new TestClient({ + dsn: PUBLIC_DSN, + sampleRate: 0, + }); + + const recordLostEventSpy = jest.fn(); + jest.spyOn(client, 'getTransport').mockImplementationOnce( + () => + (({ + recordLostEvent: recordLostEventSpy, + } as any) as Transport), + ); + + client.captureEvent({ message: 'hello' }, {}); + expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.SampleRate, 'event'); + }); }); describe('integrations', () => { @@ -906,7 +976,7 @@ describe('BaseClient', () => { }); const delay = 1; - const transportInstance = (client as any)._getBackend().getTransport() as FakeTransport; + const transportInstance = client.getTransport() as FakeTransport; transportInstance.delay = delay; client.captureMessage('test'); @@ -935,7 +1005,7 @@ describe('BaseClient', () => { setTimeout(() => resolve({ message, level }), 150); }), ); - const transportInstance = (client as any)._getBackend().getTransport() as FakeTransport; + const transportInstance = client.getTransport() as FakeTransport; transportInstance.delay = delay; client.captureMessage('test async'); @@ -959,7 +1029,7 @@ describe('BaseClient', () => { }); const delay = 1; - const transportInstance = (client as any)._getBackend().getTransport() as FakeTransport; + const transportInstance = client.getTransport() as FakeTransport; transportInstance.delay = delay; expect(client.captureMessage('test')).toBeTruthy(); diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts index 75c86bd5c96d..7f4842b48ad4 100644 --- a/packages/node/src/client.ts +++ b/packages/node/src/client.ts @@ -98,7 +98,7 @@ export class NodeClient extends BaseClient { if (!release) { logger.warn('Cannot initialise an instance of SessionFlusher if no release is provided!'); } else { - this._sessionFlusher = new SessionFlusher(this._backend.getTransport(), { + this._sessionFlusher = new SessionFlusher(this.getTransport(), { release, environment, }); diff --git a/packages/node/src/transports/base/index.ts b/packages/node/src/transports/base/index.ts index 5868d47e9065..0cfa3f78943f 100644 --- a/packages/node/src/transports/base/index.ts +++ b/packages/node/src/transports/base/index.ts @@ -199,9 +199,6 @@ export abstract class BaseTransport implements Transport { }); } - if (!this._buffer.isReady()) { - return Promise.reject(new SentryError('Not adding Promise due to buffer limit reached.')); - } return this._buffer.add( () => new Promise((resolve, reject) => { diff --git a/packages/tracing/src/transaction.ts b/packages/tracing/src/transaction.ts index 32aefd43dd21..37befed513cb 100644 --- a/packages/tracing/src/transaction.ts +++ b/packages/tracing/src/transaction.ts @@ -2,6 +2,7 @@ import { getCurrentHub, Hub } from '@sentry/hub'; import { Event, Measurements, + Outcome, Transaction as TransactionInterface, TransactionContext, TransactionMetadata, @@ -102,6 +103,12 @@ export class Transaction extends SpanClass implements TransactionInterface { if (this.sampled !== true) { // At this point if `sampled !== true` we want to discard the transaction. logger.log('[Tracing] Discarding transaction because its trace was not chosen to be sampled.'); + + this._hub + .getClient() + ?.getTransport() + .recordLostEvent?.(Outcome.SampleRate, 'transaction'); + return undefined; } diff --git a/packages/tracing/test/idletransaction.test.ts b/packages/tracing/test/idletransaction.test.ts index d9a4f4cbe791..9934c8bbe187 100644 --- a/packages/tracing/test/idletransaction.test.ts +++ b/packages/tracing/test/idletransaction.test.ts @@ -1,13 +1,17 @@ -import { BrowserClient } from '@sentry/browser'; +import { BrowserClient, Transports } from '@sentry/browser'; import { Hub } from '@sentry/hub'; +import { Outcome } from '@sentry/types'; import { DEFAULT_IDLE_TIMEOUT, IdleTransaction, IdleTransactionSpanRecorder } from '../src/idletransaction'; import { Span } from '../src/span'; import { SpanStatus } from '../src/spanstatus'; +export class SimpleTransport extends Transports.BaseTransport {} + +const dsn = 'https://123@sentry.io/42'; let hub: Hub; beforeEach(() => { - hub = new Hub(new BrowserClient({ tracesSampleRate: 1 })); + hub = new Hub(new BrowserClient({ dsn, tracesSampleRate: 1, transport: SimpleTransport })); }); describe('IdleTransaction', () => { @@ -156,6 +160,19 @@ describe('IdleTransaction', () => { } }); + it('should record dropped transactions', async () => { + const transaction = new IdleTransaction({ name: 'foo', startTimestamp: 1234, sampled: false }, hub, 1000); + + const transport = hub.getClient()?.getTransport(); + + const spy = jest.spyOn(transport!, 'recordLostEvent'); + + transaction.initSpanRecorder(10); + transaction.finish(transaction.startTimestamp + 10); + + expect(spy).toHaveBeenCalledWith(Outcome.SampleRate, 'transaction'); + }); + describe('_initTimeout', () => { it('finishes if no activities are added to the transaction', () => { const transaction = new IdleTransaction({ name: 'foo', startTimestamp: 1234 }, hub, 1000); diff --git a/packages/types/src/client.ts b/packages/types/src/client.ts index d152136233e0..36f952524669 100644 --- a/packages/types/src/client.ts +++ b/packages/types/src/client.ts @@ -5,6 +5,7 @@ import { Options } from './options'; import { Scope } from './scope'; import { Session } from './session'; import { Severity } from './severity'; +import { Transport } from './transport'; /** * User-Facing Sentry SDK Client. @@ -59,6 +60,9 @@ export interface Client { /** Returns the current options. */ getOptions(): O; + /** Returns clients transport. */ + getTransport(): Transport; + /** * Flush the event queue and set the client to `enabled = false`. See {@link Client.flush}. * diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 9674be34db9b..2e67fc1e31f5 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -47,6 +47,6 @@ export { TransactionSamplingMethod, } from './transaction'; export { Thread } from './thread'; -export { Transport, TransportOptions, TransportClass } from './transport'; +export { Outcome, Transport, TransportOptions, TransportClass } from './transport'; export { User } from './user'; export { WrappedFunction } from './wrappedfunction'; diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts index 9b42c48ed0e8..196ca466ce73 100644 --- a/packages/types/src/options.ts +++ b/packages/types/src/options.ts @@ -132,6 +132,12 @@ export interface Options { */ autoSessionTracking?: boolean; + /** + * Send SDK Client Reports. + * By default, Client Reports are enabled. + */ + sendClientReports?: boolean; + /** * Initial data to populate scope. */ diff --git a/packages/types/src/request.ts b/packages/types/src/request.ts index 84b135fcfb3d..052de1b41c08 100644 --- a/packages/types/src/request.ts +++ b/packages/types/src/request.ts @@ -1,4 +1,5 @@ /** Possible SentryRequest types that can be used to make a distinction between Sentry features */ +// NOTE(kamil): It would be nice if we make it a valid enum instead export type SentryRequestType = 'event' | 'transaction' | 'session' | 'attachment'; /** A generic client request. */ diff --git a/packages/types/src/transport.ts b/packages/types/src/transport.ts index 182e08a137d6..40c26c146554 100644 --- a/packages/types/src/transport.ts +++ b/packages/types/src/transport.ts @@ -1,9 +1,19 @@ import { DsnLike } from './dsn'; import { Event } from './event'; +import { SentryRequestType } from './request'; import { Response } from './response'; import { SdkMetadata } from './sdkmetadata'; import { Session, SessionAggregates } from './session'; +export enum Outcome { + BeforeSend = 'before_send', + EventProcessor = 'event_processor', + NetworkError = 'network_error', + QueueOverflow = 'queue_overflow', + RateLimitBackoff = 'ratelimit_backoff', + SampleRate = 'sample_rate', +} + /** Transport used sending data to Sentry */ export interface Transport { /** @@ -29,6 +39,11 @@ export interface Transport { * still events in the queue when the timeout is reached. */ close(timeout?: number): PromiseLike; + + /** + * Increment the counter for the specific client outcome + */ + recordLostEvent?(type: Outcome, category: SentryRequestType): void; } /** JSDoc */ @@ -50,6 +65,8 @@ export interface TransportOptions { fetchParameters?: { [key: string]: string }; /** The envelope tunnel to use. */ tunnel?: string; + /** Send SDK Client Reports. Enabled by default. */ + sendClientReports?: boolean; /** * Set of metadata about the SDK that can be internally used to enhance envelopes and events, * and provide additional data about every request.