From 8caae8758e6190be463e3af389b6b6b8add51b35 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Mon, 14 Nov 2022 13:04:40 +0100 Subject: [PATCH] Introduce `before_send_transaction` option (#1424) Fixes https://github.com/getsentry/sentry-php/issues/1423 --- CHANGELOG.md | 10 +++++--- phpstan-baseline.neon | 6 ++++- src/Client.php | 47 +++++++++++++++++++++++++++++------ src/Options.php | 30 ++++++++++++++++++++++ tests/ClientTest.php | 58 ++++++++++++++++++++++++++++++++++++++++++- tests/OptionsTest.php | 9 +++++++ 6 files changed, 146 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78e1e09c9..f3564ce6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,13 @@ ## Unreleased +- feat: Add `before_send_transaction` option (#1424) + ## 3.11.0 (2022-10-25) -fix: Only include the transaction name to the DSC if it has good quality (#1410) -ref: Enable the ModulesIntegration by default (#1415) -ref: Expose the ExceptionMechanism through the event hint (#1416) +- fix: Only include the transaction name to the DSC if it has good quality (#1410) +- ref: Enable the ModulesIntegration by default (#1415) +- ref: Expose the ExceptionMechanism through the event hint (#1416) ## 3.10.0 (2022-10-19) @@ -20,7 +22,7 @@ ref: Expose the ExceptionMechanism through the event hint (#1416) ## 3.9.0 (2022-10-05) -- feat: Add tracePropagationTargets option (#1396) +- feat: Add `trace_propagation_targets` option (#1396) - feat: Expose a function to retrieve the URL of the CSP endpoint (#1378) - feat: Add support for Dynamic Sampling (#1360) - Add `segment` to `UserDataBag` diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 3b6e79699..42d41a239 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -110,6 +110,11 @@ parameters: count: 1 path: src/Options.php + - + message: "#^Method Sentry\\\\Options\\:\\:getBeforeSendTransactionCallback\\(\\) should return callable\\(Sentry\\\\Event, Sentry\\\\EventHint\\|null\\)\\: Sentry\\\\Event\\|null but returns mixed\\.$#" + count: 1 + path: src/Options.php + - message: "#^Method Sentry\\\\Options\\:\\:getClassSerializers\\(\\) should return array\\ but returns mixed\\.$#" count: 1 @@ -374,4 +379,3 @@ parameters: message: "#^Method Sentry\\\\State\\\\HubInterface\\:\\:startTransaction\\(\\) invoked with 2 parameters, 1 required\\.$#" count: 1 path: src/functions.php - diff --git a/src/Client.php b/src/Client.php index e91926c9b..f32b099df 100644 --- a/src/Client.php +++ b/src/Client.php @@ -288,28 +288,59 @@ private function prepareEvent(Event $event, ?EventHint $hint = null, ?Scope $sco } if (null !== $scope) { - $previousEvent = $event; + $beforeEventProcessors = $event; $event = $scope->applyToEvent($event, $hint); if (null === $event) { - $this->logger->info('The event will be discarded because one of the event processors returned "null".', ['event' => $previousEvent]); + $this->logger->info( + 'The event will be discarded because one of the event processors returned "null".', + ['event' => $beforeEventProcessors] + ); return null; } } - if (!$isTransaction) { - $previousEvent = $event; - $event = ($this->options->getBeforeSendCallback())($event, $hint); + $beforeSendCallback = $event; + $event = $this->applyBeforeSendCallback($event, $hint); - if (null === $event) { - $this->logger->info('The event will be discarded because the "before_send" callback returned "null".', ['event' => $previousEvent]); - } + if (null === $event) { + $this->logger->info( + sprintf( + 'The event will be discarded because the "%s" callback returned "null".', + $this->getBeforeSendCallbackName($beforeSendCallback) + ), + ['event' => $beforeSendCallback] + ); + } + + return $event; + } + + private function applyBeforeSendCallback(Event $event, ?EventHint $hint): ?Event + { + if ($event->getType() === EventType::event()) { + return ($this->options->getBeforeSendCallback())($event, $hint); + } + + if ($event->getType() === EventType::transaction()) { + return ($this->options->getBeforeSendTransactionCallback())($event, $hint); } return $event; } + private function getBeforeSendCallbackName(Event $event): string + { + $beforeSendCallbackName = 'before_send'; + + if ($event->getType() === EventType::transaction()) { + $beforeSendCallbackName = 'before_send_transaction'; + } + + return $beforeSendCallbackName; + } + /** * Optionally adds a missing stacktrace to the Event if the client is configured to do so. * diff --git a/src/Options.php b/src/Options.php index d9a980258..c3028e16d 100644 --- a/src/Options.php +++ b/src/Options.php @@ -396,6 +396,32 @@ public function setBeforeSendCallback(callable $callback): void $this->options = $this->resolver->resolve($options); } + /** + * Gets a callback that will be invoked before an transaction is sent to the server. + * If `null` is returned it won't be sent. + * + * @psalm-return callable(Event, ?EventHint): ?Event + */ + public function getBeforeSendTransactionCallback(): callable + { + return $this->options['before_send_transaction']; + } + + /** + * Sets a callable to be called to decide whether an transaction should + * be captured or not. + * + * @param callable $callback The callable + * + * @psalm-param callable(Event, ?EventHint): ?Event $callback + */ + public function setBeforeSendTransactionCallback(callable $callback): void + { + $options = array_merge($this->options, ['before_send_transaction' => $callback]); + + $this->options = $this->resolver->resolve($options); + } + /** * Gets an allow list of trace propagation targets. * @@ -801,6 +827,9 @@ private function configureOptions(OptionsResolver $resolver): void 'before_send' => static function (Event $event): Event { return $event; }, + 'before_send_transaction' => static function (Event $transaction): Event { + return $transaction; + }, 'trace_propagation_targets' => [], 'tags' => [], 'error_types' => null, @@ -836,6 +865,7 @@ private function configureOptions(OptionsResolver $resolver): void $resolver->setAllowedTypes('dsn', ['null', 'string', 'bool', Dsn::class]); $resolver->setAllowedTypes('server_name', 'string'); $resolver->setAllowedTypes('before_send', ['callable']); + $resolver->setAllowedTypes('before_send_transaction', ['callable']); $resolver->setAllowedTypes('trace_propagation_targets', 'string[]'); $resolver->setAllowedTypes('tags', 'string[]'); $resolver->setAllowedTypes('error_types', ['null', 'int']); diff --git a/tests/ClientTest.php b/tests/ClientTest.php index cbc3334de..4bee02020 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -510,6 +510,39 @@ public function processEventChecksBeforeSendOptionDataProvider(): \Generator ]; } + /** + * @dataProvider processEventChecksBeforeSendTransactionOptionDataProvider + */ + public function testProcessEventChecksBeforeSendTransactionOption(Event $event, bool $expectedBeforeSendCall): void + { + $beforeSendTransactionCalled = false; + $options = [ + 'before_send_transaction' => static function () use (&$beforeSendTransactionCalled) { + $beforeSendTransactionCalled = true; + + return null; + }, + ]; + + $client = ClientBuilder::create($options)->getClient(); + $client->captureEvent($event); + + $this->assertSame($expectedBeforeSendCall, $beforeSendTransactionCalled); + } + + public function processEventChecksBeforeSendTransactionOptionDataProvider(): \Generator + { + yield [ + Event::createEvent(), + false, + ]; + + yield [ + Event::createTransaction(), + true, + ]; + } + /** * @dataProvider processEventDiscardsEventWhenItIsSampledDueToSampleRateOptionDataProvider */ @@ -574,7 +607,30 @@ public function testProcessEventDiscardsEventWhenBeforeSendCallbackReturnsNull() ->setLogger($logger) ->getClient(); - $client->captureMessage('foo'); + $client->captureEvent(Event::createEvent()); + } + + public function testProcessEventDiscardsEventWhenBeforeSendTransactionCallbackReturnsNull(): void + { + /** @var LoggerInterface&MockObject $logger */ + $logger = $this->createMock(LoggerInterface::class); + $logger->expects($this->once()) + ->method('info') + ->with('The event will be discarded because the "before_send_transaction" callback returned "null".', $this->callback(static function (array $context): bool { + return isset($context['event']) && $context['event'] instanceof Event; + })); + + $options = [ + 'before_send_transaction' => static function () { + return null; + }, + ]; + + $client = ClientBuilder::create($options) + ->setLogger($logger) + ->getClient(); + + $client->captureEvent(Event::createTransaction()); } public function testProcessEventDiscardsEventWhenEventProcessorReturnsNull(): void diff --git a/tests/OptionsTest.php b/tests/OptionsTest.php index b1635da5c..85b6aed04 100644 --- a/tests/OptionsTest.php +++ b/tests/OptionsTest.php @@ -248,6 +248,15 @@ static function (): void {}, null, ]; + yield [ + 'before_send_transaction', + static function (): void {}, + 'getBeforeSendTransactionCallback', + 'setBeforeSendTransactionCallback', + null, + null, + ]; + yield [ 'trace_propagation_targets', ['www.example.com'],