Skip to content

enh(log): exit log() earlier if no crashreport registered #43529

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
skjnldsv merged 2 commits into from
Feb 23, 2024
Merged

enh(log): exit log() earlier if no crashreport registered #43529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9a299ed
exit log() early if no crashreporter registered
ArtificialOwl Feb 12, 2024
2232753
add hasListeners()
ArtificialOwl Feb 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 9 additions & 17 deletions lib/private/EventDispatcher/EventDispatcher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
*/
Expand Down
36 changes: 16 additions & 20 deletions lib/private/Log.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,38 +62,34 @@
* 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) {
$config = \OC::$server->getSystemConfig();
}

$this->config = $config;
$this->logger = $logger;
if ($normalizer === null) {
$this->normalizer = new Normalizer();
} else {
$this->normalizer = $normalizer;
}
$this->crashReporters = $registry;
$this->eventDispatcher = null;
}

Expand Down Expand Up @@ -211,15 +207,18 @@ 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)
&& (($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']);

$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);
Expand All @@ -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
Expand Down Expand Up @@ -329,8 +326,10 @@ 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)
&& (($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
Expand All @@ -346,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) {
Expand Down
9 changes: 9 additions & 0 deletions lib/public/EventDispatcher/IEventDispatcher.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down