From 9a299ed60beca9318c30782a470ae97293b569e1 Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Mon, 12 Feb 2024 13:43:50 -0100 Subject: [PATCH 1/2] exit log() early if no crashreporter registered Signed-off-by: Maxence Lange --- lib/private/Log.php | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/lib/private/Log.php b/lib/private/Log.php index 9975696ff0679..83ff1003294bb 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -62,24 +62,22 @@ * MonoLog is an example implementing this interface. */ class Log implements ILogger, IDataLogger { - private IWriter $logger; private ?SystemConfig $config; private ?bool $logConditionSatisfied = null; private ?Normalizer $normalizer; - private ?IRegistry $crashReporters; private ?IEventDispatcher $eventDispatcher; /** * @param IWriter $logger The logger that should be used - * @param SystemConfig $config the system config object + * @param SystemConfig|null $config the system config object * @param Normalizer|null $normalizer - * @param IRegistry|null $registry + * @param IRegistry|null $crashReporters */ public function __construct( - IWriter $logger, + private IWriter $logger, SystemConfig $config = null, Normalizer $normalizer = null, - IRegistry $registry = null + private ?IRegistry $crashReporters = null ) { // FIXME: Add this for backwards compatibility, should be fixed at some point probably if ($config === null) { @@ -87,13 +85,11 @@ public function __construct( } $this->config = $config; - $this->logger = $logger; if ($normalizer === null) { $this->normalizer = new Normalizer(); } else { $this->normalizer = $normalizer; } - $this->crashReporters = $registry; $this->eventDispatcher = null; } @@ -211,6 +207,9 @@ public function debug(string $message, array $context = []) { */ public function log(int $level, string $message, array $context = []) { $minLevel = $this->getLogLevel($context); + if ($level < $minLevel && (($this->crashReporters?->hasReporters() ?? false) === false)) { + return; // we already know that log will be fully ignored + } array_walk($context, [$this->normalizer, 'format']); @@ -241,9 +240,7 @@ public function log(int $level, string $message, array $context = []) { $this->crashReporters->delegateMessage($entry['message'], $messageContext); } } else { - if ($this->crashReporters !== null) { - $this->crashReporters->delegateBreadcrumb($entry['message'], 'log', $context); - } + $this->crashReporters?->delegateBreadcrumb($entry['message'], 'log', $context); } } catch (Throwable $e) { // make sure we dont hard crash if logging fails @@ -329,8 +326,8 @@ public function logException(Throwable $exception, array $context = []) { $level = $context['level'] ?? ILogger::ERROR; $minLevel = $this->getLogLevel($context); - if ($level < $minLevel && ($this->crashReporters === null || !$this->crashReporters->hasReporters())) { - return; + if ($level < $minLevel && (($this->crashReporters?->hasReporters() ?? false) === false)) { + return; // we already know that log will be fully ignored } // if an error is raised before the autoloader is properly setup, we can't serialize exceptions From 2232753b994dfb65ae46f07f827b89858faea4db Mon Sep 17 00:00:00 2001 From: Maxence Lange Date: Wed, 14 Feb 2024 14:02:20 -0100 Subject: [PATCH 2/2] add hasListeners() Signed-off-by: Maxence Lange --- .../EventDispatcher/EventDispatcher.php | 26 +++++++------------ lib/private/Log.php | 21 +++++++-------- .../EventDispatcher/IEventDispatcher.php | 9 +++++++ 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/lib/private/EventDispatcher/EventDispatcher.php b/lib/private/EventDispatcher/EventDispatcher.php index 14c13d516c0e2..39bf2a6afa974 100644 --- a/lib/private/EventDispatcher/EventDispatcher.php +++ b/lib/private/EventDispatcher/EventDispatcher.php @@ -33,29 +33,17 @@ use OCP\EventDispatcher\ABroadcastedEvent; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; -use OCP\IContainer; use OCP\IServerContainer; use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\EventDispatcher as SymfonyDispatcher; use function get_class; class EventDispatcher implements IEventDispatcher { - /** @var SymfonyDispatcher */ - private $dispatcher; - - /** @var IContainer */ - private $container; - - /** @var LoggerInterface */ - private $logger; - - public function __construct(SymfonyDispatcher $dispatcher, - IServerContainer $container, - LoggerInterface $logger) { - $this->dispatcher = $dispatcher; - $this->container = $container; - $this->logger = $logger; - + public function __construct( + private SymfonyDispatcher $dispatcher, + private IServerContainer $container, + private LoggerInterface $logger, + ) { // inject the event dispatcher into the logger // this is done here because there is a cyclic dependency between the event dispatcher and logger if ($this->logger instanceof Log || $this->logger instanceof Log\PsrLoggerAdapter) { @@ -86,6 +74,10 @@ public function addServiceListener(string $eventName, $this->addListener($eventName, $listener, $priority); } + public function hasListeners(string $eventName): bool { + return $this->dispatcher->hasListeners($eventName); + } + /** * @deprecated */ diff --git a/lib/private/Log.php b/lib/private/Log.php index 83ff1003294bb..2ad214ddec5e1 100644 --- a/lib/private/Log.php +++ b/lib/private/Log.php @@ -207,8 +207,10 @@ public function debug(string $message, array $context = []) { */ public function log(int $level, string $message, array $context = []) { $minLevel = $this->getLogLevel($context); - if ($level < $minLevel && (($this->crashReporters?->hasReporters() ?? false) === false)) { - return; // we already know that log will be fully ignored + if ($level < $minLevel + && (($this->crashReporters?->hasReporters() ?? false) === false) + && (($this->eventDispatcher?->hasListeners(BeforeMessageLoggedEvent::class) ?? false) === false)) { + return; // no crash reporter, no listeners, we can stop for lower log level } array_walk($context, [$this->normalizer, 'format']); @@ -216,9 +218,7 @@ public function log(int $level, string $message, array $context = []) { $app = $context['app'] ?? 'no app in context'; $entry = $this->interpolateMessage($context, $message); - if ($this->eventDispatcher) { - $this->eventDispatcher->dispatchTyped(new BeforeMessageLoggedEvent($app, $level, $entry)); - } + $this->eventDispatcher?->dispatchTyped(new BeforeMessageLoggedEvent($app, $level, $entry)); $hasBacktrace = isset($entry['exception']); $logBacktrace = $this->config->getValue('log.backtrace', false); @@ -326,8 +326,10 @@ public function logException(Throwable $exception, array $context = []) { $level = $context['level'] ?? ILogger::ERROR; $minLevel = $this->getLogLevel($context); - if ($level < $minLevel && (($this->crashReporters?->hasReporters() ?? false) === false)) { - return; // we already know that log will be fully ignored + if ($level < $minLevel + && (($this->crashReporters?->hasReporters() ?? false) === false) + && (($this->eventDispatcher?->hasListeners(BeforeMessageLoggedEvent::class) ?? false) === false)) { + return; // no crash reporter, no listeners, we can stop for lower log level } // if an error is raised before the autoloader is properly setup, we can't serialize exceptions @@ -343,12 +345,9 @@ public function logException(Throwable $exception, array $context = []) { $data = array_merge($serializer->serializeException($exception), $data); $data = $this->interpolateMessage($data, isset($context['message']) && $context['message'] !== '' ? $context['message'] : ('Exception thrown: ' . get_class($exception)), 'CustomMessage'); - array_walk($context, [$this->normalizer, 'format']); - if ($this->eventDispatcher) { - $this->eventDispatcher->dispatchTyped(new BeforeMessageLoggedEvent($app, $level, $data)); - } + $this->eventDispatcher?->dispatchTyped(new BeforeMessageLoggedEvent($app, $level, $data)); try { if ($level >= $minLevel) { diff --git a/lib/public/EventDispatcher/IEventDispatcher.php b/lib/public/EventDispatcher/IEventDispatcher.php index 0a96fa799d402..a84e0fe2f3b75 100644 --- a/lib/public/EventDispatcher/IEventDispatcher.php +++ b/lib/public/EventDispatcher/IEventDispatcher.php @@ -69,6 +69,15 @@ public function removeListener(string $eventName, callable $listener): void; */ public function addServiceListener(string $eventName, string $className, int $priority = 0): void; + /** + * @template T of \OCP\EventDispatcher\Event + * @param string $eventName preferably the fully-qualified class name of the Event sub class + * + * @return bool TRUE if event has registered listeners + * @since 29.0.0 + */ + public function hasListeners(string $eventName): bool; + /** * @template T of \OCP\EventDispatcher\Event * @param string $eventName