diff --git a/public/app/core/services/echo/backends/sentry/SentryBackend.test.ts b/public/app/core/services/echo/backends/sentry/SentryBackend.test.ts index 249f200fe3a..059e93b9363 100644 --- a/public/app/core/services/echo/backends/sentry/SentryBackend.test.ts +++ b/public/app/core/services/echo/backends/sentry/SentryBackend.test.ts @@ -66,6 +66,11 @@ describe('SentryEchoBackend', () => { dsn: options.dsn, sampleRate: options.sampleRate, transport: EchoSrvTransport, + ignoreErrors: [ + 'ResizeObserver loop limit exceeded', + 'ResizeObserver loop completed', + 'Non-Error exception captured with keys', + ], }); expect(sentrySetUser).toHaveBeenCalledWith({ email: options.user?.email, diff --git a/public/app/core/services/echo/backends/sentry/SentryBackend.ts b/public/app/core/services/echo/backends/sentry/SentryBackend.ts index 1de214a8a22..33bb0b0eeec 100644 --- a/public/app/core/services/echo/backends/sentry/SentryBackend.ts +++ b/public/app/core/services/echo/backends/sentry/SentryBackend.ts @@ -35,6 +35,11 @@ export class SentryEchoBackend implements EchoBackend { @@ -53,7 +54,7 @@ describe('CustomEndpointTransport', () => { expect(fetchSpy).toHaveBeenCalledTimes(1); // second immediate call - shot circuited because retry-after time has not expired, backend not called - await expect(transport.sendEvent(event)).rejects.toBeTruthy(); + await expect(transport.sendEvent(event)).resolves.toHaveProperty('status', Status.Skipped); expect(fetchSpy).toHaveBeenCalledTimes(1); // wait out the retry-after and call again - great success @@ -61,4 +62,61 @@ describe('CustomEndpointTransport', () => { await expect(transport.sendEvent(event)).resolves.toBeTruthy(); expect(fetchSpy).toHaveBeenCalledTimes(2); }); + + it('will back off if backend returns Retry-After', async () => { + const rateLimiterResponse = { + status: 429, + ok: false, + headers: (new Headers({ + 'Retry-After': '1', // 1 second + }) as any) as Headers, + } as Response; + fetchSpy.mockResolvedValueOnce(rateLimiterResponse).mockResolvedValueOnce({ status: 200 } as Response); + const transport = new CustomEndpointTransport({ endpoint: '/log' }); + + // first call - backend is called, rejected because of 429 + await expect(transport.sendEvent(event)).rejects.toHaveProperty('status', 429); + expect(fetchSpy).toHaveBeenCalledTimes(1); + + // second immediate call - shot circuited because retry-after time has not expired, backend not called + await expect(transport.sendEvent(event)).resolves.toHaveProperty('status', Status.Skipped); + expect(fetchSpy).toHaveBeenCalledTimes(1); + + // wait out the retry-after and call again - great success + await new Promise((resolve) => setTimeout(() => resolve(null), 1001)); + await expect(transport.sendEvent(event)).resolves.toBeTruthy(); + expect(fetchSpy).toHaveBeenCalledTimes(2); + }); + + it('will drop events if max concurrency is reached', async () => { + const calls: Array<(value: unknown) => void> = []; + fetchSpy.mockImplementation( + () => + new Promise((resolve) => { + calls.push(resolve); + }) + ); + + const transport = new CustomEndpointTransport({ endpoint: '/log', maxConcurrentRequests: 2 }); + + // first two requests are accepted + transport.sendEvent(event); + const event2 = transport.sendEvent(event); + expect(calls).toHaveLength(2); + + // third is skipped because too many requests in flight + await expect(transport.sendEvent(event)).resolves.toHaveProperty('status', 'skipped'); + + expect(calls).toHaveLength(2); + + // after resolving in flight requests, next request is accepted as well + calls.forEach((call) => { + call({ status: 200 }); + }); + await event2; + const event3 = transport.sendEvent(event); + expect(calls).toHaveLength(3); + calls[2]({ status: 200 }); + await event3; + }); }); diff --git a/public/app/core/services/echo/backends/sentry/transports/CustomEndpointTransport.ts b/public/app/core/services/echo/backends/sentry/transports/CustomEndpointTransport.ts index 3e4efa51943..b591f7b6823 100644 --- a/public/app/core/services/echo/backends/sentry/transports/CustomEndpointTransport.ts +++ b/public/app/core/services/echo/backends/sentry/transports/CustomEndpointTransport.ts @@ -6,8 +6,11 @@ import { BaseTransport } from '../types'; export interface CustomEndpointTransportOptions { endpoint: string; fetchParameters?: Partial; + maxConcurrentRequests?: number; } +const DEFAULT_MAX_CONCURRENT_REQUESTS = 3; + /** * This is a copy of sentry's FetchTransport, edited to be able to push to any custom url * instead of using Sentry-specific endpoint logic. @@ -19,16 +22,20 @@ export class CustomEndpointTransport implements BaseTransport { /** Locks transport after receiving 429 response */ private _disabledUntil: Date = new Date(Date.now()); - private readonly _buffer: PromiseBuffer = new PromiseBuffer(30); + private readonly _buffer: PromiseBuffer; - constructor(public options: CustomEndpointTransportOptions) {} + constructor(public options: CustomEndpointTransportOptions) { + this._buffer = new PromiseBuffer(options.maxConcurrentRequests ?? DEFAULT_MAX_CONCURRENT_REQUESTS); + } sendEvent(event: Event): PromiseLike { if (new Date(Date.now()) < this._disabledUntil) { - return Promise.reject({ + const reason = `Dropping frontend event due to too many requests.`; + console.warn(reason); + return Promise.resolve({ event, - reason: `Transport locked till ${this._disabledUntil} due to too many requests.`, - status: 429, + reason, + status: Status.Skipped, }); } @@ -74,6 +81,16 @@ export class CustomEndpointTransport implements BaseTransport { Object.assign(options, this.options.fetchParameters); } + if (!this._buffer.isReady()) { + const reason = `Dropping frontend log event due to too many requests in flight.`; + console.warn(reason); + return Promise.resolve({ + event, + reason, + status: Status.Skipped, + }); + } + return this._buffer.add( () => new SyncPromise((resolve, reject) => {