diff --git a/src/Transport/HttpTransport.php b/src/Transport/HttpTransport.php index f47867fe8..71da8d5a3 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 exceeded for sending requests of type "profile". The profile has been dropped.', + ['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..66d564b85 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,105 @@ public function testSendFailsDueToExceedingRateLimits(): void $this->assertSame(ResultStatus::rateLimit(), $result->getStatus()); } + /** + * @group time-sensitive + */ + public function testDropsProfileAndSendsTransactionWhenProfileRateLimited(): 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('{"foo":"bar"}'); + + $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 exceeded 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')); + } + + /** + * @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(