From 00b7575c89815aa218e7e3ed3d61b25ee65f244f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 3 Jan 2023 19:03:57 +0100 Subject: [PATCH 1/3] perf(logging): Return early when log level does not match before serializing an exception MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/private/Log.php | 6 +++++- lib/private/Support/CrashReport/Registry.php | 4 ++++ lib/public/Support/CrashReport/IRegistry.php | 9 +++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/lib/private/Log.php b/lib/private/Log.php index 4ab647bc6c155..e4be5134f11bd 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -312,6 +312,11 @@ public function logException(Throwable $exception, array $context = []) { $app = $context['app'] ?? 'no app in context'; $level = $context['level'] ?? ILogger::ERROR; + $minLevel = $this->getLogLevel($context); + if ($level < $minLevel && ($this->crashReporters === null || !$this->crashReporters->hasReporters())) { + return; + } + // if an error is raised before the autoloader is properly setup, we can't serialize exceptions try { $serializer = $this->getSerializer(); @@ -325,7 +330,6 @@ public function logException(Throwable $exception, array $context = []) { $data = array_merge($serializer->serializeException($exception), $data); $data = $this->interpolateMessage($data, $context['message'] ?? '--', 'CustomMessage'); - $minLevel = $this->getLogLevel($context); array_walk($context, [$this->normalizer, 'format']); diff --git a/lib/private/Support/CrashReport/Registry.php b/lib/private/Support/CrashReport/Registry.php index 472f39c288438..f5457f60ad485 100644 --- a/lib/private/Support/CrashReport/Registry.php +++ b/lib/private/Support/CrashReport/Registry.php @@ -147,4 +147,8 @@ private function loadLazyProviders(): void { } } } + + public function hasReporters(): bool { + return !empty($this->lazyReporters) || !empty($this->reporters); + } } diff --git a/lib/public/Support/CrashReport/IRegistry.php b/lib/public/Support/CrashReport/IRegistry.php index 6ee2b57f6135f..35cf78920da70 100644 --- a/lib/public/Support/CrashReport/IRegistry.php +++ b/lib/public/Support/CrashReport/IRegistry.php @@ -81,4 +81,13 @@ public function delegateReport($exception, array $context = []); * @since 17.0.0 */ public function delegateMessage(string $message, array $context = []): void; + + /** + * Check if any reporter has been registered to delegate to + * + * @return bool + * @deprecated use internally only + * @since 26.0.0 + */ + public function hasReporters(): bool; } From 7daa20d3095cbf99abebfe23fb307fff9d1e5fbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 3 Jan 2023 19:24:00 +0100 Subject: [PATCH 2/3] fix(ExceptionSerializer): encode arguments before filtering the trace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This will avoid running into a Nesting level too deep error as the encodeArg calls will limit potential recursive calls on the arguments to a nesting level of 5 Signed-off-by: Julius Härtl --- lib/private/Log/ExceptionSerializer.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/private/Log/ExceptionSerializer.php b/lib/private/Log/ExceptionSerializer.php index aaf6a39235e0e..5f806be0ae5bb 100644 --- a/lib/private/Log/ExceptionSerializer.php +++ b/lib/private/Log/ExceptionSerializer.php @@ -223,13 +223,13 @@ private function removeValuesFromArgs($args, $values) { } private function encodeTrace($trace) { - $filteredTrace = $this->filterTrace($trace); - return array_map(function (array $line) { + $trace = array_map(function (array $line) { if (isset($line['args'])) { $line['args'] = array_map([$this, 'encodeArg'], $line['args']); } return $line; - }, $filteredTrace); + }, $trace); + return $this->filterTrace($trace); } private function encodeArg($arg, $nestingLevel = 5) { From cf1bd0eb70e81915ba018afc22d3a2374b179cbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Thu, 12 Jan 2023 15:19:28 +0100 Subject: [PATCH 3/3] chore: Add typings to Log properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- lib/private/Log.php | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/lib/private/Log.php b/lib/private/Log.php index e4be5134f11bd..2ee5bfc9c5a41 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -59,21 +59,11 @@ * MonoLog is an example implementing this interface. */ class Log implements ILogger, IDataLogger { - - /** @var IWriter */ - private $logger; - - /** @var SystemConfig */ - private $config; - - /** @var boolean|null cache the result of the log condition check for the request */ - private $logConditionSatisfied = null; - - /** @var Normalizer */ - private $normalizer; - - /** @var IRegistry */ - private $crashReporters; + private IWriter $logger; + private ?SystemConfig $config; + private ?bool $logConditionSatisfied = null; + private ?Normalizer $normalizer; + private ?IRegistry $crashReporters; /** * @param IWriter $logger The logger that should be used @@ -81,7 +71,7 @@ class Log implements ILogger, IDataLogger { * @param Normalizer|null $normalizer * @param IRegistry|null $registry */ - public function __construct(IWriter $logger, SystemConfig $config = null, $normalizer = null, IRegistry $registry = null) { + public function __construct(IWriter $logger, SystemConfig $config = null, Normalizer $normalizer = null, IRegistry $registry = null) { // FIXME: Add this for backwards compatibility, should be fixed at some point probably if ($config === null) { $config = \OC::$server->getSystemConfig();