review

Author: Litarnus

src/ClientReport/ClientReportAggregator.php

@@ -58,7 +58,7 @@ public function flush(): void
         $reports = [];
         foreach ($this->reports as $category => $reasons) {
             foreach ($reasons as $reason => $quantity) {
-                $reports[] = new ClientReport($category, $reason, $quantity);
+                $reports[] = new DiscardedEvent($category, $reason, $quantity);
             }
         }
         $event = Event::createClientReport();

src/ClientReport/ClientReport.php src/ClientReport/DiscardedEvent.phpsrc/ClientReport/ClientReport.php renamed to src/ClientReport/DiscardedEvent.php

@@ -4,7 +4,7 @@
 
 namespace Sentry\ClientReport;
 
-class ClientReport
+class DiscardedEvent
 {
     /**
      * @var string

src/Event.php

@@ -4,7 +4,7 @@
 
 namespace Sentry;
 
-use Sentry\ClientReport\ClientReport;
+use Sentry\ClientReport\DiscardedEvent;
 use Sentry\Context\OsContext;
 use Sentry\Context\RuntimeContext;
 use Sentry\Logs\Log;
@@ -212,7 +212,7 @@ final class Event
     private $profile;
 
     /**
-     * @var ClientReport[]
+     * @var DiscardedEvent[]
      */
     private $clientReports = [];
 
@@ -991,7 +991,7 @@ public function getTraceId(): ?string
     }
 
     /**
-     * @param ClientReport[] $clientReports
+     * @param DiscardedEvent[] $clientReports
      */
     public function setClientReports(array $clientReports): self
     {
@@ -1001,7 +1001,7 @@ public function setClientReports(array $clientReports): self
     }
 
     /**
-     * @return ClientReport[]
+     * @return DiscardedEvent[]
      */
     public function getClientReports(): array
     {

src/EventType.php

@@ -86,6 +86,16 @@ public function requiresEventId(): bool
         }
     }
 
+    /**
+     * Returns false if rate limiting should not be applied.
+     *
+     * @return bool
+     */
+    public function requiresRateLimiting(): bool
+    {
+        return $this !== self::clientReport();
+    }
+
     public function __toString(): string
     {
         return $this->value;

src/Serializer/EnvelopItems/ClientReportItem.php

@@ -4,8 +4,9 @@
 
 namespace Sentry\Serializer\EnvelopItems;
 
-use Sentry\ClientReport\ClientReport;
+use Sentry\ClientReport\DiscardedEvent;
 use Sentry\Event;
+use Sentry\Util\JSON;
 
 class ClientReportItem implements EnvelopeItemInterface
 {
@@ -16,7 +17,7 @@ public static function toEnvelopeItem(Event $event): ?string
         $headers = ['type' => 'client_report'];
         $body = [
             'timestamp' => $event->getTimestamp(),
-            'discarded_events' => array_map(static function (ClientReport $report) {
+            'discarded_events' => array_map(static function (DiscardedEvent $report) {
                 return [
                     'category' => $report->getCategory(),
                     'reason' => $report->getReason(),
@@ -25,6 +26,6 @@ public static function toEnvelopeItem(Event $event): ?string
             }, $reports),
         ];
 
-        return \sprintf("%s\n%s", json_encode($headers), json_encode($body));
+        return \sprintf("%s\n%s", JSON::encode($headers), JSON::encode($body));
     }
 }

src/Transport/HttpTransport.php

@@ -91,26 +91,28 @@ public function send(Event $event): Result
         $this->logger->info(\sprintf('Sending %s to %s.', $eventDescription, $targetDescription), ['event' => $event]);
 
         $eventType = $event->getType();
-        if ($this->rateLimiter->isRateLimited((string) $eventType)) {
-            $this->logger->warning(
-                \sprintf('Rate limit exceeded for sending requests of type "%s".', (string) $eventType),
-                ['event' => $event]
-            );
-
-            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);
+        if ($eventType->requiresRateLimiting()) {
+            if ($this->rateLimiter->isRateLimited((string)$eventType)) {
                 $this->logger->warning(
-                    'Rate limit exceeded for sending requests of type "profile". The profile has been dropped.',
+                    \sprintf('Rate limit exceeded for sending requests of type "%s".', (string)$eventType),
                     ['event' => $event]
                 );
+
+                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]
+                    );
+                }
             }
         }
 

tests/Serializer/PayloadSerializerTest.php

@@ -9,7 +9,7 @@
 use Sentry\CheckIn;
 use Sentry\CheckInStatus;
 use Sentry\Client;
-use Sentry\ClientReport\ClientReport;
+use Sentry\ClientReport\DiscardedEvent;
 use Sentry\Context\OsContext;
 use Sentry\Context\RuntimeContext;
 use Sentry\Event;
@@ -488,8 +488,8 @@ public static function serializeAsEnvelopeDataProvider(): iterable
 
         $event = Event::createClientReport();
         $event->setClientReports([
-            new ClientReport('log_item', 'buffer_overflow', 1),
-            new ClientReport('log_byte', 'buffer_overflow', 256),
+            new DiscardedEvent('log_item', 'buffer_overflow', 1),
+            new DiscardedEvent('log_byte', 'buffer_overflow', 256),
         ]);
 
         yield [
@@ -504,8 +504,8 @@ public static function serializeAsEnvelopeDataProvider(): iterable
 
         $event = Event::createClientReport();
         $event->setClientReports([
-            new ClientReport('error', 'before_send', 10),
-            new ClientReport('profile', 'internal_sdk_error', 50),
+            new DiscardedEvent('error', 'before_send', 10),
+            new DiscardedEvent('profile', 'internal_sdk_error', 50),
         ]);
 
         yield [