Skip to content

Commit 24f2193

Browse files
Lms24claude
andauthored
fix(core): Use trace_metric data category for metrics rate limits and client reports (#25606)
This PR changes the data category for metrics from `metric` to `trace_metric`. This fixes two issues: 1. Relay sends rate limits for metrics as `trace_metric`, which never matched our `metric` category, so the SDK kept sending metrics while rate limited. 2. Relay has no `metric` category, so it treated our metric client reports as `unknown`. The spec ([client reports for metrics](https://develop.sentry.dev/sdk/telemetry/metrics/)) requires `trace_metric` for both. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 32e7a35 commit 24f2193

5 files changed

Lines changed: 36 additions & 8 deletions

File tree

‎packages/core/src/metrics/internal.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,12 +191,12 @@ export function _INTERNAL_captureMetric(beforeMetric: Metric, options?: Internal
191191
: enrichedMetric;
192192

193193
if (processedMetric === CALLBACK_ERROR) {
194-
client.recordDroppedEvent('callback_error', 'metric', 1);
194+
client.recordDroppedEvent('callback_error', 'trace_metric', 1);
195195
return;
196196
}
197197

198198
if (!processedMetric) {
199-
client.recordDroppedEvent('before_send', 'metric', 1);
199+
client.recordDroppedEvent('before_send', 'trace_metric', 1);
200200
DEBUG_BUILD && debug.log('`beforeSendMetric` returned `null`, will not send metric.');
201201
return;
202202
}

‎packages/core/src/types/datacategory.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,6 @@ export type DataCategory =
3333
// Log bytes stored (unused for rate limiting)
3434
| 'log_byte'
3535
// Metric event
36-
| 'metric'
36+
| 'trace_metric'
3737
// Unknown data category
3838
| 'unknown';

‎packages/core/src/utils/envelope.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ export function createAttachmentEnvelopeItem(attachment: Attachment): Attachment
195195
type OverriddenItemType = Exclude<EnvelopeItemType, DataCategory>;
196196

197197
// Map of envelope item types to data categories where the category differs from the type.
198-
// Types that map to themselves (session, attachment, transaction, profile, feedback, span, metric) fall through.
198+
// Types that map to themselves (session, attachment, transaction, profile, feedback, span, trace_metric) fall through.
199199
const DATA_CATEGORY_OVERRIDES: Record<OverriddenItemType, DataCategory> = {
200200
sessions: 'session',
201201
event: 'error',
@@ -207,7 +207,7 @@ const DATA_CATEGORY_OVERRIDES: Record<OverriddenItemType, DataCategory> = {
207207
check_in: 'monitor',
208208
raw_security: 'security',
209209
log: 'log_item',
210-
trace_metric: 'metric',
210+
metric: 'trace_metric',
211211
};
212212

213213
function _isOverriddenType(type: EnvelopeItemType): type is OverriddenItemType {

‎packages/core/test/lib/metrics/internal.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ describe('_INTERNAL_captureMetric', () => {
358358
);
359359

360360
expect(beforeSendMetric).toHaveBeenCalled();
361-
expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'metric', 1);
361+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('before_send', 'trace_metric', 1);
362362
expect(loggerWarnSpy).toHaveBeenCalledWith('`beforeSendMetric` returned `null`, will not send metric.');
363363
expect(_INTERNAL_getMetricBuffer(client)).toBeUndefined();
364364

@@ -385,7 +385,7 @@ describe('_INTERNAL_captureMetric', () => {
385385
expect(() => _INTERNAL_captureMetric({ type: 'counter', name: 'test.metric', value: 1 }, { scope })).not.toThrow();
386386

387387
expect(beforeSendMetric).toHaveBeenCalled();
388-
expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'metric', 1);
388+
expect(recordDroppedEventSpy).toHaveBeenCalledWith('callback_error', 'trace_metric', 1);
389389
expect(debugErrorSpy).toHaveBeenCalledWith(
390390
'The `beforeSendMetric` callback threw an error, dropping the metric:',
391391
exception,

‎packages/core/test/lib/transports/base.test.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,34 @@ describe('createTransport', () => {
351351
expect(requestExecutor).not.toHaveBeenCalled();
352352
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'log_item', 3);
353353
});
354+
355+
it('back-off using X-Sentry-Rate-Limits with trace_metric category', async () => {
356+
const { retryAfterSeconds, beforeLimit, withinLimit } = setRateLimitTimes();
357+
const [transport, , requestExecutor, recordDroppedEventCallback] = createTestTransport({
358+
headers: {
359+
'x-sentry-rate-limits': `${retryAfterSeconds}:trace_metric:scope`,
360+
'retry-after': null,
361+
},
362+
});
363+
364+
const dateNowSpy = vi.spyOn(Date, 'now').mockImplementation(() => beforeLimit);
365+
366+
await transport.send(ERROR_ENVELOPE);
367+
requestExecutor.mockClear();
368+
369+
dateNowSpy.mockImplementation(() => withinLimit);
370+
371+
await transport.send(
372+
createEnvelope<Envelope>({ sent_at: '123' }, [
373+
[
374+
{ type: 'trace_metric', item_count: 2, content_type: 'application/vnd.sentry.items.trace-metric+json' },
375+
{ items: [] },
376+
],
377+
] as Envelope[1]),
378+
);
379+
expect(requestExecutor).not.toHaveBeenCalled();
380+
expect(recordDroppedEventCallback).toHaveBeenCalledWith('ratelimit_backoff', 'trace_metric', 2);
381+
});
354382
});
355383

356384
describe('Client Reports', () => {
@@ -479,7 +507,7 @@ describe('createTransport', () => {
479507
expect(mockRecordDroppedEventCallback).toHaveBeenCalledTimes(3);
480508
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'span', 5);
481509
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'log_item', 3);
482-
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'metric', 2);
510+
expect(mockRecordDroppedEventCallback).toHaveBeenCalledWith('send_error', 'trace_metric', 2);
483511
});
484512

485513
it('does not record outcomes for client reports when receiving 413', async () => {

0 commit comments

Comments
 (0)