From fd5d15d21738cf3671f6c037e7588b48c4c1776c Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Thu, 14 Nov 2024 12:41:55 +0100 Subject: [PATCH 1/7] Use `SENTRY_SPOTLIGHT` as default value for Spotlight options --- phpstan-baseline.neon | 5 ----- src/Options.php | 28 +++++++++++++++++++++++++--- tests/OptionsTest.php | 33 ++++++++++++++++++++++++++++++++- 3 files changed, 57 insertions(+), 9 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index ae65ebb11..f5a9fe9f8 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -260,11 +260,6 @@ parameters: count: 1 path: src/Options.php - - - message: "#^Method Sentry\\\\Options\\:\\:isSpotlightEnabled\\(\\) should return bool but returns mixed\\.$#" - count: 1 - path: src/Options.php - - message: "#^Method Sentry\\\\Options\\:\\:shouldAttachMetricCodeLocations\\(\\) should return bool but returns mixed\\.$#" count: 1 diff --git a/src/Options.php b/src/Options.php index dd2930547..70e924672 100644 --- a/src/Options.php +++ b/src/Options.php @@ -368,7 +368,7 @@ public function setLogger(LoggerInterface $logger): self public function isSpotlightEnabled(): bool { - return $this->options['spotlight']; + return \is_string($this->options['spotlight']) || $this->options['spotlight']; } public function enableSpotlight(bool $enable): self @@ -382,6 +382,10 @@ public function enableSpotlight(bool $enable): self public function getSpotlightUrl(): string { + if (\is_string($this->options['spotlight'])) { + return $this->options['spotlight']; + } + return $this->options['spotlight_url']; } @@ -1127,7 +1131,7 @@ private function configureOptions(OptionsResolver $resolver): void 'context_lines' => 5, 'environment' => $_SERVER['SENTRY_ENVIRONMENT'] ?? null, 'logger' => null, - 'spotlight' => false, + 'spotlight' => $_SERVER['SENTRY_SPOTLIGHT'] ?? null, 'spotlight_url' => 'http://localhost:8969', 'release' => $_SERVER['SENTRY_RELEASE'] ?? $_SERVER['AWS_LAMBDA_FUNCTION_VERSION'] ?? null, 'dsn' => $_SERVER['SENTRY_DSN'] ?? null, @@ -1187,7 +1191,7 @@ private function configureOptions(OptionsResolver $resolver): void $resolver->setAllowedTypes('in_app_exclude', 'string[]'); $resolver->setAllowedTypes('in_app_include', 'string[]'); $resolver->setAllowedTypes('logger', ['null', LoggerInterface::class]); - $resolver->setAllowedTypes('spotlight', 'bool'); + $resolver->setAllowedTypes('spotlight', ['bool', 'string', 'null']); $resolver->setAllowedTypes('spotlight_url', 'string'); $resolver->setAllowedTypes('release', ['null', 'string']); $resolver->setAllowedTypes('dsn', ['null', 'string', 'bool', Dsn::class]); @@ -1229,6 +1233,8 @@ private function configureOptions(OptionsResolver $resolver): void return array_map([$this, 'normalizeAbsolutePath'], $value); }); + $resolver->setNormalizer('spotlight', \Closure::fromCallable([$this, 'normalizeBooleanOrUrl'])); + $resolver->setNormalizer('in_app_exclude', function (SymfonyOptions $options, array $value) { return array_map([$this, 'normalizeAbsolutePath'], $value); }); @@ -1254,6 +1260,22 @@ private function normalizeAbsolutePath(string $value): string return $path; } + /** + * @return bool|string + */ + private function normalizeBooleanOrUrl(SymfonyOptions $options, ?string $booleanOrUrl) + { + if (empty($booleanOrUrl)) { + return false; + } + + if (str_starts_with($booleanOrUrl, 'http')) { + return $booleanOrUrl; + } + + return filter_var($booleanOrUrl, \FILTER_VALIDATE_BOOLEAN); + } + /** * Normalizes the DSN option by parsing the host, public and secret keys and * an optional path. diff --git a/tests/OptionsTest.php b/tests/OptionsTest.php index 3bbc3c5ea..9ef4662ea 100644 --- a/tests/OptionsTest.php +++ b/tests/OptionsTest.php @@ -177,7 +177,7 @@ static function (): void {}, 'spotlight', true, 'isSpotlightEnabled', - 'EnableSpotlight', + 'enableSpotlight', ]; yield [ @@ -653,6 +653,37 @@ public function testReleaseOptionDefaultValueIsPreferredFromSentryEnvironmentVar $this->assertSame('0.0.4', (new Options())->getRelease()); } + /** + * @backupGlobals enabled + * + * @dataProvider spotlightEnvironmentValueDataProvider + */ + public function testSpotlightOptionDefaultValueIsControlledFromEnvironmentVariable(string $environmentVariableValue, bool $expectedSpotlightEnabled, string $expectedSpotlightUrl): void + { + $_SERVER['SENTRY_SPOTLIGHT'] = $environmentVariableValue; + + $options = new Options(); + + $this->assertEquals($expectedSpotlightEnabled, $options->isSpotlightEnabled()); + $this->assertEquals($expectedSpotlightUrl, $options->getSpotlightUrl()); + } + + public static function spotlightEnvironmentValueDataProvider(): array + { + $defaultSpotlightUrl = 'http://localhost:8969'; + + return [ + ['', false, $defaultSpotlightUrl], + ['true', true, $defaultSpotlightUrl], + ['1', true, $defaultSpotlightUrl], + ['false', false, $defaultSpotlightUrl], + ['0', false, $defaultSpotlightUrl], + ['null', false, $defaultSpotlightUrl], + ['http://localhost:1234', true, 'http://localhost:1234'], + ['some invalid looking value', false, $defaultSpotlightUrl], + ]; + } + public function testErrorTypesOptionIsNotDynamiclyReadFromErrorReportingLevelWhenSet(): void { $errorReportingBeforeTest = error_reporting(\E_ERROR); From 3b3768d5af1381dc42c94440beca57d3f4d42671 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Thu, 28 Nov 2024 12:37:56 +0100 Subject: [PATCH 2/7] Deprecate obsolete spotlight options --- src/Options.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/Options.php b/src/Options.php index 70e924672..02b67d559 100644 --- a/src/Options.php +++ b/src/Options.php @@ -371,7 +371,10 @@ public function isSpotlightEnabled(): bool return \is_string($this->options['spotlight']) || $this->options['spotlight']; } - public function enableSpotlight(bool $enable): self + /** + * @param bool|string $enable can be passed a boolean or the Spotlight URL (which will also enable Spotlight) + */ + public function enableSpotlight($enable): self { $options = array_merge($this->options, ['spotlight' => $enable]); @@ -389,6 +392,11 @@ public function getSpotlightUrl(): string return $this->options['spotlight_url']; } + /** + * @return $this + * + * @deprecated Use `enableSpotlight` instead. Will be removed in 5.x. + */ public function setSpotlightUrl(string $url): self { $options = array_merge($this->options, ['spotlight_url' => $url]); @@ -1132,6 +1140,9 @@ private function configureOptions(OptionsResolver $resolver): void 'environment' => $_SERVER['SENTRY_ENVIRONMENT'] ?? null, 'logger' => null, 'spotlight' => $_SERVER['SENTRY_SPOTLIGHT'] ?? null, + /** + * @deprecated Use `spotlight` instead. Will be removed in 5.x. + */ 'spotlight_url' => 'http://localhost:8969', 'release' => $_SERVER['SENTRY_RELEASE'] ?? $_SERVER['AWS_LAMBDA_FUNCTION_VERSION'] ?? null, 'dsn' => $_SERVER['SENTRY_DSN'] ?? null, From 20daec19871c7b3cd0a374a8caafc1cb518d1542 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Thu, 28 Nov 2024 12:38:36 +0100 Subject: [PATCH 3/7] =?UTF-8?q?Don=E2=80=99t=20use=20polyfilled=20`str=5Fs?= =?UTF-8?q?tarts=5Fwith`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Options.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Options.php b/src/Options.php index 02b67d559..ee7b73c15 100644 --- a/src/Options.php +++ b/src/Options.php @@ -1280,7 +1280,7 @@ private function normalizeBooleanOrUrl(SymfonyOptions $options, ?string $boolean return false; } - if (str_starts_with($booleanOrUrl, 'http')) { + if (filter_var($booleanOrUrl, \FILTER_VALIDATE_URL)) { return $booleanOrUrl; } From b887a01f60a783473641b068b49ca72fee8b5630 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Thu, 28 Nov 2024 12:42:08 +0100 Subject: [PATCH 4/7] CS --- src/Options.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Options.php b/src/Options.php index ee7b73c15..f75331d05 100644 --- a/src/Options.php +++ b/src/Options.php @@ -1088,7 +1088,7 @@ public function setClassSerializers(array $serializers): self /** * Gets a callback that will be invoked when we sample a Transaction. * - * @psalm-return null|callable(\Sentry\Tracing\SamplingContext): float + * @psalm-return null|callable(Tracing\SamplingContext): float */ public function getTracesSampler(): ?callable { @@ -1101,7 +1101,7 @@ public function getTracesSampler(): ?callable * * @param ?callable $sampler The sampler * - * @psalm-param null|callable(\Sentry\Tracing\SamplingContext): float $sampler + * @psalm-param null|callable(Tracing\SamplingContext): float $sampler */ public function setTracesSampler(?callable $sampler): self { From 2a5022a6986f9e9a08be3545194acd624f55bde9 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Sat, 30 Nov 2024 16:55:31 +0100 Subject: [PATCH 5/7] Update src/Options.php Co-authored-by: Michi Hoffmann --- src/Options.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Options.php b/src/Options.php index f75331d05..f8c7b5fcb 100644 --- a/src/Options.php +++ b/src/Options.php @@ -395,7 +395,7 @@ public function getSpotlightUrl(): string /** * @return $this * - * @deprecated Use `enableSpotlight` instead. Will be removed in 5.x. + * @deprecated since version 4.11. To be removed in 5.x. You may use `enableSpotlight` instead. */ public function setSpotlightUrl(string $url): self { From 1580c95aa43e81e8af2bd684d2943ffc48eb0803 Mon Sep 17 00:00:00 2001 From: Alex Bouma Date: Sat, 30 Nov 2024 16:55:37 +0100 Subject: [PATCH 6/7] Update src/Options.php Co-authored-by: Michi Hoffmann --- src/Options.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Options.php b/src/Options.php index f8c7b5fcb..6950ba494 100644 --- a/src/Options.php +++ b/src/Options.php @@ -1141,7 +1141,7 @@ private function configureOptions(OptionsResolver $resolver): void 'logger' => null, 'spotlight' => $_SERVER['SENTRY_SPOTLIGHT'] ?? null, /** - * @deprecated Use `spotlight` instead. Will be removed in 5.x. + * @deprecated since version 4.11. To be removed in 5.0. You may use `spotlight` instead. */ 'spotlight_url' => 'http://localhost:8969', 'release' => $_SERVER['SENTRY_RELEASE'] ?? $_SERVER['AWS_LAMBDA_FUNCTION_VERSION'] ?? null, From 773e26558945e6265ecdd03648b4f87df9bf319b Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Tue, 10 Dec 2024 13:48:53 +0100 Subject: [PATCH 7/7] CS --- src/Options.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Options.php b/src/Options.php index 6950ba494..eb4ffb1f1 100644 --- a/src/Options.php +++ b/src/Options.php @@ -395,7 +395,7 @@ public function getSpotlightUrl(): string /** * @return $this * - * @deprecated since version 4.11. To be removed in 5.x. You may use `enableSpotlight` instead. + * @deprecated since version 4.11. To be removed in 5.x. You may use `enableSpotlight` instead. */ public function setSpotlightUrl(string $url): self {