From b632b13d607967ecbe9bf1f87ef1037e647ef0e1 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Fri, 19 Feb 2021 20:15:30 +0100 Subject: [PATCH 1/5] Make the environment always required --- src/Event.php | 14 ++++++++++---- src/Options.php | 12 ++++++++++-- src/Serializer/PayloadSerializer.php | 5 +---- tests/Serializer/PayloadSerializerTest.php | 8 ++++++-- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/Event.php b/src/Event.php index 60bf4ae8e5..c9f7e36b6c 100644 --- a/src/Event.php +++ b/src/Event.php @@ -16,6 +16,8 @@ */ final class Event { + public const DEFAULT_ENVIRONMENT = 'default'; + /** * @var EventId The ID */ @@ -74,9 +76,9 @@ final class Event private $messageParams = []; /** - * @var string|null The environment where this event generated (e.g. production) + * @var string The environment where this event generated (e.g. production) */ - private $environment; + private $environment = self::DEFAULT_ENVIRONMENT; /** * @var array A list of relevant modules and their versions @@ -566,7 +568,7 @@ public function setFingerprint(array $fingerprint): void /** * Gets the environment in which this event was generated. */ - public function getEnvironment(): ?string + public function getEnvironment(): string { return $this->environment; } @@ -578,7 +580,11 @@ public function getEnvironment(): ?string */ public function setEnvironment(?string $environment): void { - $this->environment = $environment; + if (null === $environment) { + @trigger_error('Setting the environment to a null value is deprecated since version 3.1 and will not work in 4.0.', E_USER_DEPRECATED); + } + + $this->environment = $environment ?? self::DEFAULT_ENVIRONMENT; } /** diff --git a/src/Options.php b/src/Options.php index 43c1963fe7..024b4f4c87 100644 --- a/src/Options.php +++ b/src/Options.php @@ -207,7 +207,7 @@ public function setEnableCompression(bool $enabled): void /** * Gets the environment. */ - public function getEnvironment(): ?string + public function getEnvironment(): string { return $this->options['environment']; } @@ -707,7 +707,7 @@ private function configureOptions(OptionsResolver $resolver): void 'attach_stacktrace' => false, 'context_lines' => 5, 'enable_compression' => true, - 'environment' => $_SERVER['SENTRY_ENVIRONMENT'] ?? null, + 'environment' => $_SERVER['SENTRY_ENVIRONMENT'] ?? Event::DEFAULT_ENVIRONMENT, 'logger' => 'php', 'release' => $_SERVER['SENTRY_RELEASE'] ?? null, 'dsn' => $_SERVER['SENTRY_DSN'] ?? null, @@ -775,6 +775,14 @@ private function configureOptions(OptionsResolver $resolver): void return $value; }); + $resolver->setNormalizer('environment', static function (SymfonyOptions $options, ?string $value): string { + if (null === $value) { + @trigger_error('Setting the "environment" option to a null value is deprecated since version 3.1 and will not work in 4.0.', E_USER_DEPRECATED); + } + + return $value ?? Event::DEFAULT_ENVIRONMENT; + }); + $resolver->setNormalizer('prefixes', function (SymfonyOptions $options, array $value) { return array_map([$this, 'normalizeAbsolutePath'], $value); }); diff --git a/src/Serializer/PayloadSerializer.php b/src/Serializer/PayloadSerializer.php index 12071a9dbf..bd1032f27f 100644 --- a/src/Serializer/PayloadSerializer.php +++ b/src/Serializer/PayloadSerializer.php @@ -38,6 +38,7 @@ private function serializeAsEvent(Event $event): string 'event_id' => (string) $event->getId(), 'timestamp' => $event->getTimestamp(), 'platform' => 'php', + 'environment' => $event->getEnvironment(), 'sdk' => [ 'name' => $event->getSdkIdentifier(), 'version' => $event->getSdkVersion(), @@ -68,10 +69,6 @@ private function serializeAsEvent(Event $event): string $result['release'] = $event->getRelease(); } - if (null !== $event->getEnvironment()) { - $result['environment'] = $event->getEnvironment(); - } - if (!empty($event->getFingerprint())) { $result['fingerprint'] = $event->getFingerprint(); } diff --git a/tests/Serializer/PayloadSerializerTest.php b/tests/Serializer/PayloadSerializerTest.php index 0ea26b5028..d4afb9452e 100644 --- a/tests/Serializer/PayloadSerializerTest.php +++ b/tests/Serializer/PayloadSerializerTest.php @@ -68,6 +68,7 @@ public function serializeDataProvider(): iterable "event_id": "fc9442f5aef34234bb22b9a615e30ccd", "timestamp": 1597790835, "platform": "php", + "environment": "default", "sdk": { "name": "sentry.php", "version": "$sdkVersion" @@ -178,6 +179,7 @@ public function serializeDataProvider(): iterable "event_id": "fc9442f5aef34234bb22b9a615e30ccd", "timestamp": 1597790835, "platform": "php", + "environment": "production", "sdk": { "name": "sentry.php", "version": "$sdkVersion" @@ -188,7 +190,6 @@ public function serializeDataProvider(): iterable "transaction": "/users//", "server_name": "foo.example.com", "release": "721e41770371db95eee98ca2707686226b993eda", - "environment": "production", "fingerprint": [ "myrpc", "POST", @@ -326,6 +327,7 @@ public function serializeDataProvider(): iterable "event_id": "fc9442f5aef34234bb22b9a615e30ccd", "timestamp": 1597790835, "platform": "php", + "environment": "default", "sdk": { "name": "sentry.php", "version": "$sdkVersion" @@ -347,6 +349,7 @@ public function serializeDataProvider(): iterable "event_id": "fc9442f5aef34234bb22b9a615e30ccd", "timestamp": 1597790835, "platform": "php", + "environment": "default", "sdk": { "name": "sentry.php", "version": "$sdkVersion" @@ -372,6 +375,7 @@ public function serializeDataProvider(): iterable "event_id": "fc9442f5aef34234bb22b9a615e30ccd", "timestamp": 1597790835, "platform": "php", + "environment": "default", "sdk": { "name": "sentry.php", "version": "$sdkVersion" @@ -417,7 +421,7 @@ public function serializeDataProvider(): iterable << Date: Fri, 19 Feb 2021 20:16:10 +0100 Subject: [PATCH 2/5] Update the CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f844b0516..ee0327effd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Deprecate the `logger` option (#1167) - Pass the event hint from the `capture*()` methods down to the `before_send` callback (#1138) - Deprecate the `tags` option, see the [docs](https://docs.sentry.io/platforms/php/guides/laravel/enriching-events/tags/) for other ways to set tags (#1174) +- Make the `environment` option and field always required (#1116) ## 3.1.5 (2021-02-18) From c55a810906ec3f281f5a9fe77db1d1cbdd3fb581 Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Fri, 19 Feb 2021 20:16:53 +0100 Subject: [PATCH 3/5] Revert deprecation when setting null as value of the environment option --- src/Client.php | 2 +- src/Event.php | 12 ++++-------- src/Options.php | 14 +++----------- src/Serializer/PayloadSerializer.php | 5 ++++- tests/Serializer/PayloadSerializerTest.php | 6 +----- 5 files changed, 13 insertions(+), 26 deletions(-) diff --git a/src/Client.php b/src/Client.php index f40e68fecf..87ceca46b9 100644 --- a/src/Client.php +++ b/src/Client.php @@ -242,7 +242,7 @@ private function prepareEvent(Event $event, ?EventHint $hint = null, ?Scope $sco } if (null === $event->getEnvironment()) { - $event->setEnvironment($this->options->getEnvironment()); + $event->setEnvironment($this->options->getEnvironment() ?? Event::DEFAULT_ENVIRONMENT); } if (null === $event->getLogger()) { diff --git a/src/Event.php b/src/Event.php index c9f7e36b6c..313fb97de5 100644 --- a/src/Event.php +++ b/src/Event.php @@ -16,7 +16,7 @@ */ final class Event { - public const DEFAULT_ENVIRONMENT = 'default'; + public const DEFAULT_ENVIRONMENT = 'production'; /** * @var EventId The ID @@ -76,9 +76,9 @@ final class Event private $messageParams = []; /** - * @var string The environment where this event generated (e.g. production) + * @var string|null The environment where this event generated (e.g. production) */ - private $environment = self::DEFAULT_ENVIRONMENT; + private $environment; /** * @var array A list of relevant modules and their versions @@ -568,7 +568,7 @@ public function setFingerprint(array $fingerprint): void /** * Gets the environment in which this event was generated. */ - public function getEnvironment(): string + public function getEnvironment(): ?string { return $this->environment; } @@ -580,10 +580,6 @@ public function getEnvironment(): string */ public function setEnvironment(?string $environment): void { - if (null === $environment) { - @trigger_error('Setting the environment to a null value is deprecated since version 3.1 and will not work in 4.0.', E_USER_DEPRECATED); - } - $this->environment = $environment ?? self::DEFAULT_ENVIRONMENT; } diff --git a/src/Options.php b/src/Options.php index 024b4f4c87..8b5c5bd17f 100644 --- a/src/Options.php +++ b/src/Options.php @@ -207,7 +207,7 @@ public function setEnableCompression(bool $enabled): void /** * Gets the environment. */ - public function getEnvironment(): string + public function getEnvironment(): ?string { return $this->options['environment']; } @@ -311,7 +311,7 @@ public function getRelease(): ?string /** * Sets the release tag to be passed with every event sent to Sentry. * - * @param string $release The release + * @param string|null $release The release */ public function setRelease(?string $release): void { @@ -707,7 +707,7 @@ private function configureOptions(OptionsResolver $resolver): void 'attach_stacktrace' => false, 'context_lines' => 5, 'enable_compression' => true, - 'environment' => $_SERVER['SENTRY_ENVIRONMENT'] ?? Event::DEFAULT_ENVIRONMENT, + 'environment' => $_SERVER['SENTRY_ENVIRONMENT'] ?? null, 'logger' => 'php', 'release' => $_SERVER['SENTRY_RELEASE'] ?? null, 'dsn' => $_SERVER['SENTRY_DSN'] ?? null, @@ -775,14 +775,6 @@ private function configureOptions(OptionsResolver $resolver): void return $value; }); - $resolver->setNormalizer('environment', static function (SymfonyOptions $options, ?string $value): string { - if (null === $value) { - @trigger_error('Setting the "environment" option to a null value is deprecated since version 3.1 and will not work in 4.0.', E_USER_DEPRECATED); - } - - return $value ?? Event::DEFAULT_ENVIRONMENT; - }); - $resolver->setNormalizer('prefixes', function (SymfonyOptions $options, array $value) { return array_map([$this, 'normalizeAbsolutePath'], $value); }); diff --git a/src/Serializer/PayloadSerializer.php b/src/Serializer/PayloadSerializer.php index bd1032f27f..1e9cbae634 100644 --- a/src/Serializer/PayloadSerializer.php +++ b/src/Serializer/PayloadSerializer.php @@ -38,13 +38,16 @@ private function serializeAsEvent(Event $event): string 'event_id' => (string) $event->getId(), 'timestamp' => $event->getTimestamp(), 'platform' => 'php', - 'environment' => $event->getEnvironment(), 'sdk' => [ 'name' => $event->getSdkIdentifier(), 'version' => $event->getSdkVersion(), ], ]; + if (null !== $event->getEnvironment()) { + $result['environment'] = $event->getEnvironment(); + } + if (null !== $event->getStartTimestamp()) { $result['start_timestamp'] = $event->getStartTimestamp(); } diff --git a/tests/Serializer/PayloadSerializerTest.php b/tests/Serializer/PayloadSerializerTest.php index d4afb9452e..2384561052 100644 --- a/tests/Serializer/PayloadSerializerTest.php +++ b/tests/Serializer/PayloadSerializerTest.php @@ -68,7 +68,6 @@ public function serializeDataProvider(): iterable "event_id": "fc9442f5aef34234bb22b9a615e30ccd", "timestamp": 1597790835, "platform": "php", - "environment": "default", "sdk": { "name": "sentry.php", "version": "$sdkVersion" @@ -327,7 +326,6 @@ public function serializeDataProvider(): iterable "event_id": "fc9442f5aef34234bb22b9a615e30ccd", "timestamp": 1597790835, "platform": "php", - "environment": "default", "sdk": { "name": "sentry.php", "version": "$sdkVersion" @@ -349,7 +347,6 @@ public function serializeDataProvider(): iterable "event_id": "fc9442f5aef34234bb22b9a615e30ccd", "timestamp": 1597790835, "platform": "php", - "environment": "default", "sdk": { "name": "sentry.php", "version": "$sdkVersion" @@ -375,7 +372,6 @@ public function serializeDataProvider(): iterable "event_id": "fc9442f5aef34234bb22b9a615e30ccd", "timestamp": 1597790835, "platform": "php", - "environment": "default", "sdk": { "name": "sentry.php", "version": "$sdkVersion" @@ -421,7 +417,7 @@ public function serializeDataProvider(): iterable << Date: Mon, 22 Feb 2021 18:52:17 +0100 Subject: [PATCH 4/5] Fix CR issues --- CHANGELOG.md | 2 +- src/Event.php | 2 +- src/Serializer/PayloadSerializer.php | 8 ++++---- tests/Serializer/PayloadSerializerTest.php | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee0327effd..30e4c60902 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ - Deprecate the `logger` option (#1167) - Pass the event hint from the `capture*()` methods down to the `before_send` callback (#1138) - Deprecate the `tags` option, see the [docs](https://docs.sentry.io/platforms/php/guides/laravel/enriching-events/tags/) for other ways to set tags (#1174) -- Make the `environment` option and field always required (#1116) +- Make sure the `environment` field is set to `production` if it has not been overridden explicitly (#1116) ## 3.1.5 (2021-02-18) diff --git a/src/Event.php b/src/Event.php index 313fb97de5..6f0e582d4d 100644 --- a/src/Event.php +++ b/src/Event.php @@ -580,7 +580,7 @@ public function getEnvironment(): ?string */ public function setEnvironment(?string $environment): void { - $this->environment = $environment ?? self::DEFAULT_ENVIRONMENT; + $this->environment = $environment; } /** diff --git a/src/Serializer/PayloadSerializer.php b/src/Serializer/PayloadSerializer.php index 1e9cbae634..12071a9dbf 100644 --- a/src/Serializer/PayloadSerializer.php +++ b/src/Serializer/PayloadSerializer.php @@ -44,10 +44,6 @@ private function serializeAsEvent(Event $event): string ], ]; - if (null !== $event->getEnvironment()) { - $result['environment'] = $event->getEnvironment(); - } - if (null !== $event->getStartTimestamp()) { $result['start_timestamp'] = $event->getStartTimestamp(); } @@ -72,6 +68,10 @@ private function serializeAsEvent(Event $event): string $result['release'] = $event->getRelease(); } + if (null !== $event->getEnvironment()) { + $result['environment'] = $event->getEnvironment(); + } + if (!empty($event->getFingerprint())) { $result['fingerprint'] = $event->getFingerprint(); } diff --git a/tests/Serializer/PayloadSerializerTest.php b/tests/Serializer/PayloadSerializerTest.php index 2384561052..0ea26b5028 100644 --- a/tests/Serializer/PayloadSerializerTest.php +++ b/tests/Serializer/PayloadSerializerTest.php @@ -178,7 +178,6 @@ public function serializeDataProvider(): iterable "event_id": "fc9442f5aef34234bb22b9a615e30ccd", "timestamp": 1597790835, "platform": "php", - "environment": "production", "sdk": { "name": "sentry.php", "version": "$sdkVersion" @@ -189,6 +188,7 @@ public function serializeDataProvider(): iterable "transaction": "/users//", "server_name": "foo.example.com", "release": "721e41770371db95eee98ca2707686226b993eda", + "environment": "production", "fingerprint": [ "myrpc", "POST", From 2797e5cf38a0ed02788e5c4fd775d18eaa98eefb Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Mon, 22 Feb 2021 19:05:28 +0100 Subject: [PATCH 5/5] Improve unit tests --- tests/ClientTest.php | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 461f2aad93..304167725e 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -14,7 +14,6 @@ use Sentry\ClientBuilder; use Sentry\Event; use Sentry\EventHint; -use Sentry\EventId; use Sentry\ExceptionMechanism; use Sentry\Frame; use Sentry\Integration\IntegrationInterface; @@ -204,7 +203,9 @@ public function captureExceptionWithEventHintDataProvider(): \Generator */ public function testCaptureEvent(array $options, Event $event, Event $expectedEvent): void { - $this->expectDeprecation('The option "tags" is deprecated since version 3.2 and will be removed in 4.0. Either set the tags on the scope or on the event.'); + if (isset($options['tags'])) { + $this->expectDeprecation('The option "tags" is deprecated since version 3.2 and will be removed in 4.0. Either set the tags on the scope or on the event.'); + } $transport = $this->createMock(TransportInterface::class); $transport->expects($this->once()) @@ -224,8 +225,7 @@ public function testCaptureEvent(array $options, Event $event, Event $expectedEv public function captureEventDataProvider(): \Generator { - $eventId = EventId::generate(); - $event = Event::createEvent($eventId); + $event = Event::createEvent(); yield 'Options set && no event properties set => use options' => [ [ @@ -238,7 +238,7 @@ public function captureEventDataProvider(): \Generator $event, ]; - $event = Event::createEvent($eventId); + $event = Event::createEvent(); $event->setServerName('foo.example.com'); $event->setRelease('721e41770371db95eee98ca2707686226b993eda'); $event->setEnvironment('production'); @@ -254,6 +254,14 @@ public function captureEventDataProvider(): \Generator $event, $event, ]; + + $event = Event::createEvent(); + + yield 'Environment option set to null && no event property set => fallback to default value' => [ + ['environment' => null], + $event, + $event, + ]; } public function testCaptureEventWithEventHint(): void