From 8211d493c023a6f28979a8f850987aa9210e8bcd Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Wed, 19 Nov 2025 16:43:13 +0100 Subject: [PATCH 1/3] ref(rate-limit): handle profile and check_in rate limiting --- src/Transport/HttpTransport.php | 14 ++++++ src/Transport/RateLimiter.php | 16 +++++-- tests/Transport/HttpTransportTest.php | 61 ++++++++++++++++++++++++++- 3 files changed, 86 insertions(+), 5 deletions(-) diff --git a/src/Transport/HttpTransport.php b/src/Transport/HttpTransport.php index f47867fe8..14ca69c28 100644 --- a/src/Transport/HttpTransport.php +++ b/src/Transport/HttpTransport.php @@ -100,6 +100,20 @@ public function send(Event $event): Result return new Result(ResultStatus::rateLimit()); } + // Since profiles are attached to transaction we have to check separately if they are rate limited. + // We can do this after transactions have been checked because if transactions are rate limited, + // so are profiles but not the other way around. + if ($event->getSdkMetadata('profile') !== null) { + if ($this->rateLimiter->isRateLimited(RateLimiter::DATA_CATEGORY_PROFILE)) { + // Just remove profiling data so the normal transaction can be sent. + $event->setSdkMetadata('profile', null); + $this->logger->warning( + 'Rate limit exceeding for sending requests of type "profile".', + ['event' => $event] + ); + } + } + $request = new Request(); $request->setStringBody($this->payloadSerializer->serialize($event)); diff --git a/src/Transport/RateLimiter.php b/src/Transport/RateLimiter.php index dbb8deccf..ab219577f 100644 --- a/src/Transport/RateLimiter.php +++ b/src/Transport/RateLimiter.php @@ -11,6 +11,11 @@ final class RateLimiter { + /** + * @var string + */ + public const DATA_CATEGORY_PROFILE = 'profile'; + /** * @var string */ @@ -21,6 +26,11 @@ final class RateLimiter */ private const DATA_CATEGORY_LOG_ITEM = 'log_item'; + /** + * @var string + */ + private const DATA_CATEGORY_CHECK_IN = 'monitor'; + /** * The name of the header to look at to know the rate limits for the events * categories supported by the server. @@ -103,9 +113,7 @@ public function handleResponse(Response $response): bool */ public function isRateLimited($eventType): bool { - $disabledUntil = $this->getDisabledUntil($eventType); - - return $disabledUntil > time(); + return $this->getDisabledUntil($eventType) > time(); } /** @@ -119,6 +127,8 @@ public function getDisabledUntil($eventType): int $eventType = self::DATA_CATEGORY_ERROR; } elseif ($eventType === 'log') { $eventType = self::DATA_CATEGORY_LOG_ITEM; + } elseif ($eventType === 'check_in') { + $eventType = self::DATA_CATEGORY_CHECK_IN; } return max($this->rateLimits['all'] ?? 0, $this->rateLimits[$eventType] ?? 0); diff --git a/tests/Transport/HttpTransportTest.php b/tests/Transport/HttpTransportTest.php index 942fb9b49..7511f359f 100644 --- a/tests/Transport/HttpTransportTest.php +++ b/tests/Transport/HttpTransportTest.php @@ -12,6 +12,7 @@ use Sentry\HttpClient\HttpClientInterface; use Sentry\HttpClient\Response; use Sentry\Options; +use Sentry\Profiling\Profile; use Sentry\Serializer\PayloadSerializerInterface; use Sentry\Transport\HttpTransport; use Sentry\Transport\ResultStatus; @@ -25,7 +26,7 @@ final class HttpTransportTest extends TestCase private $logger; /** - * @var MockObject&HttpAsyncClientInterface + * @var MockObject&HttpClientInterface */ private $httpClient; @@ -180,7 +181,7 @@ public function testSendFailsDueToHttpClientException(): void $this->assertSame(ResultStatus::failed(), $result->getStatus()); } - public function testSendFailsDueToCulrError(): void + public function testSendFailsDueToCurlError(): void { $event = Event::createEvent(); @@ -263,6 +264,62 @@ public function testSendFailsDueToExceedingRateLimits(): void $this->assertSame(ResultStatus::rateLimit(), $result->getStatus()); } + /** + * @group time-sensitive + */ + public function testProfileIsDroppedWhenRateLimitedButTransactionIsSent(): void + { + ClockMock::withClockMock(1644105600); + + $transport = new HttpTransport( + new Options(['dsn' => 'http://public@example.com/1']), + $this->httpClient, + $this->payloadSerializer, + $this->logger + ); + + $event = Event::createTransaction(); + $event->setSdkMetadata('profile', new Profile()); + + $this->payloadSerializer->expects($this->exactly(2)) + ->method('serialize') + ->willReturn('{}'); + + $this->httpClient->expects($this->exactly(2)) + ->method('sendRequest') + ->willReturnOnConsecutiveCalls( + new Response(429, ['X-Sentry-Rate-Limits' => ['60:profile:key']], ''), + new Response(200, [], '') + ); + + + // First request is rate limited because of profiles + $result = $transport->send($event); + + $this->assertEquals(ResultStatus::rateLimit(), $result->getStatus()); + + // profile information is still present + $this->assertNotNull($event->getSdkMetadata('profile')); + + $event = Event::createTransaction(); + $event->setSdkMetadata('profile', new Profile()); + + $this->logger->expects($this->once()) + ->method('warning') + ->with( + $this->stringContains('Rate limit exceeding for sending requests of type "profile".'), + ['event' => $event] + ); + + $result = $transport->send($event); + + // Sending transaction is successful because only profiles are rate limited + $this->assertEquals(ResultStatus::success(), $result->getStatus()); + + // profile information is removed because it was rate limited + $this->assertNull($event->getSdkMetadata('profile')); + } + public function testClose(): void { $transport = new HttpTransport( From 5e26a65bbcb19f1c03ff463f14f29b6362836b68 Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Wed, 19 Nov 2025 16:49:28 +0100 Subject: [PATCH 2/3] CS --- tests/Transport/HttpTransportTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Transport/HttpTransportTest.php b/tests/Transport/HttpTransportTest.php index 7511f359f..8da6240f0 100644 --- a/tests/Transport/HttpTransportTest.php +++ b/tests/Transport/HttpTransportTest.php @@ -292,7 +292,6 @@ public function testProfileIsDroppedWhenRateLimitedButTransactionIsSent(): void new Response(200, [], '') ); - // First request is rate limited because of profiles $result = $transport->send($event); From a9d8344c5c74434760c05c64cced66fa0358bd0e Mon Sep 17 00:00:00 2001 From: Martin Linzmayer Date: Wed, 19 Nov 2025 18:26:59 +0100 Subject: [PATCH 3/3] add test case for check_in --- src/Transport/HttpTransport.php | 2 +- tests/Transport/HttpTransportTest.php | 50 +++++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/src/Transport/HttpTransport.php b/src/Transport/HttpTransport.php index 14ca69c28..71da8d5a3 100644 --- a/src/Transport/HttpTransport.php +++ b/src/Transport/HttpTransport.php @@ -108,7 +108,7 @@ public function send(Event $event): Result // Just remove profiling data so the normal transaction can be sent. $event->setSdkMetadata('profile', null); $this->logger->warning( - 'Rate limit exceeding for sending requests of type "profile".', + 'Rate limit exceeded for sending requests of type "profile". The profile has been dropped.', ['event' => $event] ); } diff --git a/tests/Transport/HttpTransportTest.php b/tests/Transport/HttpTransportTest.php index 8da6240f0..66d564b85 100644 --- a/tests/Transport/HttpTransportTest.php +++ b/tests/Transport/HttpTransportTest.php @@ -267,7 +267,7 @@ public function testSendFailsDueToExceedingRateLimits(): void /** * @group time-sensitive */ - public function testProfileIsDroppedWhenRateLimitedButTransactionIsSent(): void + public function testDropsProfileAndSendsTransactionWhenProfileRateLimited(): void { ClockMock::withClockMock(1644105600); @@ -283,7 +283,7 @@ public function testProfileIsDroppedWhenRateLimitedButTransactionIsSent(): void $this->payloadSerializer->expects($this->exactly(2)) ->method('serialize') - ->willReturn('{}'); + ->willReturn('{"foo":"bar"}'); $this->httpClient->expects($this->exactly(2)) ->method('sendRequest') @@ -306,7 +306,7 @@ public function testProfileIsDroppedWhenRateLimitedButTransactionIsSent(): void $this->logger->expects($this->once()) ->method('warning') ->with( - $this->stringContains('Rate limit exceeding for sending requests of type "profile".'), + $this->stringContains('Rate limit exceeded for sending requests of type "profile".'), ['event' => $event] ); @@ -319,6 +319,50 @@ public function testProfileIsDroppedWhenRateLimitedButTransactionIsSent(): void $this->assertNull($event->getSdkMetadata('profile')); } + /** + * @group time-sensitive + */ + public function testCheckInsAreRateLimited(): void + { + ClockMock::withClockMock(1644105600); + + $transport = new HttpTransport( + new Options(['dsn' => 'http://public@example.com/1']), + $this->httpClient, + $this->payloadSerializer, + $this->logger + ); + + $event = Event::createCheckIn(); + + $this->payloadSerializer->expects($this->exactly(1)) + ->method('serialize') + ->willReturn('{"foo":"bar"}'); + + $this->httpClient->expects($this->exactly(1)) + ->method('sendRequest') + ->willReturn( + new Response(429, ['X-Sentry-Rate-Limits' => ['60:monitor:key']], '') + ); + + $result = $transport->send($event); + + $this->assertEquals(ResultStatus::rateLimit(), $result->getStatus()); + + $event = Event::createCheckIn(); + + $this->logger->expects($this->once()) + ->method('warning') + ->with( + $this->stringContains('Rate limit exceeded for sending requests of type "check_in".'), + ['event' => $event] + ); + + $result = $transport->send($event); + + $this->assertEquals(ResultStatus::rateLimit(), $result->getStatus()); + } + public function testClose(): void { $transport = new HttpTransport(