From 18fb717ddd7d58262c1ef377a77561f096ba54e8 Mon Sep 17 00:00:00 2001
From: Stefano Arlandini <sarlandini@alice.it>
Date: Sun, 27 Dec 2020 01:03:26 +0100
Subject: [PATCH 1/2] Avoid calling the before_send callback with transaction
 events

---
 CHANGELOG.md         |  4 ++++
 src/Client.php       | 13 ++++++++-----
 src/EventHint.php    |  2 +-
 tests/ClientTest.php | 40 ++++++++++++++++++++++++----------------
 4 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 26abeaae30..b3d50e249a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # CHANGELOG
 
+## Unreleased
+
+- Fix unwanted call to the `before_send` callback with transaction events (#1158)
+
 ## 3.1.1 (2020-12-07)
 
 - Add support for PHP 8.0 (#1087)
diff --git a/src/Client.php b/src/Client.php
index eac510f772..51606745ba 100644
--- a/src/Client.php
+++ b/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;
diff --git a/src/EventHint.php b/src/EventHint.php
index 18bc61a8a3..c5b8abe8ab 100644
--- a/src/EventHint.php
+++ b/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
      */
diff --git a/tests/ClientTest.php b/tests/ClientTest.php
index 0e5aa45af2..3c2b219b83 100644
--- a/tests/ClientTest.php
+++ b/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,
+        ];
     }
 
     /**

From 7337986a981e66276c463e6b78e01461e86d8dba Mon Sep 17 00:00:00 2001
From: Stefano Arlandini <sarlandini@alice.it>
Date: Sun, 27 Dec 2020 13:48:36 +0100
Subject: [PATCH 2/2] Update the CHANGELOG with alternative solution

---
 CHANGELOG.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b3d50e249a..cf99830746 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,7 +2,7 @@
 
 ## Unreleased
 
-- Fix unwanted call to the `before_send` callback with transaction events (#1158)
+- 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)