From 0c4192891a25b55509352cdea783f87c5297b9ef Mon Sep 17 00:00:00 2001 From: Michael Hoffmann Date: Mon, 6 Nov 2023 05:46:56 +0100 Subject: [PATCH] Refactor `logger` option --- phpstan-baseline.neon | 12 +++- src/Client.php | 11 ++-- src/ClientBuilder.php | 7 +++ src/Logger/DebugFileLogger.php | 29 +++++++++ src/Logger/DebugStdOutLogger.php | 19 ++++++ src/Options.php | 33 +++------- src/Transport/Result.php | 4 +- tests/ClientTest.php | 4 -- tests/OptionsTest.php | 100 ++----------------------------- 9 files changed, 84 insertions(+), 135 deletions(-) create mode 100644 src/Logger/DebugFileLogger.php create mode 100644 src/Logger/DebugStdOutLogger.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index cf0df747f..7083cd320 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -30,6 +30,16 @@ parameters: count: 1 path: src/Integration/RequestIntegration.php + - + message: "#^Parameter \\#2 \\.\\.\\.\\$values of function sprintf expects bool\\|float\\|int\\|string\\|null, mixed given\\.$#" + count: 1 + path: src/Logger/DebugFileLogger.php + + - + message: "#^Parameter \\#2 \\.\\.\\.\\$values of function sprintf expects bool\\|float\\|int\\|string\\|null, mixed given\\.$#" + count: 1 + path: src/Logger/DebugStdOutLogger.php + - message: "#^Parameter \\#1 \\$level of method Monolog\\\\Handler\\\\AbstractHandler\\:\\:__construct\\(\\) expects 100\\|200\\|250\\|300\\|400\\|500\\|550\\|600\\|'ALERT'\\|'alert'\\|'CRITICAL'\\|'critical'\\|'DEBUG'\\|'debug'\\|'EMERGENCY'\\|'emergency'\\|'ERROR'\\|'error'\\|'INFO'\\|'info'\\|'NOTICE'\\|'notice'\\|'WARNING'\\|'warning'\\|Monolog\\\\Level, int\\|Monolog\\\\Level\\|string given\\.$#" count: 1 @@ -136,7 +146,7 @@ parameters: path: src/Options.php - - message: "#^Method Sentry\\\\Options\\:\\:getLogger\\(\\) should return string but returns mixed\\.$#" + message: "#^Method Sentry\\\\Options\\:\\:getLogger\\(\\) should return Psr\\\\Log\\\\LoggerInterface\\|null but returns mixed\\.$#" count: 1 path: src/Options.php diff --git a/src/Client.php b/src/Client.php index f93e6210f..5cc99cd3e 100644 --- a/src/Client.php +++ b/src/Client.php @@ -93,11 +93,12 @@ public function __construct( ) { $this->options = $options; $this->transport = $transport; - $this->logger = $logger ?? new NullLogger(); - $this->integrations = IntegrationRegistry::getInstance()->setupIntegrations($options, $this->logger); - $this->stacktraceBuilder = new StacktraceBuilder($options, $representationSerializer ?? new RepresentationSerializer($this->options)); $this->sdkIdentifier = $sdkIdentifier ?? self::SDK_IDENTIFIER; $this->sdkVersion = $sdkVersion ?? self::SDK_VERSION; + $this->stacktraceBuilder = new StacktraceBuilder($options, $representationSerializer ?? new RepresentationSerializer($this->options)); + $this->logger = $logger ?? new NullLogger(); + + $this->integrations = IntegrationRegistry::getInstance()->setupIntegrations($options, $this->logger); } /** @@ -269,10 +270,6 @@ private function prepareEvent(Event $event, ?EventHint $hint = null, ?Scope $sco $event->setEnvironment($this->options->getEnvironment() ?? Event::DEFAULT_ENVIRONMENT); } - if ($event->getLogger() === null) { - $event->setLogger($this->options->getLogger(false)); - } - $isTransaction = EventType::transaction() === $event->getType(); $sampleRate = $this->options->getSampleRate(); diff --git a/src/ClientBuilder.php b/src/ClientBuilder.php index 0594ba08e..54a433eb0 100644 --- a/src/ClientBuilder.php +++ b/src/ClientBuilder.php @@ -63,6 +63,8 @@ public function __construct(?Options $options = null) { $this->options = $options ?? new Options(); + $this->logger = $this->options->getLogger() ?? null; + $this->httpClient = $this->options->getHttpClient() ?? new HttpClient($this->sdkIdentifier, $this->sdkVersion); $this->transport = $this->options->getTransport() ?? new HttpTransport( $this->options, @@ -92,6 +94,11 @@ public function setRepresentationSerializer(RepresentationSerializerInterface $r return $this; } + public function getLogger(): ?LoggerInterface + { + return $this->logger; + } + public function setLogger(LoggerInterface $logger): self { $this->logger = $logger; diff --git a/src/Logger/DebugFileLogger.php b/src/Logger/DebugFileLogger.php new file mode 100644 index 000000000..da406c44f --- /dev/null +++ b/src/Logger/DebugFileLogger.php @@ -0,0 +1,29 @@ +filePath = $filePath; + } + + /** + * @param mixed $level + * @param mixed[] $context + */ + public function log($level, \Stringable|string $message, array $context = []): void + { + file_put_contents($this->filePath, sprintf("sentry/sentry: [%s] %s\n", $level, (string) $message), \FILE_APPEND); + } +} diff --git a/src/Logger/DebugStdOutLogger.php b/src/Logger/DebugStdOutLogger.php new file mode 100644 index 000000000..fc1212931 --- /dev/null +++ b/src/Logger/DebugStdOutLogger.php @@ -0,0 +1,19 @@ +options['logger']; } /** - * Sets the logger used by Sentry. - * - * @param string $logger The logger - * - * @deprecated since version 3.2, to be removed in 4.0 + * Sets a PSR-3 compatible logger to log internal debug messages. */ - public function setLogger(string $logger): self + public function setLogger(LoggerInterface $logger): self { - @trigger_error(sprintf('Method %s() is deprecated since version 3.2 and will be removed in 4.0.', __METHOD__), \E_USER_DEPRECATED); - $options = array_merge($this->options, ['logger' => $logger]); $this->options = $this->resolver->resolve($options); @@ -994,7 +983,7 @@ private function configureOptions(OptionsResolver $resolver): void 'attach_stacktrace' => false, 'context_lines' => 5, 'environment' => $_SERVER['SENTRY_ENVIRONMENT'] ?? null, - 'logger' => 'php', + 'logger' => null, 'release' => $_SERVER['SENTRY_RELEASE'] ?? null, 'dsn' => $_SERVER['SENTRY_DSN'] ?? null, 'server_name' => gethostname(), @@ -1041,7 +1030,7 @@ private function configureOptions(OptionsResolver $resolver): void $resolver->setAllowedTypes('environment', ['null', 'string']); $resolver->setAllowedTypes('in_app_exclude', 'string[]'); $resolver->setAllowedTypes('in_app_include', 'string[]'); - $resolver->setAllowedTypes('logger', ['null', 'string']); + $resolver->setAllowedTypes('logger', ['null', LoggerInterface::class]); $resolver->setAllowedTypes('release', ['null', 'string']); $resolver->setAllowedTypes('dsn', ['null', 'string', 'bool', Dsn::class]); $resolver->setAllowedTypes('server_name', 'string'); @@ -1089,14 +1078,6 @@ private function configureOptions(OptionsResolver $resolver): void $resolver->setNormalizer('in_app_include', function (SymfonyOptions $options, array $value) { return array_map([$this, 'normalizeAbsolutePath'], $value); }); - - $resolver->setNormalizer('logger', function (SymfonyOptions $options, ?string $value): ?string { - if ($value !== 'php') { - @trigger_error('The option "logger" is deprecated.', \E_USER_DEPRECATED); - } - - return $value; - }); } /** diff --git a/src/Transport/Result.php b/src/Transport/Result.php index 881624f11..a4a6b8238 100644 --- a/src/Transport/Result.php +++ b/src/Transport/Result.php @@ -9,8 +9,10 @@ /** * This class contains the details of the sending operation of an event, e.g. * if it was sent successfully or if it was skipped because of some reason. + * + * @internal */ -final class Result +class Result { /** * @var ResultStatus The status of the sending operation of the event diff --git a/tests/ClientTest.php b/tests/ClientTest.php index 3b5011ddf..30d2842a5 100644 --- a/tests/ClientTest.php +++ b/tests/ClientTest.php @@ -217,7 +217,6 @@ public static function captureEventDataProvider(): \Generator { $event = Event::createEvent(); $expectedEvent = clone $event; - $expectedEvent->setLogger('php'); $expectedEvent->setServerName('example.com'); $expectedEvent->setRelease('0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33'); $expectedEvent->setEnvironment('development'); @@ -241,7 +240,6 @@ public static function captureEventDataProvider(): \Generator $event->setTags(['context' => 'production']); $expectedEvent = clone $event; - $expectedEvent->setLogger('php'); $expectedEvent->setTags(['context' => 'production', 'ios_version' => '14.0']); yield 'Options set && event properties set => event properties override options' => [ @@ -259,7 +257,6 @@ public static function captureEventDataProvider(): \Generator $event->setServerName('example.com'); $expectedEvent = clone $event; - $expectedEvent->setLogger('php'); $expectedEvent->setEnvironment('production'); yield 'Environment option set to null && no event property set => fallback to default value' => [ @@ -274,7 +271,6 @@ public static function captureEventDataProvider(): \Generator $event->setExceptions([new ExceptionDataBag(new \ErrorException())]); $expectedEvent = clone $event; - $expectedEvent->setLogger('php'); $expectedEvent->setEnvironment('production'); yield 'Error level is set && exception is instance of ErrorException => preserve the error level set by the user' => [ diff --git a/tests/OptionsTest.php b/tests/OptionsTest.php index 1b1e7c145..09bec7b9b 100644 --- a/tests/OptionsTest.php +++ b/tests/OptionsTest.php @@ -5,6 +5,7 @@ namespace Sentry\Tests; use PHPUnit\Framework\TestCase; +use Psr\Log\NullLogger; use Sentry\Dsn; use Sentry\HttpClient\HttpClient; use Sentry\Options; @@ -45,13 +46,8 @@ public function testConstructor( string $option, $value, string $getterMethod, - ?string $setterMethod, - ?string $expectedGetterDeprecationMessage + ?string $setterMethod ): void { - if ($expectedGetterDeprecationMessage !== null) { - $this->expectDeprecation($expectedGetterDeprecationMessage); - } - $options = new Options([$option => $value]); $this->assertEquals($value, $options->$getterMethod()); @@ -66,18 +62,8 @@ public function testGettersAndSetters( string $option, $value, string $getterMethod, - ?string $setterMethod, - ?string $expectedGetterDeprecationMessage, - ?string $expectedSetterDeprecationMessage + ?string $setterMethod ): void { - if ($expectedSetterDeprecationMessage !== null) { - $this->expectDeprecation($expectedSetterDeprecationMessage); - } - - if ($expectedGetterDeprecationMessage !== null) { - $this->expectDeprecation($expectedGetterDeprecationMessage); - } - $options = new Options(); if ($setterMethod !== null) { @@ -94,8 +80,6 @@ public static function optionsDataProvider(): \Generator ['foo', 'bar'], 'getPrefixes', 'setPrefixes', - null, - null, ]; yield [ @@ -103,8 +87,6 @@ public static function optionsDataProvider(): \Generator 0.5, 'getSampleRate', 'setSampleRate', - null, - null, ]; yield [ @@ -112,8 +94,6 @@ public static function optionsDataProvider(): \Generator 0.5, 'getTracesSampleRate', 'setTracesSampleRate', - null, - null, ]; yield [ @@ -121,8 +101,6 @@ public static function optionsDataProvider(): \Generator null, 'getTracesSampleRate', 'setTracesSampleRate', - null, - null, ]; yield [ @@ -130,8 +108,6 @@ public static function optionsDataProvider(): \Generator static function (): void {}, 'getTracesSampler', 'setTracesSampler', - null, - null, ]; yield [ @@ -139,8 +115,6 @@ static function (): void {}, true, 'getEnableTracing', 'setEnableTracing', - null, - null, ]; yield [ @@ -148,8 +122,6 @@ static function (): void {}, 0.5, 'getProfilesSampleRate', 'setProfilesSampleRate', - null, - null, ]; yield [ @@ -157,8 +129,6 @@ static function (): void {}, false, 'shouldAttachStacktrace', 'setAttachStacktrace', - null, - null, ]; yield [ @@ -166,8 +136,6 @@ static function (): void {}, 3, 'getContextLines', 'setContextLines', - null, - null, ]; yield [ @@ -175,8 +143,6 @@ static function (): void {}, 'foo', 'getEnvironment', 'setEnvironment', - null, - null, ]; yield [ @@ -184,8 +150,6 @@ static function (): void {}, ['foo', 'bar'], 'getInAppExcludedPaths', 'setInAppExcludedPaths', - null, - null, ]; yield [ @@ -193,17 +157,13 @@ static function (): void {}, ['foo', 'bar'], 'getInAppIncludedPaths', 'setInAppIncludedPaths', - null, - null, ]; yield [ 'logger', - 'foo', + new NullLogger(), 'getLogger', 'setLogger', - 'Method Sentry\\Options::getLogger() is deprecated since version 3.2 and will be removed in 4.0.', - 'Method Sentry\\Options::setLogger() is deprecated since version 3.2 and will be removed in 4.0.', ]; yield [ @@ -211,8 +171,6 @@ static function (): void {}, 'dev', 'getRelease', 'setRelease', - null, - null, ]; yield [ @@ -220,8 +178,6 @@ static function (): void {}, 'foo', 'getServerName', 'setServerName', - null, - null, ]; yield [ @@ -229,8 +185,6 @@ static function (): void {}, ['foo', 'bar'], 'getTags', 'setTags', - null, - null, ]; yield [ @@ -238,8 +192,6 @@ static function (): void {}, 0, 'getErrorTypes', 'setErrorTypes', - null, - null, ]; yield [ @@ -247,8 +199,6 @@ static function (): void {}, 50, 'getMaxBreadcrumbs', 'setMaxBreadcrumbs', - null, - null, ]; yield [ @@ -256,8 +206,6 @@ static function (): void {}, ['foo', 'bar'], 'getIgnoreExceptions', 'setIgnoreExceptions', - null, - null, ]; yield [ @@ -265,8 +213,6 @@ static function (): void {}, ['foo', 'bar'], 'getIgnoreTransactions', 'setIgnoreTransactions', - null, - null, ]; yield [ @@ -274,8 +220,6 @@ static function (): void {}, static function (): void {}, 'getBeforeSendCallback', 'setBeforeSendCallback', - null, - null, ]; yield [ @@ -283,8 +227,6 @@ static function (): void {}, static function (): void {}, 'getBeforeSendTransactionCallback', 'setBeforeSendTransactionCallback', - null, - null, ]; yield [ @@ -292,8 +234,6 @@ static function (): void {}, ['www.example.com'], 'getTracePropagationTargets', 'setTracePropagationTargets', - null, - null, ]; yield [ @@ -301,8 +241,6 @@ static function (): void {}, static function (): void {}, 'getBeforeBreadcrumbCallback', 'setBeforeBreadcrumbCallback', - null, - null, ]; yield [ @@ -310,8 +248,6 @@ static function (): void {}, true, 'shouldSendDefaultPii', 'setSendDefaultPii', - null, - null, ]; yield [ @@ -319,8 +255,6 @@ static function (): void {}, false, 'hasDefaultIntegrations', 'setDefaultIntegrations', - null, - null, ]; yield [ @@ -328,8 +262,6 @@ static function (): void {}, 50, 'getMaxValueLength', 'setMaxValueLength', - null, - null, ]; yield [ @@ -337,8 +269,6 @@ static function (): void {}, new HttpTransport(new Options(), new HttpClient('foo', 'bar'), new PayloadSerializer(new Options())), 'getTransport', 'setTransport', - null, - null, ]; yield [ @@ -346,8 +276,6 @@ static function (): void {}, new HttpClient('foo', 'bar'), 'getHttpClient', 'setHttpClient', - null, - null, ]; yield [ @@ -355,8 +283,6 @@ static function (): void {}, '127.0.0.1', 'getHttpProxy', 'setHttpProxy', - null, - null, ]; yield [ @@ -364,8 +290,6 @@ static function (): void {}, 'username:password', 'getHttpProxyAuthentication', 'setHttpProxyAuthentication', - null, - null, ]; yield [ @@ -373,8 +297,6 @@ static function (): void {}, 1, 'getHttpTimeout', 'setHttpTimeout', - null, - null, ]; yield [ @@ -382,8 +304,6 @@ static function (): void {}, 1.2, 'getHttpTimeout', 'setHttpTimeout', - null, - null, ]; yield [ @@ -391,8 +311,6 @@ static function (): void {}, 1, 'getHttpConnectTimeout', 'setHttpConnectTimeout', - null, - null, ]; yield [ @@ -400,8 +318,6 @@ static function (): void {}, 1.2, 'getHttpConnectTimeout', 'setHttpConnectTimeout', - null, - null, ]; yield [ @@ -409,8 +325,6 @@ static function (): void {}, false, 'getHttpSslVerifyPeer', 'setHttpSslVerifyPeer', - null, - null, ]; yield [ @@ -418,8 +332,6 @@ static function (): void {}, false, 'isHttpCompressionEnabled', 'setEnableHttpCompression', - null, - null, ]; yield [ @@ -427,8 +339,6 @@ static function (): void {}, true, 'shouldCaptureSilencedErrors', 'setCaptureSilencedErrors', - null, - null, ]; yield [ @@ -436,8 +346,6 @@ static function (): void {}, 'small', 'getMaxRequestBodySize', 'setMaxRequestBodySize', - null, - null, ]; }