From cbfd97e3ebd0571470874f3eebd58c20f1e95dc1 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 8 Oct 2026 15:46:46 +0200 Subject: [PATCH 1/2] fix(core): Record actual item counts in client reports for failed envelopes When sending an envelope failed, each item was recorded with quantity 1. Span, log and metric containers now report their item_count, and transactions also report spans.length + 1 dropped spans. Co-Authored-By: Claude Opus 5.5 (1M context) --- packages/core/src/transports/base.ts | 16 +++++++- .../core/test/lib/transports/base.test.ts | 40 +++++++++++++++---- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index 45553115fa64..c39b1ca942e3 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -1,6 +1,7 @@ import { DEBUG_BUILD } from '../debug-build'; import type { EventDropReason } from '../types/clientreport'; import type { Envelope, EnvelopeItem } from '../types/envelope'; +import type { Event } from '../types/event'; import type { InternalBaseTransportOptions, Transport, @@ -63,8 +64,19 @@ export function createTransport( DEBUG_BUILD && debug.warn(`Dropping client report. Will not send outcomes (reason: ${reason}).`); return; } - forEachEnvelopeItem(filteredEnvelope, (item, type) => { - options.recordDroppedEvent(reason, envelopeItemTypeToDataCategory(type)); + forEachEnvelopeItem(filteredEnvelope, ([headers, payload], type) => { + const dataCategory = envelopeItemTypeToDataCategory(type); + if (type === 'transaction') { + options.recordDroppedEvent(reason, dataCategory); + // The transaction itself counts as a span, too + options.recordDroppedEvent(reason, 'span', ((payload as Event).spans?.length ?? 0) + 1); + } else { + options.recordDroppedEvent( + reason, + dataCategory, + typeof headers.item_count === 'number' ? headers.item_count : 1, + ); + } }); }; diff --git a/packages/core/test/lib/transports/base.test.ts b/packages/core/test/lib/transports/base.test.ts index 9a2d53582686..bfd15ce4f16e 100644 --- a/packages/core/test/lib/transports/base.test.ts +++ b/packages/core/test/lib/transports/base.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it, vi } from 'vitest'; import { createTransport } from '../../../src/transports/base'; import type { ClientReport } from '../../../src/types/clientreport'; -import type { AttachmentItem, EventEnvelope, EventItem } from '../../../src/types/envelope'; +import type { AttachmentItem, Envelope, EventEnvelope, EventItem } from '../../../src/types/envelope'; import type { TransportMakeRequestResponse } from '../../../src/types/transport'; import { createClientReportEnvelope } from '../../../src/utils/clientreport'; import { createEnvelope, serializeEnvelope } from '../../../src/utils/envelope'; @@ -388,7 +388,7 @@ describe('createTransport', () => { } // recordDroppedEvent SHOULD be called for regular events - expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('network_error', 'error'); + expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('network_error', 'error', 1); }); }); @@ -405,7 +405,7 @@ describe('createTransport', () => { // Should resolve without throwing expect(result).toEqual({ statusCode: 413 }); // recordDroppedEvent SHOULD be called with send_error reason - expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error'); + expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error', 1); }); it('should record send_error for each item in envelope when receiving 413', async () => { @@ -415,7 +415,7 @@ describe('createTransport', () => { { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2', sent_at: '123' }, [ [{ type: 'event' }, { event_id: 'aa3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, - [{ type: 'transaction' }, { event_id: 'bb3ff046696b4bc6b609ce6d28fde9e2' }] as EventItem, + [{ type: 'transaction' }, { event_id: 'bb3ff046696b4bc6b609ce6d28fde9e2', spans: [{}, {}] }] as EventItem, ], ); @@ -425,10 +425,34 @@ describe('createTransport', () => { await transport.send(multiItemEnvelope); - // recordDroppedEvent SHOULD be called for each item - expect(mockRecordDroppedEventCallback).toHaveBeenCalledTimes(2); - expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error'); + expect(mockRecordDroppedEventCallback).toHaveBeenCalledTimes(3); + expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error', 1); expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'transaction'); + expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'span', 3); + }); + + it('should record send_error with the item count of container items when receiving 413', async () => { + const mockRecordDroppedEventCallback = vi.fn(); + + const containerEnvelope = createEnvelope({ sent_at: '123' }, [ + [{ type: 'span', item_count: 5, content_type: 'application/vnd.sentry.items.span.v2+json' }, { items: [] }], + [{ type: 'log', item_count: 3, content_type: 'application/vnd.sentry.items.log+json' }, { items: [] }], + [ + { type: 'trace_metric', item_count: 2, content_type: 'application/vnd.sentry.items.trace-metric+json' }, + { items: [] }, + ], + ] as Envelope[1]); + + const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, () => + resolvedSyncPromise({ statusCode: 413 }), + ); + + await transport.send(containerEnvelope); + + expect(mockRecordDroppedEventCallback).toHaveBeenCalledTimes(3); + expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'span', 5); + expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'log_item', 3); + expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'metric', 2); }); it('should not record outcomes for client reports when receiving 413', async () => { @@ -455,7 +479,7 @@ describe('createTransport', () => { // First request gets 413 await transport.send(ERROR_ENVELOPE); expect(mockRequestExecutor).toHaveBeenCalledTimes(1); - expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error'); + expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error', 1); mockRequestExecutor.mockClear(); mockRecordDroppedEventCallback.mockClear(); From 3de69e62d099179ac9f368dad886338f952ba216 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Thu, 8 Oct 2026 15:53:41 +0200 Subject: [PATCH 2/2] also handle ratelimit_backoff --- packages/core/src/transports/base.ts | 33 ++++++------ .../core/test/lib/transports/base.test.ts | 53 ++++++++++++++----- 2 files changed, 56 insertions(+), 30 deletions(-) diff --git a/packages/core/src/transports/base.ts b/packages/core/src/transports/base.ts index c39b1ca942e3..86c52037d6f7 100644 --- a/packages/core/src/transports/base.ts +++ b/packages/core/src/transports/base.ts @@ -1,6 +1,6 @@ import { DEBUG_BUILD } from '../debug-build'; import type { EventDropReason } from '../types/clientreport'; -import type { Envelope, EnvelopeItem } from '../types/envelope'; +import type { Envelope, EnvelopeItem, EnvelopeItemType } from '../types/envelope'; import type { Event } from '../types/event'; import type { InternalBaseTransportOptions, @@ -37,14 +37,24 @@ export function createTransport( let rateLimits: RateLimits = {}; const flush = (timeout?: number): PromiseLike => buffer.drain(timeout); + function recordItemLoss(reason: EventDropReason, [headers, payload]: EnvelopeItem, type: EnvelopeItemType): void { + const dataCategory = envelopeItemTypeToDataCategory(type); + if (type === 'transaction') { + options.recordDroppedEvent(reason, dataCategory); + // The transaction itself counts as a span, too + options.recordDroppedEvent(reason, 'span', ((payload as Event).spans?.length ?? 0) + 1); + } else { + options.recordDroppedEvent(reason, dataCategory, typeof headers.item_count === 'number' ? headers.item_count : 1); + } + } + function send(envelope: Envelope): PromiseLike { const filteredEnvelopeItems: EnvelopeItem[] = []; // Drop rate limited items from envelope forEachEnvelopeItem(envelope, (item, type) => { - const dataCategory = envelopeItemTypeToDataCategory(type); - if (isRateLimited(rateLimits, dataCategory)) { - options.recordDroppedEvent('ratelimit_backoff', dataCategory); + if (isRateLimited(rateLimits, envelopeItemTypeToDataCategory(type))) { + recordItemLoss('ratelimit_backoff', item, type); } else { filteredEnvelopeItems.push(item); } @@ -64,19 +74,8 @@ export function createTransport( DEBUG_BUILD && debug.warn(`Dropping client report. Will not send outcomes (reason: ${reason}).`); return; } - forEachEnvelopeItem(filteredEnvelope, ([headers, payload], type) => { - const dataCategory = envelopeItemTypeToDataCategory(type); - if (type === 'transaction') { - options.recordDroppedEvent(reason, dataCategory); - // The transaction itself counts as a span, too - options.recordDroppedEvent(reason, 'span', ((payload as Event).spans?.length ?? 0) + 1); - } else { - options.recordDroppedEvent( - reason, - dataCategory, - typeof headers.item_count === 'number' ? headers.item_count : 1, - ); - } + forEachEnvelopeItem(filteredEnvelope, (item, type) => { + recordItemLoss(reason, item, type); }); }; diff --git a/packages/core/test/lib/transports/base.test.ts b/packages/core/test/lib/transports/base.test.ts index bfd15ce4f16e..9a2a0ef6275b 100644 --- a/packages/core/test/lib/transports/base.test.ts +++ b/packages/core/test/lib/transports/base.test.ts @@ -157,7 +157,7 @@ describe('createTransport', () => { await transport.send(ERROR_ENVELOPE); expect(requestExecutor).not.toHaveBeenCalled(); - expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', 1); requestExecutor.mockClear(); recordDroppedEventCallback.mockClear(); @@ -199,7 +199,7 @@ describe('createTransport', () => { await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit expect(requestExecutor).not.toHaveBeenCalled(); - expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', 1); requestExecutor.mockClear(); recordDroppedEventCallback.mockClear(); @@ -242,18 +242,19 @@ describe('createTransport', () => { await transport.send(TRANSACTION_ENVELOPE); // Transaction envelope should not be sent because of pending rate limit expect(requestExecutor).not.toHaveBeenCalled(); expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'span', 1); requestExecutor.mockClear(); recordDroppedEventCallback.mockClear(); await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit expect(requestExecutor).not.toHaveBeenCalled(); - expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', 1); requestExecutor.mockClear(); recordDroppedEventCallback.mockClear(); await transport.send(ATTACHMENT_ENVELOPE); // Attachment envelope should not be sent because of pending rate limit expect(requestExecutor).not.toHaveBeenCalled(); - expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'attachment'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'attachment', 1); requestExecutor.mockClear(); recordDroppedEventCallback.mockClear(); @@ -302,12 +303,13 @@ describe('createTransport', () => { await transport.send(TRANSACTION_ENVELOPE); // Transaction envelope should not be sent because of pending rate limit expect(requestExecutor).not.toHaveBeenCalled(); expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'transaction'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'span', 1); requestExecutor.mockClear(); recordDroppedEventCallback.mockClear(); await transport.send(ERROR_ENVELOPE); // Error envelope should not be sent because of pending rate limit expect(requestExecutor).not.toHaveBeenCalled(); - expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error'); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'error', 1); requestExecutor.mockClear(); recordDroppedEventCallback.mockClear(); @@ -324,10 +326,35 @@ describe('createTransport', () => { expect(requestExecutor).toHaveBeenCalledTimes(1); expect(recordDroppedEventCallback).not.toHaveBeenCalled(); }); + + it('records the item count of rate limited container items', async () => { + const { retryAfterSeconds, beforeLimit, withinLimit } = setRateLimitTimes(); + const [transport, , requestExecutor, recordDroppedEventCallback] = createTestTransport({ + headers: { + 'x-sentry-rate-limits': `${retryAfterSeconds}:log_item:scope`, + 'retry-after': null, + }, + }); + + const dateNowSpy = vi.spyOn(Date, 'now').mockImplementation(() => beforeLimit); + + await transport.send(ERROR_ENVELOPE); + requestExecutor.mockClear(); + + dateNowSpy.mockImplementation(() => withinLimit); + + await transport.send( + createEnvelope({ sent_at: '123' }, [ + [{ type: 'log', item_count: 3, content_type: 'application/vnd.sentry.items.log+json' }, { items: [] }], + ] as Envelope[1]), + ); + expect(requestExecutor).not.toHaveBeenCalled(); + expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'log_item', 3); + }); }); describe('Client Reports', () => { - it('should not record outcomes when client reports fail to send', async () => { + it('does not record outcomes when client reports fail to send', async () => { expect.assertions(2); const mockRecordDroppedEventCallback = vi.fn(); @@ -347,7 +374,7 @@ describe('createTransport', () => { expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled(); }); - it('should not record outcomes when client reports fail due to buffer overflow', async () => { + it('does not record outcomes when client reports fail due to buffer overflow', async () => { expect.assertions(2); const mockRecordDroppedEventCallback = vi.fn(); @@ -371,7 +398,7 @@ describe('createTransport', () => { expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled(); }); - it('should record outcomes when regular events fail to send', async () => { + it('records outcomes when regular events fail to send', async () => { expect.assertions(2); const mockRecordDroppedEventCallback = vi.fn(); @@ -393,7 +420,7 @@ describe('createTransport', () => { }); describe('HTTP 413 Content Too Large', () => { - it('should record send_error outcome when receiving 413 response', async () => { + it('records send_error outcome when receiving 413 response', async () => { const mockRecordDroppedEventCallback = vi.fn(); const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, () => @@ -408,7 +435,7 @@ describe('createTransport', () => { expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'error', 1); }); - it('should record send_error for each item in envelope when receiving 413', async () => { + it('records send_error for each item in envelope when receiving 413', async () => { const mockRecordDroppedEventCallback = vi.fn(); const multiItemEnvelope = createEnvelope( @@ -431,7 +458,7 @@ describe('createTransport', () => { expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'span', 3); }); - it('should record send_error with the item count of container items when receiving 413', async () => { + it('records send_error with the item count of container items when receiving 413', async () => { const mockRecordDroppedEventCallback = vi.fn(); const containerEnvelope = createEnvelope({ sent_at: '123' }, [ @@ -455,7 +482,7 @@ describe('createTransport', () => { expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'metric', 2); }); - it('should not record outcomes for client reports when receiving 413', async () => { + it('does not record outcomes for client reports when receiving 413', async () => { const mockRecordDroppedEventCallback = vi.fn(); const transport = createTransport({ recordDroppedEvent: mockRecordDroppedEventCallback }, () => @@ -470,7 +497,7 @@ describe('createTransport', () => { expect(mockRecordDroppedEventCallback).not.toHaveBeenCalled(); }); - it('should not apply rate limits after receiving 413', async () => { + it('does not apply rate limits after receiving 413', async () => { const mockRecordDroppedEventCallback = vi.fn(); const mockRequestExecutor = vi.fn(() => resolvedSyncPromise({ statusCode: 413 }));