Introduce before_send_transaction option (#1424)

Fixes #1423
getsentry · Nov 14, 2022 · 8caae87 · 8caae87
1 parent b455e35
commit 8caae87
Show file tree

Hide file tree

Showing 6 changed files with 146 additions and 14 deletions.
diff --git 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
@@ -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\\<string, callable\\(\\)\\: mixed\\> 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
@@ -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
@@ -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
@@ -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
@@ -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'],