Dont overwrite Event properties

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Fix unwanted call to the `before_send` callback with transaction events, use `traces_sampler` instead to filter transactions (#1158)
 - Fix the `logger` option not being applied to the event object (#1165)
+- Fix a bug that made some event attributes being overwritten by option config values when calling `captureEvent()` (#1148)
 
 ## 3.1.1 (2020-12-07)
 

src/Client.php

@@ -227,10 +227,19 @@ private function prepareEvent(Event $event, ?EventHint $hint = null, ?Scope $sco
 
         $event->setSdkIdentifier($this->sdkIdentifier);
         $event->setSdkVersion($this->sdkVersion);
-        $event->setServerName($this->options->getServerName());
-        $event->setRelease($this->options->getRelease());
-        $event->setTags($this->options->getTags());
-        $event->setEnvironment($this->options->getEnvironment());
+        $event->setTags(array_merge($this->options->getTags(), $event->getTags()));
+
+        if (null === $event->getServerName()) {
+            $event->setServerName($this->options->getServerName());
+        }
+
+        if (null === $event->getRelease()) {
+            $event->setRelease($this->options->getRelease());
+        }
+
+        if (null === $event->getEnvironment()) {
+            $event->setEnvironment($this->options->getEnvironment());
+        }
 
         if (null === $event->getLogger()) {
             $event->setLogger($this->options->getLogger());

tests/ClientTest.php

@@ -14,6 +14,7 @@
 use Sentry\ClientBuilder;
 use Sentry\Event;
 use Sentry\EventHint;
+use Sentry\EventId;
 use Sentry\ExceptionMechanism;
 use Sentry\Frame;
 use Sentry\Integration\IntegrationInterface;
@@ -28,7 +29,6 @@
 use Sentry\State\Scope;
 use Sentry\Transport\TransportFactoryInterface;
 use Sentry\Transport\TransportInterface;
-use Sentry\UserDataBag;
 
 final class ClientTest extends TestCase
 {
@@ -132,29 +132,59 @@ public function testCaptureException(): void
         $this->assertNotNull($client->captureException($exception));
     }
 
-    public function testCaptureEvent(): void
+    /**
+     * @dataProvider captureEventDataProvider
+     */
+    public function testCaptureEvent(array $options, Event $event, Event $expectedEvent): void
     {
-        /** @var TransportInterface&MockObject $transport */
         $transport = $this->createMock(TransportInterface::class);
         $transport->expects($this->once())
             ->method('send')
-            ->willReturnCallback(static function (Event $event): FulfilledPromise {
+            ->willReturnCallback(function (Event $event) use ($expectedEvent): FulfilledPromise {
+                $this->assertEquals($expectedEvent, $event);
+
                 return new FulfilledPromise(new Response(ResponseStatus::success(), $event));
             });
 
-        $client = ClientBuilder::create()
+        $client = ClientBuilder::create($options)
             ->setTransportFactory($this->createTransportFactory($transport))
             ->getClient();
 
-        $event = Event::createEvent();
-        $event->setTransaction('foo bar');
-        $event->setLevel(Severity::debug());
-        $event->setLogger('foo');
-        $event->setTags(['foo', 'bar']);
-        $event->setExtra(['foo' => 'bar']);
-        $event->setUser(UserDataBag::createFromUserIdentifier('foo'));
+        $this->assertSame($event->getId(), $client->captureEvent($event));
+    }
+
+    public function captureEventDataProvider(): \Generator
+    {
+        $eventId = EventId::generate();
+        $event = Event::createEvent($eventId);
+
+        yield 'Options set && no event properties set => use options' => [
+            [
+                'server_name' => 'example.com',
+                'release' => '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33',
+                'environment' => 'development',
+                'tags' => ['context' => 'development'],
+            ],
+            $event,
+            $event,
+        ];
 
-        $this->assertNotNull($client->captureEvent($event));
+        $event = Event::createEvent($eventId);
+        $event->setServerName('foo.example.com');
+        $event->setRelease('721e41770371db95eee98ca2707686226b993eda');
+        $event->setEnvironment('production');
+        $event->setTags(['context' => 'production']);
+
+        yield 'Options set && event properties set => event properties override options' => [
+            [
+                'server_name' => 'example.com',
+                'release' => '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33',
+                'environment' => 'development',
+                'tags' => ['context' => 'development', 'ios_version' => '14.0'],
+            ],
+            $event,
+            $event,
+        ];
     }
 
     /**
@@ -454,44 +484,6 @@ public function testFlush(): void
         $this->assertTrue($promise->wait());
     }
 
-    public function testBuildEventWithDefaultValues(): void
-    {
-        $options = new Options();
-        $options->setServerName('testServerName');
-        $options->setRelease('testRelease');
-        $options->setTags(['test' => 'tag']);
-        $options->setEnvironment('testEnvironment');
-        $options->setLogger('app.logger');
-
-        /** @var TransportInterface&MockObject $transport */
-        $transport = $this->createMock(TransportInterface::class);
-        $transport->expects($this->once())
-            ->method('send')
-            ->with($this->callback(function (Event $event) use ($options): bool {
-                $this->assertSame('sentry.sdk.identifier', $event->getSdkIdentifier());
-                $this->assertSame('1.2.3', $event->getSdkVersion());
-                $this->assertSame($options->getServerName(), $event->getServerName());
-                $this->assertSame($options->getRelease(), $event->getRelease());
-                $this->assertSame($options->getTags(), $event->getTags());
-                $this->assertSame($options->getEnvironment(), $event->getEnvironment());
-                $this->assertSame($options->getLogger(), $event->getLogger());
-                $this->assertNull($event->getStacktrace());
-
-                return true;
-            }));
-
-        $client = new Client(
-            $options,
-            $transport,
-            'sentry.sdk.identifier',
-            '1.2.3',
-            $this->createMock(SerializerInterface::class),
-            $this->createMock(RepresentationSerializerInterface::class)
-        );
-
-        $client->captureEvent(Event::createEvent());
-    }
-
     public function testBuildEventInCLIDoesntSetTransaction(): void
     {
         /** @var TransportInterface&MockObject $transport */