ref(rate-limit): handle profile and check_in rate limiting (#1970)

Author: Litarnus

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));
 

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);

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://[email protected]/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://[email protected]/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(