From 2d58238661212a8c8d0eba5f83acad6854967295 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Tue, 9 Apr 2024 14:13:35 +0200 Subject: [PATCH 1/4] metrics rate limits --- packages/utils/src/ratelimit.ts | 13 +++++++++-- packages/utils/test/ratelimit.test.ts | 32 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/packages/utils/src/ratelimit.ts b/packages/utils/src/ratelimit.ts index eda9375a9de6..8aede2935152 100644 --- a/packages/utils/src/ratelimit.ts +++ b/packages/utils/src/ratelimit.ts @@ -67,7 +67,7 @@ export function updateRateLimits( * rate limit headers are of the form *
,
,.. * where each
is of the form - * : : : + * : : : : * where * is a delay in seconds * is the event type(s) (error, transaction, etc) being rate limited and is of the form @@ -85,7 +85,16 @@ export function updateRateLimits( updatedRateLimits.all = now + delay; } else { for (const category of categories.split(';')) { - updatedRateLimits[category] = now + delay; + if (category === 'metric_bucket') { + const namespaces = limit.split(':', 5)[4] ? limit.split(':', 5)[4].split(';') : []; + + if (!namespaces || namespaces.includes('custom')) { + // back off transmitting metrics from the SDK + updatedRateLimits[category] = now + delay; + } + } else { + updatedRateLimits[category] = now + delay; + } } } } diff --git a/packages/utils/test/ratelimit.test.ts b/packages/utils/test/ratelimit.test.ts index e9447aefcc73..af208d1e8773 100644 --- a/packages/utils/test/ratelimit.test.ts +++ b/packages/utils/test/ratelimit.test.ts @@ -197,3 +197,35 @@ describe('updateRateLimits()', () => { expect(updatedRateLimits.all).toEqual(60_000); }); }); + +describe('data category "metric_bucket"', () => { + test('should add limit for `metric_bucket` category when namespaces contain "custom"', () => { + const rateLimits: RateLimits = {}; + const headers = { + 'retry-after': null, + 'x-sentry-rate-limits': '42:metric_bucket:::custom', + }; + const updatedRateLimits = updateRateLimits(rateLimits, { headers }, 0); + expect(updatedRateLimits.metric_bucket).toEqual(42 * 1000); + }); + + test('should not add limit for `metric_bucket` category when namespaces do not contain "custom"', () => { + const rateLimits: RateLimits = {}; + const headers = { + 'retry-after': null, + 'x-sentry-rate-limits': '42:metric_bucket:::namespace1;namespace2', + }; + const updatedRateLimits = updateRateLimits(rateLimits, { headers }, 0); + expect(updatedRateLimits.metric_bucket).toBeUndefined(); + }); + + test('should add limit for `metric_bucket` category when namespaces are empty', () => { + const rateLimits: RateLimits = {}; + const headers = { + 'retry-after': null, + 'x-sentry-rate-limits': '42:metric_bucket', + }; + const updatedRateLimits = updateRateLimits(rateLimits, { headers }, 0); + expect(updatedRateLimits.metric_bucket).toEqual(42 * 1000); + }); +}); From 945c6e509021f8746dbe8a2231101d134cd695a6 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Tue, 9 Apr 2024 15:44:09 +0200 Subject: [PATCH 2/4] fix test --- packages/utils/src/ratelimit.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/utils/src/ratelimit.ts b/packages/utils/src/ratelimit.ts index 8aede2935152..8124b31cae99 100644 --- a/packages/utils/src/ratelimit.ts +++ b/packages/utils/src/ratelimit.ts @@ -86,7 +86,7 @@ export function updateRateLimits( } else { for (const category of categories.split(';')) { if (category === 'metric_bucket') { - const namespaces = limit.split(':', 5)[4] ? limit.split(':', 5)[4].split(';') : []; + const namespaces = limit.split(':', 5)[4] ? limit.split(':', 5)[4].split(';') : null; if (!namespaces || namespaces.includes('custom')) { // back off transmitting metrics from the SDK From afca5eef91df6d288f816e5ec58c64f118f51175 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 10 Apr 2024 09:18:13 +0200 Subject: [PATCH 3/4] shorten code --- packages/utils/src/ratelimit.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/utils/src/ratelimit.ts b/packages/utils/src/ratelimit.ts index 8124b31cae99..3b67b013a1c5 100644 --- a/packages/utils/src/ratelimit.ts +++ b/packages/utils/src/ratelimit.ts @@ -78,21 +78,15 @@ export function updateRateLimits( * Only present if rate limit applies to the metric_bucket data category. */ for (const limit of rateLimitHeader.trim().split(',')) { - const [retryAfter, categories] = limit.split(':', 2); + const [retryAfter, categories, , , namespaces] = limit.split(':', 5); const headerDelay = parseInt(retryAfter, 10); const delay = (!isNaN(headerDelay) ? headerDelay : 60) * 1000; // 60sec default if (!categories) { updatedRateLimits.all = now + delay; } else { for (const category of categories.split(';')) { - if (category === 'metric_bucket') { - const namespaces = limit.split(':', 5)[4] ? limit.split(':', 5)[4].split(';') : null; - - if (!namespaces || namespaces.includes('custom')) { - // back off transmitting metrics from the SDK - updatedRateLimits[category] = now + delay; - } - } else { + // namespaces will be present when category === 'metric_bucket' + if (category !== 'metric_bucket' || !namespaces || namespaces.split(';').includes('custom')) { updatedRateLimits[category] = now + delay; } } From d8985febff005a2052fe4b3b1e76e1202f1a9ea2 Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 10 Apr 2024 14:10:08 +0200 Subject: [PATCH 4/4] change if cond. Add test --- packages/utils/src/ratelimit.ts | 8 ++++++-- packages/utils/test/ratelimit.test.ts | 16 ++++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/utils/src/ratelimit.ts b/packages/utils/src/ratelimit.ts index 3b67b013a1c5..77be8cf7fa9e 100644 --- a/packages/utils/src/ratelimit.ts +++ b/packages/utils/src/ratelimit.ts @@ -85,8 +85,12 @@ export function updateRateLimits( updatedRateLimits.all = now + delay; } else { for (const category of categories.split(';')) { - // namespaces will be present when category === 'metric_bucket' - if (category !== 'metric_bucket' || !namespaces || namespaces.split(';').includes('custom')) { + if (category === 'metric_bucket') { + // namespaces will be present when category === 'metric_bucket' + if (!namespaces || namespaces.split(';').includes('custom')) { + updatedRateLimits[category] = now + delay; + } + } else { updatedRateLimits[category] = now + delay; } } diff --git a/packages/utils/test/ratelimit.test.ts b/packages/utils/test/ratelimit.test.ts index af208d1e8773..5833548727f7 100644 --- a/packages/utils/test/ratelimit.test.ts +++ b/packages/utils/test/ratelimit.test.ts @@ -221,11 +221,19 @@ describe('data category "metric_bucket"', () => { test('should add limit for `metric_bucket` category when namespaces are empty', () => { const rateLimits: RateLimits = {}; - const headers = { + + const headers1 = { 'retry-after': null, - 'x-sentry-rate-limits': '42:metric_bucket', + 'x-sentry-rate-limits': '42:metric_bucket', // without semicolon at the end }; - const updatedRateLimits = updateRateLimits(rateLimits, { headers }, 0); - expect(updatedRateLimits.metric_bucket).toEqual(42 * 1000); + const updatedRateLimits1 = updateRateLimits(rateLimits, { headers: headers1 }, 0); + expect(updatedRateLimits1.metric_bucket).toEqual(42 * 1000); + + const headers2 = { + 'retry-after': null, + 'x-sentry-rate-limits': '42:metric_bucket:organization:quota_exceeded:', // with semicolon at the end + }; + const updatedRateLimits2 = updateRateLimits(rateLimits, { headers: headers2 }, 0); + expect(updatedRateLimits2.metric_bucket).toEqual(42 * 1000); }); });