Avoid calling the before_send callback with transaction events

CHANGELOG.md

@@ -1,5 +1,9 @@
 # CHANGELOG
 
+## Unreleased
+
+- Fix unwanted call to the `before_send` callback with transaction events, use `traces_sampler` instead to filter transactions (#1158)
+
 ## 3.1.1 (2020-12-07)
 
 - Add support for PHP 8.0 (#1087)

src/Client.php

@@ -232,9 +232,10 @@ private function prepareEvent(Event $event, ?EventHint $hint = null, ?Scope $sco
         $event->setTags($this->options->getTags());
         $event->setEnvironment($this->options->getEnvironment());
 
+        $isTransaction = EventType::transaction() === $event->getType();
         $sampleRate = $this->options->getSampleRate();
 
-        if (EventType::transaction() !== $event->getType() && $sampleRate < 1 && mt_rand(1, 100) / 100.0 > $sampleRate) {
+        if (!$isTransaction && $sampleRate < 1 && mt_rand(1, 100) / 100.0 > $sampleRate) {
             $this->logger->info('The event will be discarded because it has been sampled.', ['event' => $event]);
 
             return null;
@@ -251,11 +252,13 @@ private function prepareEvent(Event $event, ?EventHint $hint = null, ?Scope $sco
             }
         }
 
-        $previousEvent = $event;
-        $event = ($this->options->getBeforeSendCallback())($event);
+        if (!$isTransaction) {
+            $previousEvent = $event;
+            $event = ($this->options->getBeforeSendCallback())($event);
 
-        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('The event will be discarded because the "before_send" callback returned "null".', ['event' => $previousEvent]);
+            }
         }
 
         return $event;

src/EventHint.php

@@ -35,7 +35,7 @@ final class EventHint
      *
      * @psalm-param array{
      *     exception?: \Throwable,
-     *     stacktrace?: Event,
+     *     stacktrace?: Stacktrace|null,
      *     extra?: array<string, mixed>
      * } $hintData
      */

tests/ClientTest.php

@@ -291,29 +291,37 @@ public function testCaptureLastErrorDoesNothingWhenThereIsNoError(): void
         $this->assertNull($client->captureLastError());
     }
 
-    public function testSendChecksBeforeSendOption(): void
+    /**
+     * @dataProvider processEventChecksBeforeSendOptionDataProvider
+     */
+    public function testProcessEventChecksBeforeSendOption(Event $event, bool $expectedBeforeSendCall): void
     {
         $beforeSendCalled = false;
+        $options = [
+            'before_send' => static function () use (&$beforeSendCalled) {
+                $beforeSendCalled = true;
 
-        /** @var TransportInterface&MockObject $transport */
-        $transport = $this->createMock(TransportInterface::class);
-        $transport->expects($this->never())
-            ->method('send');
-
-        $options = new Options(['dsn' => 'http://public:secret@example.com/1']);
-        $options->setBeforeSendCallback(function () use (&$beforeSendCalled) {
-            $beforeSendCalled = true;
+                return null;
+            },
+        ];
 
-            return null;
-        });
+        $client = ClientBuilder::create($options)->getClient();
+        $client->captureEvent($event);
 
-        $client = (new ClientBuilder($options))
-            ->setTransportFactory($this->createTransportFactory($transport))
-            ->getClient();
+        $this->assertSame($expectedBeforeSendCall, $beforeSendCalled);
+    }
 
-        $client->captureEvent(Event::createEvent());
+    public function processEventChecksBeforeSendOptionDataProvider(): \Generator
+    {
+        yield [
+            Event::createEvent(),
+            true,
+        ];
 
-        $this->assertTrue($beforeSendCalled);
+        yield [
+            Event::createTransaction(),
+            false,
+        ];
     }
 
     /**