Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/core/src/carrier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ export interface SentryCarrier {
*/
clientToMetricBufferMap?: WeakMap<Client, Array<SerializedMetric>>;

/**
* A map of Sentry clients to the number of metrics currently in-flight (flushed but not yet sent/completed).
* This is used to track metrics that have been flushed but are still waiting for network requests to complete.
*/
clientToInFlightMetricsMap?: WeakMap<Client, number>;

/** Overwrites TextEncoder used in `@sentry/core`, need for `[email protected]` and older */
encodePolyfill?: (input: string) => Uint8Array;
/** Overwrites TextDecoder used in `@sentry/core`, need for `[email protected]` and older */
Expand Down
44 changes: 43 additions & 1 deletion packages/core/src/metrics/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { _getTraceInfoFromScope } from '../utils/trace-info';
import { createMetricEnvelope } from './envelope';

const MAX_METRIC_BUFFER_SIZE = 1000;
const DEFAULT_MAX_METRIC_DROP_LIMIT = 2000;

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

if (metricBuffer === undefined) {
bufferMap.set(client, [serializedMetric]);
} else {
const totalMetrics = metricBuffer.length + inFlightCount;

if (totalMetrics >= maxDropLimit && maxDropLimit > 0) {
client.recordDroppedEvent('buffer_overflow', 'metric');
return;
}

if (metricBuffer.length >= MAX_METRIC_BUFFER_SIZE) {
_INTERNAL_flushMetricsBuffer(client, metricBuffer);
bufferMap.set(client, [serializedMetric]);
Expand Down Expand Up @@ -266,14 +276,18 @@ export function _INTERNAL_flushMetricsBuffer(client: Client, maybeMetricBuffer?:
const clientOptions = client.getOptions();
const envelope = createMetricEnvelope(metricBuffer, clientOptions._metadata, clientOptions.tunnel, client.getDsn());

// Track the number of metrics being flushed as in-flight
const metricsCount = metricBuffer.length;
_setInFlightCount(client, count => count + metricsCount);

// Clear the metric buffer after envelopes have been constructed.
_getBufferMap().set(client, []);

client.emit('flushMetrics');

// sendEnvelope should not throw
// eslint-disable-next-line @typescript-eslint/no-floating-promises
client.sendEnvelope(envelope);
client.sendEnvelope(envelope).then(() => _setInFlightCount(client, count => count - metricsCount));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: In-flight count leaks if sendEnvelope promise rejects

The in-flight count decrement uses .then() without a .catch() or .finally(). If sendEnvelope rejects for any reason (despite the comment saying it shouldn't throw), the callback never runs and metricsCount is never subtracted from the in-flight counter. This causes a permanent leak where the in-flight count stays artificially high, leading to future metrics being incorrectly dropped due to the inflated total. Using .finally() instead would ensure the count is always decremented regardless of promise resolution or rejection.

Fix in Cursor Fix in Web

}

/**
Expand Down Expand Up @@ -306,3 +320,31 @@ function _getBufferMap(): WeakMap<Client, Array<SerializedMetric>> {
// The reference to the Client <> MetricBuffer map is stored on the carrier to ensure it's always the same
return getGlobalSingleton('clientToMetricBufferMap', () => new WeakMap<Client, Array<SerializedMetric>>());
}

function _getInFlightMap(): WeakMap<Client, number> {
// Track the number of metrics currently in-flight (flushed but not yet sent/completed)
return getGlobalSingleton('clientToInFlightMetricsMap', () => new WeakMap<Client, number>());
}

/**
* Gets the number of metrics currently in-flight (flushed but not yet sent/completed) for a given client.
*
* @param client - The client to get the in-flight count for.
* @returns The number of metrics in-flight.
*/
function _getInFlightCount(client: Client): number {
return _getInFlightMap().get(client) ?? 0;
}

/**
* Sets the in-flight metrics count for a given client.
*
* @param client - The client to set the count for.
* @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.
*/
function _setInFlightCount(client: Client, countOrUpdater: number | ((current: number) => number)): void {
const inFlightMap = _getInFlightMap();
const currentCount = _getInFlightCount(client);
const nextCount = typeof countOrUpdater === 'function' ? countOrUpdater(currentCount) : countOrUpdater;
inFlightMap.set(client, Math.max(0, nextCount));
}
11 changes: 11 additions & 0 deletions packages/core/src/types-hoist/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,17 @@ export interface ClientOptions<TO extends BaseTransportOptions = BaseTransportOp
*/
beforeSendMetric?: (metric: Metric) => Metric | null;

/**
* Maximum number of metrics that can be queued before dropping incoming metrics.
* When the buffer reaches this limit, new metrics will be dropped and recorded as dropped events.
* This prevents unbounded buffer growth when metrics arrive faster than they can be flushed.
*
* Set to 0 to disable dropping metrics.
*
* @default 2000
*/
maxMetricDropLimit?: number;

/**
* Function to compute tracing sample rate dynamically and filter unwanted traces.
*
Expand Down
54 changes: 54 additions & 0 deletions packages/core/test/lib/metrics/internal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,60 @@ describe('_INTERNAL_captureMetric', () => {
expect(mockSendEnvelope).not.toHaveBeenCalled();
});

it('drops metrics when in-flight + buffer count exceeds drop limit', async () => {
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, maxMetricDropLimit: 2000 });
const client = new TestClient(options);
const scope = new Scope();
scope.setClient(client);

// Create a promise that we can control to simulate in-flight network requests
let resolveEnvelope: () => void;
const envelopePromise = new Promise<void>(resolve => {
resolveEnvelope = resolve;
});

const mockSendEnvelope = vi.spyOn(client as any, 'sendEnvelope').mockImplementation(() => envelopePromise);

// Fill buffer to 1000 and trigger flush - this will mark 1000 metrics as in-flight
for (let i = 0; i < 1000; i++) {
_INTERNAL_captureMetric({ type: 'counter', name: `metric.${i}`, value: i }, { scope });
}
expect(_INTERNAL_getMetricBuffer(client)).toHaveLength(1000);

// Trigger flush - buffer cleared, 1000 metrics now in-flight
_INTERNAL_captureMetric({ type: 'counter', name: 'trigger.flush', value: 1000 }, { scope });
expect(_INTERNAL_getMetricBuffer(client)).toHaveLength(1);
expect(mockSendEnvelope).toHaveBeenCalledTimes(1);

// Add 999 more metrics to buffer (total: 1 in buffer + 1000 in-flight = 1001)
for (let i = 0; i < 999; i++) {
_INTERNAL_captureMetric({ type: 'counter', name: `metric.after.${i}`, value: i }, { scope });
}
expect(_INTERNAL_getMetricBuffer(client)).toHaveLength(1000);

// Add one more - should be dropped because (1000 in buffer + 1000 in-flight = 2000 >= 2000)
_INTERNAL_captureMetric({ type: 'counter', name: 'dropped.metric', value: 999 }, { scope });

// Buffer should still be at 1000 (metric was dropped)
const finalBuffer = _INTERNAL_getMetricBuffer(client);
expect(finalBuffer).toHaveLength(1000);
expect(finalBuffer?.some(m => m.name === 'dropped.metric')).toBe(false);

// Verify dropped event was recorded
const outcomes = client._clearOutcomes();
expect(outcomes).toEqual([
{
reason: 'buffer_overflow',
category: 'metric',
quantity: 1,
},
]);

// Resolve the envelope promise to clean up
resolveEnvelope!();
await envelopePromise;
});

it('processes metrics through beforeSendMetric when provided', () => {
const beforeSendMetric = vi.fn().mockImplementation(metric => ({
...metric,
Expand Down
Loading