Skip to content

Commit 791454c

Browse files
committed
feat(core): enforce metric drop limit with in-flight tracking
Track in-flight metrics when flushing and include them in the drop limit check to prevent unbounded growth when network sends are slower than metric production.
1 parent 5c5c7d4 commit 791454c

File tree

4 files changed

+114
-1
lines changed

4 files changed

+114
-1
lines changed

packages/core/src/carrier.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,12 @@ export interface SentryCarrier {
3939
*/
4040
clientToMetricBufferMap?: WeakMap<Client, Array<SerializedMetric>>;
4141

42+
/**
43+
* A map of Sentry clients to the number of metrics currently in-flight (flushed but not yet sent/completed).
44+
* This is used to track metrics that have been flushed but are still waiting for network requests to complete.
45+
*/
46+
clientToInFlightMetricsMap?: WeakMap<Client, number>;
47+
4248
/** Overwrites TextEncoder used in `@sentry/core`, need for `react-native@0.73` and older */
4349
encodePolyfill?: (input: string) => Uint8Array;
4450
/** Overwrites TextDecoder used in `@sentry/core`, need for `react-native@0.73` and older */

packages/core/src/metrics/internal.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { _getTraceInfoFromScope } from '../utils/trace-info';
1313
import { createMetricEnvelope } from './envelope';
1414

1515
const MAX_METRIC_BUFFER_SIZE = 1000;
16+
const DEFAULT_MAX_METRIC_DROP_LIMIT = 2000;
1617

1718
/**
1819
* Converts a metric attribute to a serialized metric attribute.
@@ -89,10 +90,19 @@ function setMetricAttribute(
8990
export function _INTERNAL_captureSerializedMetric(client: Client, serializedMetric: SerializedMetric): void {
9091
const bufferMap = _getBufferMap();
9192
const metricBuffer = _INTERNAL_getMetricBuffer(client);
93+
const maxDropLimit = client.getOptions().maxMetricDropLimit ?? DEFAULT_MAX_METRIC_DROP_LIMIT;
94+
const inFlightCount = _getInFlightCount(client);
9295

9396
if (metricBuffer === undefined) {
9497
bufferMap.set(client, [serializedMetric]);
9598
} else {
99+
const totalMetrics = metricBuffer.length + inFlightCount;
100+
101+
if (totalMetrics >= maxDropLimit && maxDropLimit > 0) {
102+
client.recordDroppedEvent('buffer_overflow', 'metric');
103+
return;
104+
}
105+
96106
if (metricBuffer.length >= MAX_METRIC_BUFFER_SIZE) {
97107
_INTERNAL_flushMetricsBuffer(client, metricBuffer);
98108
bufferMap.set(client, [serializedMetric]);
@@ -266,14 +276,18 @@ export function _INTERNAL_flushMetricsBuffer(client: Client, maybeMetricBuffer?:
266276
const clientOptions = client.getOptions();
267277
const envelope = createMetricEnvelope(metricBuffer, clientOptions._metadata, clientOptions.tunnel, client.getDsn());
268278

279+
// Track the number of metrics being flushed as in-flight
280+
const metricsCount = metricBuffer.length;
281+
_setInFlightCount(client, count => count + metricsCount);
282+
269283
// Clear the metric buffer after envelopes have been constructed.
270284
_getBufferMap().set(client, []);
271285

272286
client.emit('flushMetrics');
273287

274288
// sendEnvelope should not throw
275289
// eslint-disable-next-line @typescript-eslint/no-floating-promises
276-
client.sendEnvelope(envelope);
290+
client.sendEnvelope(envelope).then(() => _setInFlightCount(client, count => count - metricsCount));
277291
}
278292

279293
/**
@@ -306,3 +320,31 @@ function _getBufferMap(): WeakMap<Client, Array<SerializedMetric>> {
306320
// The reference to the Client <> MetricBuffer map is stored on the carrier to ensure it's always the same
307321
return getGlobalSingleton('clientToMetricBufferMap', () => new WeakMap<Client, Array<SerializedMetric>>());
308322
}
323+
324+
function _getInFlightMap(): WeakMap<Client, number> {
325+
// Track the number of metrics currently in-flight (flushed but not yet sent/completed)
326+
return getGlobalSingleton('clientToInFlightMetricsMap', () => new WeakMap<Client, number>());
327+
}
328+
329+
/**
330+
* Gets the number of metrics currently in-flight (flushed but not yet sent/completed) for a given client.
331+
*
332+
* @param client - The client to get the in-flight count for.
333+
* @returns The number of metrics in-flight.
334+
*/
335+
function _getInFlightCount(client: Client): number {
336+
return _getInFlightMap().get(client) ?? 0;
337+
}
338+
339+
/**
340+
* Sets the in-flight metrics count for a given client.
341+
*
342+
* @param client - The client to set the count for.
343+
* @param countOrUpdater - The value to set the count to, or a function to update the count. If a function is provided, it will be called with the current count as an argument.
344+
*/
345+
function _setInFlightCount(client: Client, countOrUpdater: number | ((current: number) => number)): void {
346+
const inFlightMap = _getInFlightMap();
347+
const currentCount = _getInFlightCount(client);
348+
const nextCount = typeof countOrUpdater === 'function' ? countOrUpdater(currentCount) : countOrUpdater;
349+
inFlightMap.set(client, Math.max(0, nextCount));
350+
}

packages/core/src/types-hoist/options.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,17 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
432432
*/
433433
beforeSendMetric?: (metric: Metric) => Metric | null;
434434

435+
/**
436+
* Maximum number of metrics that can be queued before dropping incoming metrics.
437+
* When the buffer reaches this limit, new metrics will be dropped and recorded as dropped events.
438+
* This prevents unbounded buffer growth when metrics arrive faster than they can be flushed.
439+
*
440+
* Set to 0 to disable dropping metrics.
441+
*
442+
* @default 2000
443+
*/
444+
maxMetricDropLimit?: number;
445+
435446
/**
436447
* Function to compute tracing sample rate dynamically and filter unwanted traces.
437448
*

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,60 @@ describe('_INTERNAL_captureMetric', () => {
271271
expect(mockSendEnvelope).not.toHaveBeenCalled();
272272
});
273273

274+
it('drops metrics when in-flight + buffer count exceeds drop limit', async () => {
275+
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, maxMetricDropLimit: 2000 });
276+
const client = new TestClient(options);
277+
const scope = new Scope();
278+
scope.setClient(client);
279+
280+
// Create a promise that we can control to simulate in-flight network requests
281+
let resolveEnvelope: () => void;
282+
const envelopePromise = new Promise<void>(resolve => {
283+
resolveEnvelope = resolve;
284+
});
285+
286+
const mockSendEnvelope = vi.spyOn(client as any, 'sendEnvelope').mockImplementation(() => envelopePromise);
287+
288+
// Fill buffer to 1000 and trigger flush - this will mark 1000 metrics as in-flight
289+
for (let i = 0; i < 1000; i++) {
290+
_INTERNAL_captureMetric({ type: 'counter', name: `metric.${i}`, value: i }, { scope });
291+
}
292+
expect(_INTERNAL_getMetricBuffer(client)).toHaveLength(1000);
293+
294+
// Trigger flush - buffer cleared, 1000 metrics now in-flight
295+
_INTERNAL_captureMetric({ type: 'counter', name: 'trigger.flush', value: 1000 }, { scope });
296+
expect(_INTERNAL_getMetricBuffer(client)).toHaveLength(1);
297+
expect(mockSendEnvelope).toHaveBeenCalledTimes(1);
298+
299+
// Add 999 more metrics to buffer (total: 1 in buffer + 1000 in-flight = 1001)
300+
for (let i = 0; i < 999; i++) {
301+
_INTERNAL_captureMetric({ type: 'counter', name: `metric.after.${i}`, value: i }, { scope });
302+
}
303+
expect(_INTERNAL_getMetricBuffer(client)).toHaveLength(1000);
304+
305+
// Add one more - should be dropped because (1000 in buffer + 1000 in-flight = 2000 >= 2000)
306+
_INTERNAL_captureMetric({ type: 'counter', name: 'dropped.metric', value: 999 }, { scope });
307+
308+
// Buffer should still be at 1000 (metric was dropped)
309+
const finalBuffer = _INTERNAL_getMetricBuffer(client);
310+
expect(finalBuffer).toHaveLength(1000);
311+
expect(finalBuffer?.some(m => m.name === 'dropped.metric')).toBe(false);
312+
313+
// Verify dropped event was recorded
314+
const outcomes = client._clearOutcomes();
315+
expect(outcomes).toEqual([
316+
{
317+
reason: 'buffer_overflow',
318+
category: 'metric',
319+
quantity: 1,
320+
},
321+
]);
322+
323+
// Resolve the envelope promise to clean up
324+
resolveEnvelope!();
325+
await envelopePromise;
326+
});
327+
274328
it('processes metrics through beforeSendMetric when provided', () => {
275329
const beforeSendMetric = vi.fn().mockImplementation(metric => ({
276330
...metric,

0 commit comments

Comments
 (0)