diff --git a/analysis-baseline.toml b/analysis-baseline.toml index 839354d6a..e90237451 100644 --- a/analysis-baseline.toml +++ b/analysis-baseline.toml @@ -1044,6 +1044,12 @@ code = "mixed-return-statement" message = 'Could not infer a precise return type for function `Sentry\Options::shouldCaptureSilencedErrors`. Saw type `mixed`.' count = 1 +[[issues]] +file = "src/Options.php" +code = "mixed-return-statement" +message = 'Could not infer a precise return type for function `Sentry\Options::shouldSendClientReports`. Saw type `mixed`.' +count = 1 + [[issues]] file = "src/Options.php" code = "mixed-return-statement" diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index d68b4346c..fa17f6f60 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -492,6 +492,12 @@ parameters: count: 1 path: src/Options.php + - + message: '#^Method Sentry\\Options\:\:shouldSendClientReports\(\) should return bool but returns mixed\.$#' + identifier: return.type + count: 1 + path: src/Options.php + - message: '#^Method Sentry\\Options\:\:shouldSendDefaultPii\(\) should return bool but returns mixed\.$#' identifier: return.type diff --git a/src/ClientReport/ClientReportAggregator.php b/src/ClientReport/ClientReportAggregator.php index 8045a51b2..e744d32db 100644 --- a/src/ClientReport/ClientReportAggregator.php +++ b/src/ClientReport/ClientReportAggregator.php @@ -5,6 +5,7 @@ namespace Sentry\ClientReport; use Sentry\Event; +use Sentry\EventType; use Sentry\State\HubAdapter; use Sentry\Transport\DataCategory; @@ -32,6 +33,10 @@ class ClientReportAggregator public function add(DataCategory $category, Reason $reason, int $quantity): void { + if (!$this->isEnabled()) { + return; + } + $category = $category->getValue(); $reason = $reason->getValue(); if ($quantity <= 0) { @@ -50,6 +55,44 @@ public function add(DataCategory $category, Reason $reason, int $quantity): void $this->reports[$category][$reason] = ($this->reports[$category][$reason] ?? 0) + $quantity; } + /** + * Records a discarded event in all the data categories it accounts for, e.g. a + * transaction also discards its spans and its profile. + */ + public function addEvent(Event $event, Reason $reason): void + { + if (!$this->isEnabled()) { + return; + } + + switch ($event->getType()) { + case EventType::event(): + $this->add(DataCategory::error(), $reason, 1); + $this->addAttachments($event, $reason); + break; + case EventType::transaction(): + $this->add(DataCategory::transaction(), $reason, 1); + // Relay extracts an additional span from the transaction itself + $this->add(DataCategory::span(), $reason, \count($event->getSpans()) + 1); + + if ($event->getSdkMetadata('profile') !== null) { + $this->add(DataCategory::profile(), $reason, 1); + } + + $this->addAttachments($event, $reason); + break; + case EventType::checkIn(): + $this->add(DataCategory::checkIn(), $reason, 1); + break; + case EventType::logs(): + $this->add(DataCategory::logItem(), $reason, \count($event->getLogs())); + break; + case EventType::metrics(): + $this->add(DataCategory::metric(), $reason, \count($event->getMetrics())); + break; + } + } + public function flush(): void { if (empty($this->reports)) { @@ -73,6 +116,26 @@ public function flush(): void } } + private function isEnabled(): bool + { + $client = HubAdapter::getInstance()->getClient(); + + return $client !== null && $client->getOptions()->shouldSendClientReports(); + } + + private function addAttachments(Event $event, Reason $reason): void + { + $bytes = 0; + + foreach ($event->getAttachments() as $attachment) { + $bytes += $attachment->getSize() ?? 0; + } + + if ($bytes > 0) { + $this->add(DataCategory::attachment(), $reason, $bytes); + } + } + public static function getInstance(): self { if (self::$instance === null) { diff --git a/src/Options.php b/src/Options.php index a253bed61..6789b3d5f 100644 --- a/src/Options.php +++ b/src/Options.php @@ -1005,6 +1005,24 @@ public function setSendDefaultPii(bool $enable): self return $this->updateOptions(['send_default_pii' => $enable]); } + /** + * Returns whether the SDK should record and send client reports about discarded telemetry. + */ + public function shouldSendClientReports(): bool + { + return $this->options['send_client_reports']; + } + + /** + * Sets whether the SDK should record and send client reports about discarded telemetry. + * + * @param bool $enable Flag indicating if client reports will be sent + */ + public function setSendClientReports(bool $enable): self + { + return $this->updateOptions(['send_client_reports' => $enable]); + } + /** * Returns whether the default integrations are enabled. */ @@ -1338,6 +1356,7 @@ private function configureOptions(OptionsResolver $resolver): void $resolver->setAllowedTypes('before_breadcrumb', ['callable']); $resolver->setAllowedTypes('integrations', ['Sentry\\Integration\\IntegrationInterface[]', 'callable']); $resolver->setAllowedTypes('send_default_pii', 'bool'); + $resolver->setAllowedTypes('send_client_reports', 'bool'); $resolver->setAllowedTypes('default_integrations', 'bool'); $resolver->setAllowedTypes('max_value_length', 'int'); $resolver->setAllowedTypes('max_breadth', ['null', 'int']); @@ -1452,6 +1471,7 @@ private function configureOptions(OptionsResolver $resolver): void 'in_app_exclude' => [], 'in_app_include' => [], 'send_default_pii' => false, + 'send_client_reports' => false, 'max_value_length' => 1024, 'max_breadth' => null, 'transport' => null, diff --git a/src/Transport/DataCategory.php b/src/Transport/DataCategory.php index b2ee0884e..3e7e14feb 100644 --- a/src/Transport/DataCategory.php +++ b/src/Transport/DataCategory.php @@ -31,6 +31,11 @@ public static function transaction(): self return self::getInstance('transaction'); } + public static function span(): self + { + return self::getInstance('span'); + } + // TODO: not sure if this should be called monitor or checkIn. public static function checkIn(): self { @@ -52,6 +57,11 @@ public static function profile(): self return self::getInstance('profile'); } + public static function attachment(): self + { + return self::getInstance('attachment'); + } + public static function metric(): self { return self::getInstance('trace_metric'); diff --git a/src/functions.php b/src/functions.php index 6e2eb1418..555d1feba 100644 --- a/src/functions.php +++ b/src/functions.php @@ -81,6 +81,7 @@ * release?: string|null, * sample_rate?: float|int, * send_attempts?: int, + * send_client_reports?: bool, * send_default_pii?: bool, * server_name?: string, * spotlight?: bool, diff --git a/tests/ClientReport/ClientReportAggregatorTest.php b/tests/ClientReport/ClientReportAggregatorTest.php index ce82c4d50..2097d0e87 100644 --- a/tests/ClientReport/ClientReportAggregatorTest.php +++ b/tests/ClientReport/ClientReportAggregatorTest.php @@ -5,18 +5,31 @@ namespace Sentry\Tests\ClientReport; use PHPUnit\Framework\TestCase; +use Sentry\Attachment\Attachment; use Sentry\Client; use Sentry\ClientReport\ClientReportAggregator; use Sentry\ClientReport\Reason; +use Sentry\Event; +use Sentry\Logs\Log; +use Sentry\Logs\LogLevel; +use Sentry\Metrics\Types\CounterMetric; +use Sentry\Metrics\Types\DistributionMetric; use Sentry\Options; +use Sentry\Profiling\Profile; use Sentry\SentrySdk; use Sentry\State\Hub; use Sentry\Tests\StubLogger; use Sentry\Tests\StubTransport; +use Sentry\Tracing\Span; +use Sentry\Tracing\SpanId; +use Sentry\Tracing\TraceId; use Sentry\Transport\DataCategory; +use Sentry\Unit; class ClientReportAggregatorTest extends TestCase { + use ClientReportAssertions; + protected function setUp(): void { ini_set('zend.exception_ignore_args', '0'); @@ -24,6 +37,7 @@ protected function setUp(): void StubLogger::$logs = []; SentrySdk::init()->bindClient(new Client(new Options([ 'logger' => StubLogger::getInstance(), + 'send_client_reports' => true, ]), StubTransport::getInstance())); } @@ -115,4 +129,101 @@ public function testNegativeQuantityDiscardedWhenNoClientIsBound(): void $this->assertEmpty(StubTransport::$events); $this->assertEmpty(StubLogger::$logs); } + + public function testAddEventForError(): void + { + $event = Event::createEvent(); + $event->setAttachments([ + Attachment::fromBytes('foo.txt', 'foo'), + Attachment::fromBytes('bar.txt', 'barbaz'), + ]); + + ClientReportAggregator::getInstance()->addEvent($event, Reason::beforeSend()); + + $this->assertClientReports([ + 'error' => ['before_send' => 1], + 'attachment' => ['before_send' => 9], + ]); + } + + public function testAddEventForTransaction(): void + { + $event = Event::createTransaction(); + $event->setSpans([new Span(), new Span()]); + $event->setSdkMetadata('profile', new Profile()); + + ClientReportAggregator::getInstance()->addEvent($event, Reason::ratelimitBackoff()); + + $this->assertClientReports([ + 'transaction' => ['ratelimit_backoff' => 1], + 'span' => ['ratelimit_backoff' => 3], + 'profile' => ['ratelimit_backoff' => 1], + ]); + } + + public function testAddEventForTransactionWithoutSpans(): void + { + ClientReportAggregator::getInstance()->addEvent(Event::createTransaction(), Reason::sampleRate()); + + $this->assertClientReports([ + 'transaction' => ['sample_rate' => 1], + 'span' => ['sample_rate' => 1], + ]); + } + + public function testAddEventForCheckIn(): void + { + ClientReportAggregator::getInstance()->addEvent(Event::createCheckIn(), Reason::beforeSend()); + + $this->assertClientReports([ + 'monitor' => ['before_send' => 1], + ]); + } + + public function testAddEventForLogs(): void + { + $event = Event::createLogs()->setLogs([ + new Log(1597790835.0, '566e3688a61d4bc888951642d6f14a19', LogLevel::info(), 'foo'), + new Log(1597790835.0, '566e3688a61d4bc888951642d6f14a19', LogLevel::error(), 'bar'), + ]); + + ClientReportAggregator::getInstance()->addEvent($event, Reason::ratelimitBackoff()); + + $this->assertClientReports([ + 'log_item' => ['ratelimit_backoff' => 2], + ]); + } + + public function testAddEventForMetrics(): void + { + $event = Event::createMetrics()->setMetrics([ + new CounterMetric('foo', 1, new TraceId('566e3688a61d4bc888951642d6f14a19'), new SpanId('566e3688a61d4bc8'), [], 1597790835.0, null), + new DistributionMetric('bar', 12.75, new TraceId('566e3688a61d4bc888951642d6f14a19'), new SpanId('566e3688a61d4bc8'), [], 1597790835.0, Unit::millisecond()), + ]); + + ClientReportAggregator::getInstance()->addEvent($event, Reason::ratelimitBackoff()); + + $this->assertClientReports([ + 'trace_metric' => ['ratelimit_backoff' => 2], + ]); + } + + public function testAddEventIgnoresClientReports(): void + { + ClientReportAggregator::getInstance()->addEvent(Event::createClientReport(), Reason::sendError()); + + $this->assertClientReports([]); + } + + public function testNothingIsRecordedWhenClientReportsAreDisabled(): void + { + SentrySdk::getCurrentHub()->bindClient(new Client(new Options([ + 'send_client_reports' => false, + ]), StubTransport::getInstance())); + + ClientReportAggregator::getInstance()->add(DataCategory::error(), Reason::beforeSend(), 1); + ClientReportAggregator::getInstance()->addEvent(Event::createEvent(), Reason::beforeSend()); + + $this->assertClientReports([]); + } } diff --git a/tests/ClientReport/ClientReportAssertions.php b/tests/ClientReport/ClientReportAssertions.php new file mode 100644 index 000000000..84a1e4d03 --- /dev/null +++ b/tests/ClientReport/ClientReportAssertions.php @@ -0,0 +1,46 @@ +> $expected the quantities keyed by category and reason + */ + protected function assertClientReports(array $expected): void + { + $reflectionProperty = new \ReflectionProperty(ClientReportAggregator::class, 'reports'); + if (\PHP_VERSION_ID < 80100) { + $reflectionProperty->setAccessible(true); + } + /** @var array> $actual */ + $actual = $reflectionProperty->getValue(ClientReportAggregator::getInstance()); + if (\PHP_VERSION_ID < 80100) { + $reflectionProperty->setAccessible(false); + } + + $this->assertSame(self::sortClientReports($expected), self::sortClientReports($actual)); + } + + /** + * @param array> $reports + * + * @return array> + */ + private static function sortClientReports(array $reports): array + { + ksort($reports); + + foreach ($reports as &$reasons) { + ksort($reasons); + } + + return $reports; + } +} diff --git a/tests/OptionsTest.php b/tests/OptionsTest.php index 48fc1da18..4956a7230 100644 --- a/tests/OptionsTest.php +++ b/tests/OptionsTest.php @@ -386,6 +386,13 @@ static function (): void {}, 'setSendDefaultPii', ]; + yield [ + 'send_client_reports', + true, + 'shouldSendClientReports', + 'setSendClientReports', + ]; + yield [ 'data_collection', (new DataCollectionOptions())->setUserInfo(false), @@ -650,6 +657,7 @@ public function testDefaultOptionValues(): void 'in_app_exclude' => [], 'in_app_include' => [], 'send_default_pii' => false, + 'send_client_reports' => false, 'data_collection' => null, 'max_value_length' => 1024, 'max_breadth' => null, diff --git a/tests/SentrySdkExtension.php b/tests/SentrySdkExtension.php index 145e35251..4d93d6e74 100644 --- a/tests/SentrySdkExtension.php +++ b/tests/SentrySdkExtension.php @@ -5,6 +5,7 @@ namespace Sentry\Tests; use PHPUnit\Runner\BeforeTestHook as BeforeTestHookInterface; +use Sentry\ClientReport\ClientReportAggregator; use Sentry\Integration\IntegrationRegistry; use Sentry\SentrySdk; use Sentry\State\Scope; @@ -68,5 +69,14 @@ public function executeBeforeTest(string $test): void if (\PHP_VERSION_ID < 80100) { $reflectionProperty->setAccessible(false); } + + $reflectionProperty = new \ReflectionProperty(ClientReportAggregator::class, 'instance'); + if (\PHP_VERSION_ID < 80100) { + $reflectionProperty->setAccessible(true); + } + $reflectionProperty->setValue(null, null); + if (\PHP_VERSION_ID < 80100) { + $reflectionProperty->setAccessible(false); + } } }