Skip to content
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.

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

[stable21] Diagnostics event logging to log #31404

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions apps/dav/lib/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,11 @@ function () {
}

public function exec() {
/** @var IEventLogger $eventLogger */
$eventLogger = \OC::$server->get(IEventLogger::class);
$eventLogger->start('dav_server_exec', '');
$this->server->exec();
$eventLogger->end('dav_server_exec');
}

private function requestIsForSubtree(array $subTrees): bool {
Expand Down
6 changes: 6 additions & 0 deletions lib/base.php
Original file line number Diff line number Diff line change
Expand Up @@ -586,8 +586,13 @@ public static function init() {
// setup the basic server
self::$server = new \OC\Server(\OC::$WEBROOT, self::$config);
self::$server->boot();

$eventLogger = \OC::$server->getEventLogger();
$eventLogger->log('autoloader', 'Autoloader', $loaderStart, $loaderEnd);
$eventLogger->start('request', 'Full request after autoloading');
register_shutdown_function(function () use ($eventLogger) {
$eventLogger->end('request');
});
$eventLogger->start('boot', 'Initialize');

// Override php.ini and log everything if we're troubleshooting
Expand Down Expand Up @@ -783,6 +788,7 @@ public static function init() {
}
}
$eventLogger->end('boot');
$eventLogger->log('init', 'OC::init', $loaderStart, microtime(true));
}

/**
Expand Down
4 changes: 4 additions & 0 deletions lib/private/AppFramework/Bootstrap/Coordinator.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public function __construct(IServerContainer $container,
$this->registry = $registry;
$this->dashboardManager = $dashboardManager;
$this->eventDispatcher = $eventListener;
$this->eventLogger = $eventLogger;
$this->logger = $logger;
}

Expand Down Expand Up @@ -123,6 +124,7 @@ private function registerApps(array $appIds): void {
continue;
}
$application->register($this->registrationContext->for($appId));
$this->eventLogger->end('bootstrap:register_app_' . $appId);
}
} catch (Throwable $e) {
$this->logger->logException($e, [
Expand Down Expand Up @@ -169,6 +171,7 @@ public function bootApp(string $appId): void {
* the instance was already created for register, but any other
* (legacy) code will now do their magic via the constructor.
*/
$this->eventLogger->start('bootstrap:boot_app_' . $appId, '');
try {
/** @var App $application */
$application = $this->serverContainer->query($applicationClassName);
Expand All @@ -187,6 +190,7 @@ public function bootApp(string $appId): void {
'level' => ILogger::FATAL,
]);
}
$this->eventLogger->end('bootstrap:boot_app_' . $appId);
}

public function isBootable(string $appId) {
Expand Down
4 changes: 3 additions & 1 deletion lib/private/AppFramework/DependencyInjection/DIContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
use OC\AppFramework\ScopedPsrLogger;
use OC\AppFramework\Utility\SimpleContainer;
use OC\Core\Middleware\TwoFactorMiddleware;
use OC\Diagnostics\EventLogger;
use OC\Log\PsrLoggerAdapter;
use OC\ServerContainer;
use OCA\WorkflowEngine\Manager;
Expand Down Expand Up @@ -184,7 +185,8 @@ public function __construct($appName, $urlParams = [], ServerContainer $server =
$c->get(IRequest::class),
$c->get(IConfig::class),
$c->get(IDBConnection::class),
$c->get(LoggerInterface::class)
$c->get(LoggerInterface::class),
$c->get(EventLogger::class)
);
});

Expand Down
11 changes: 10 additions & 1 deletion lib/private/AppFramework/Http/Dispatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http\DataResponse;
use OCP\AppFramework\Http\Response;
use OCP\Diagnostics\IEventLogger;
use OCP\IConfig;
use OCP\IRequest;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -70,6 +71,9 @@ class Dispatcher {
/** @var LoggerInterface */
private $logger;

/** @var IEventLogger */
private $eventLogger;

/**
* @param Http $protocol the http protocol with contains all status headers
* @param MiddlewareDispatcher $middlewareDispatcher the dispatcher which
Expand All @@ -80,21 +84,24 @@ class Dispatcher {
* @param IConfig $config
* @param ConnectionAdapter $connection
* @param LoggerInterface $logger
* @param IEventLogger $eventLogger
*/
public function __construct(Http $protocol,
MiddlewareDispatcher $middlewareDispatcher,
ControllerMethodReflector $reflector,
IRequest $request,
IConfig $config,
ConnectionAdapter $connection,
LoggerInterface $logger) {
LoggerInterface $logger,
IEventLogger $eventLogger) {
$this->protocol = $protocol;
$this->middlewareDispatcher = $middlewareDispatcher;
$this->reflector = $reflector;
$this->request = $request;
$this->config = $config;
$this->connection = $connection;
$this->logger = $logger;
$this->eventLogger = $eventLogger;
}


Expand Down Expand Up @@ -215,7 +222,9 @@ private function executeController(Controller $controller, string $methodName):
$arguments[] = $value;
}

$this->eventLogger->start('controller:' . get_class($controller) . '::' . $methodName, 'App framework controller execution');
$response = \call_user_func_array([$controller, $methodName], $arguments);
$this->eventLogger->end('controller:' . get_class($controller) . '::' . $methodName);

// format response
if ($response instanceof DataResponse || !($response instanceof Response)) {
Expand Down
12 changes: 11 additions & 1 deletion lib/private/DB/Connection.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,17 @@ class Connection extends ReconnectWrapper {
*/
public function connect() {
try {
return parent::connect();
if ($this->_conn) {
return parent::connect();
}

// Only trigger the event logger for the initial connect call
$eventLogger = \OC::$server->getEventLogger();
$eventLogger->start('connect:db', 'db connection opened');
$status = parent::connect();
$eventLogger->end('connect:db');

return $status;
} catch (Exception $e) {
// throw a new exception to prevent leaking info from the stacktrace
throw new Exception('Failed to connect to the database: ' . $e->getMessage(), $e->getCode());
Expand Down
4 changes: 4 additions & 0 deletions lib/private/Diagnostics/Event.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,8 @@ public function getDuration() {
}
return $this->end - $this->start;
}

public function __toString() {
return $this->getId() . ' ' . $this->getDescription() . ' ' . $this->getDuration();
}
}
65 changes: 59 additions & 6 deletions lib/private/Diagnostics/EventLogger.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,25 +25,58 @@

namespace OC\Diagnostics;

use OC\Log;
use OC\SystemConfig;
use OCP\Diagnostics\IEvent;
use OCP\Diagnostics\IEventLogger;
use Psr\Log\LoggerInterface;

class EventLogger implements IEventLogger {
/**
* @var \OC\Diagnostics\Event[]
*/

/** @var Event[] */
private $events = [];


/** @var SystemConfig */
private $config;

/** @var LoggerInterface */
private $logger;

/** @var Log */
private $internalLogger;

/**
* @var bool - Module needs to be activated by some app
*/
private $activated = false;

public function __construct(SystemConfig $config, LoggerInterface $logger, Log $internalLogger) {
$this->config = $config;
$this->logger = $logger;
$this->internalLogger = $internalLogger;

if ($this->isLoggingActivated()) {
$this->activate();
}
}

public function isLoggingActivated(): bool {
if ($this->config->getValue('debug', false)) {
return true;
}

$isDebugLevel = $this->internalLogger->getLogLevel([]) === Log::DEBUG;
$systemValue = (bool)$this->config->getValue('diagnostics.logging', false);
return $systemValue && $isDebugLevel;
}

/**
* @inheritdoc
*/
public function start($id, $description) {
public function start($id, $description = '') {
if ($this->activated) {
$this->events[$id] = new Event($id, $description, microtime(true));
$this->writeLog($this->events[$id]);
}
}

Expand All @@ -54,6 +87,7 @@ public function end($id) {
if ($this->activated && isset($this->events[$id])) {
$timing = $this->events[$id];
$timing->end(microtime(true));
$this->writeLog($timing);
}
}

Expand All @@ -64,6 +98,7 @@ public function log($id, $description, $start, $end) {
if ($this->activated) {
$this->events[$id] = new Event($id, $description, $start);
$this->events[$id]->end($end);
$this->writeLog($this->events[$id]);
}
}

Expand All @@ -73,11 +108,29 @@ public function log($id, $description, $start, $end) {
public function getEvents() {
return $this->events;
}

/**
* @inheritdoc
*/
public function activate() {
$this->activated = true;
}

private function writeLog(IEvent $event) {
if ($this->activated) {
if ($event->getEnd() === null) {
return;
}
$duration = $event->getDuration();
$timeInMs = round($duration * 1000, 4);

$loggingMinimum = (int)$this->config->getValue('diagnostics.logging.threshold', 0);
if ($loggingMinimum > 0 && $timeInMs < $loggingMinimum) {
return;
}

$message = microtime() . ' - ' . $event->getId() . ': ' . $timeInMs . ' (' . $event->getDescription() . ')';
$this->logger->debug($message, ['app' => 'diagnostics']);
}
}
}
2 changes: 1 addition & 1 deletion lib/private/Log.php
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ public function log(int $level, string $message, array $context = []) {
}
}

private function getLogLevel($context) {
public function getLogLevel($context) {
$logCondition = $this->config->getValue('log.condition', []);

/**
Expand Down
11 changes: 8 additions & 3 deletions lib/private/RedisFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,24 @@

namespace OC;

use OCP\Diagnostics\IEventLogger;

class RedisFactory {
/** @var \Redis */
private $instance;

/** @var SystemConfig */
private $config;
private SystemConfig $config;

private IEventLogger $eventLogger;

/**
* RedisFactory constructor.
*
* @param SystemConfig $config
*/
public function __construct(SystemConfig $config) {
public function __construct(SystemConfig $config, IEventLogger $eventLogger) {
$this->config = $config;
$this->eventLogger = $eventLogger;
}

private function create() {
Expand Down Expand Up @@ -97,6 +101,7 @@ private function create() {
if (isset($config['dbindex'])) {
$this->instance->select($config['dbindex']);
}
$this->eventLogger->end('connect:redis');
}
}

Expand Down
9 changes: 2 additions & 7 deletions lib/private/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ public function __construct($webRoot, \OC\Config $config) {

$this->registerService('RedisFactory', function (Server $c) {
$systemConfig = $c->get(SystemConfig::class);
return new RedisFactory($systemConfig);
return new RedisFactory($systemConfig, $c->getEventLogger());
});

$this->registerService(\OCP\Activity\IManager::class, function (Server $c) {
Expand Down Expand Up @@ -843,12 +843,7 @@ public function __construct($webRoot, \OC\Config $config) {
$this->registerAlias(IClientService::class, ClientService::class);
$this->registerDeprecatedAlias('HttpClientService', IClientService::class);
$this->registerService(IEventLogger::class, function (ContainerInterface $c) {
$eventLogger = new EventLogger();
if ($c->get(SystemConfig::class)->getValue('debug', false)) {
// In debug mode, module is being activated by default
$eventLogger->activate();
}
return $eventLogger;
return new EventLogger($c->get(SystemConfig::class), $c->get(LoggerInterface::class), $c->get(Log::class));
});
/** @deprecated 19.0.0 */
$this->registerDeprecatedAlias('EventLogger', IEventLogger::class);
Expand Down
5 changes: 3 additions & 2 deletions lib/private/legacy/OC_App.php
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ public static function loadApp(string $app) {

$hasAppPhpFile = is_file($appPath . '/appinfo/app.php');

\OC::$server->getEventLogger()->start('bootstrap:load_app_' . $app, 'Load app: ' . $app);
if ($isBootable && $hasAppPhpFile) {
\OC::$server->getLogger()->error('/appinfo/app.php is not loaded when \OCP\AppFramework\Bootstrap\IBootstrap on the application class is used. Migrate everything from app.php to the Application class.', [
'app' => $app,
Expand All @@ -180,7 +181,6 @@ public static function loadApp(string $app) {
\OC::$server->getLogger()->debug('/appinfo/app.php is deprecated, use \OCP\AppFramework\Bootstrap\IBootstrap on the application class instead.', [
'app' => $app,
]);
\OC::$server->getEventLogger()->start('load_app_' . $app, 'Load app: ' . $app);
try {
self::requireAppFile($app);
} catch (Throwable $ex) {
Expand All @@ -200,8 +200,9 @@ public static function loadApp(string $app) {
]);
}
}
\OC::$server->getEventLogger()->end('load_app_' . $app);
}
\OC::$server->getEventLogger()->end('bootstrap:load_app_' . $app);

$coordinator->bootApp($app);

$info = self::getAppInfo($app);
Expand Down
2 changes: 2 additions & 0 deletions tests/lib/AppFramework/Bootstrap/CoordinatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
use OCP\AppFramework\Bootstrap\IRegistrationContext;
use OCP\AppFramework\QueryException;
use OCP\Dashboard\IManager;
use OCP\Diagnostics\IEventLogger;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\ILogger;
use OCP\IServerContainer;
Expand Down Expand Up @@ -78,6 +79,7 @@ protected function setUp(): void {
$this->crashReporterRegistry,
$this->dashboardManager,
$this->eventDispatcher,
$this->eventLogger,
$this->logger
);
}
Expand Down
Loading