From 92d473a9d81b567affa7dbc79fe834a2431ac506 Mon Sep 17 00:00:00 2001 From: Holger Veltrup Date: Wed, 19 Jun 2024 16:46:28 +0200 Subject: [PATCH] feat: add log rotation checks --- config/services.yaml | 6 + src/Service/CheckStatus.php | 13 +- src/Service/Checker/MonologChecker.php | 194 ++++++++++++++---- test/Service/Checker/MonologCheckerTest.php | 70 ++++++- .../Checker/MonologCheckerTest/logging.log | 1 + .../Checker/MonologCheckerTest/logging.log.1 | 1 + .../MonologCheckerTest/logging.log.1.gz | 1 + .../Checker/MonologCheckerTest/logging.log.2 | 1 + .../MonologCheckerTest/logging.log.2.gz | 1 + 9 files changed, 239 insertions(+), 49 deletions(-) create mode 100644 test/resources/Service/Checker/MonologCheckerTest/logging.log.1 create mode 100644 test/resources/Service/Checker/MonologCheckerTest/logging.log.1.gz create mode 100644 test/resources/Service/Checker/MonologCheckerTest/logging.log.2 create mode 100644 test/resources/Service/Checker/MonologCheckerTest/logging.log.2.gz diff --git a/config/services.yaml b/config/services.yaml index 9ced087..0f6de3b 100644 --- a/config/services.yaml +++ b/config/services.yaml @@ -3,6 +3,9 @@ parameters: env(SUPERVISOR_GROUP_NAME): default ## The period in minutes after which the worker status file is updated. atoolo_runtime_check.worker_status_update_period: 10 + atoolo_runtime_check.checker.monolog.max_log_file_size: '100M' + atoolo_runtime_check.checker.monolog.max_log_dir_size: '10G' + atoolo_runtime_check.checker.monolog.max_log_file_rotations: 10 services: @@ -38,6 +41,9 @@ services: Atoolo\Runtime\Check\Service\Checker\MonologChecker: arguments: + - '%atoolo_runtime_check.checker.monolog.max_log_file_size%', + - '%atoolo_runtime_check.checker.monolog.max_log_dir_size%', + - '%atoolo_runtime_check.checker.monolog.max_log_file_rotations%', - '@logger' tags: - { name: 'atoolo_runtime_check.checker' } diff --git a/src/Service/CheckStatus.php b/src/Service/CheckStatus.php index d9e550b..d21ba1a 100644 --- a/src/Service/CheckStatus.php +++ b/src/Service/CheckStatus.php @@ -34,11 +34,22 @@ public static function createFailure(): self } public function addMessage(string $scope, string $message): self + { + return $this->addMessages($scope, [$message]); + } + + /** + * @param array $messages + */ + public function addMessages(string $scope, array $messages): self { if (!isset($this->messages[$scope])) { $this->messages[$scope] = []; } - $this->messages[$scope][] = $message; + $this->messages[$scope] = array_merge( + $this->messages[$scope], + $messages + ); return $this; } diff --git a/src/Service/Checker/MonologChecker.php b/src/Service/Checker/MonologChecker.php index 888b699..f4b53a4 100644 --- a/src/Service/Checker/MonologChecker.php +++ b/src/Service/Checker/MonologChecker.php @@ -9,10 +9,25 @@ use Monolog\Handler\FingersCrossedHandler; use Monolog\Logger; use Psr\Log\LoggerInterface; +use RecursiveDirectoryIterator; +use RecursiveIteratorIterator; +use SplFileInfo; +/** + * @phpstan-type ReportData = array{ + * logfile: ?string, + * level: string, + * logfile-size?: int, + * logdir-size?: int, + * logfile-rotations?: int + * } + */ class MonologChecker implements Checker { public function __construct( + private readonly string $maxLogFileSize, + private readonly string $maxLogDirSize, + private readonly int $maxLogFileRotations, private readonly LoggerInterface $logger ) { } @@ -54,84 +69,175 @@ private function checkHandler( CheckStatus $mergedStatus, StreamHandler $handler ): CheckStatus { + + $reportData = $this->getReportData($handler); $file = $handler->getUrl(); - if ($file === null) { + + $errors = []; + $logfileError = $this->checkLogfile($file); + if ($logfileError !== null) { + $errors[] = $logfileError; + } + $rotatingErrors = $this->checkLogRotating($reportData); + $errors = array_merge($errors, $rotatingErrors); + + if (!empty($errors)) { return $this->createFailure( $mergedStatus, - $handler, - 'logfile not set' + $reportData, + $errors ); } + + return $this->createSuccess( + $mergedStatus, + $reportData + ); + } + + /** + * @return ReportData + */ + private function getReportData(StreamHandler $handler): array + { + $file = $handler->getUrl(); + $reportData = [ + 'logfile' => $file, + 'level' => $handler->getLevel()->getName(), + ]; + + if ($file === null || !file_exists($file) || !is_readable($file)) { + return $reportData; + } + + $dir = dirname($file); + + $fileSize = filesize($file) ?: 0; + $reportData['logfile-size'] = $fileSize; + + $rii = new RecursiveIteratorIterator( + new RecursiveDirectoryIterator($dir) + ); + + $dirSize = 0; + /** @var SplFileInfo $f */ + foreach ($rii as $f) { + if ($f->isDir()) { + continue; + } + $dirSize += $f->getSize() ?: 0; + } + $reportData['logdir-size'] = $dirSize; + + $rotations = + count(glob($file . '.*.gz') ?: []) + + count(glob($file . '.[0-9]') ?: []); + $reportData['logfile-rotations'] = $rotations; + + return $reportData; + } + + private function checkLogfile(?string $file): ?string + { + if ($file === null) { + return'logfile not set'; + } if (file_exists($file) && !is_writable($file)) { - return $this->createFailure( - $mergedStatus, - $handler, - 'logfile not writable: ' . $file - ); + return 'logfile not writable: ' . $file; } if (!file_exists($file)) { $dir = dirname($file); if (!is_dir($dir)) { if (!@mkdir($dir, 0777, true) && !is_dir($dir)) { - return $this->createFailure( - $mergedStatus, - $handler, - 'log directory cannot be created: ' . $dir - ); + return 'log directory cannot be created: ' . $dir; } } if (!@touch($file)) { - return $this->createFailure( - $mergedStatus, - $handler, - 'logfile cannot be created: ' . $file - ); + return 'logfile cannot be created: ' . $file; } } - return $this->createSuccess( - $mergedStatus, - $handler - ); + return null; + } + + /** + * @param ReportData $reportData + * @return array + */ + private function checkLogRotating(array $reportData): array + { + $errors = []; + + $maxLogFileSize = $this->toMemoryStringToInteger($this->maxLogFileSize); + if ( + $maxLogFileSize > 0 + && ($reportData['logfile-size'] ?? 0) > $maxLogFileSize + ) { + $errors[] = 'logfile size exceeds ' + . $this->maxLogFileSize . ' bytes'; + } + + $maxLogDirSize = $this->toMemoryStringToInteger($this->maxLogDirSize); + if ( + $maxLogDirSize > 0 + && ($reportData['logdir-size'] ?? 0) > $maxLogDirSize + ) { + $errors[] = 'logdir size exceeds ' + . $this->maxLogDirSize . ' bytes'; + } + if ( + $this->maxLogFileRotations > 0 + && ($reportData['logfile-rotations'] ?? 0) + > $this->maxLogFileRotations + ) { + $errors[] = 'logfile rotations exceed ' + . $this->maxLogFileRotations; + } + + return $errors; } + /** + * @param ReportData $reportData + * @param array $messages + */ private function createFailure( CheckStatus $mergedStatus, - StreamHandler $handler, - string $message + array $reportData, + array $messages ): CheckStatus { $status = $mergedStatus->apply(CheckStatus::createFailure()); - $status->addMessage($this->getScope(), $message); - return $this->applyStatusReport($status, $handler); + $status->addMessages($this->getScope(), $messages); + return $this->applyStatusReport($status, $reportData); } + /** + * @param ReportData $reportData + */ private function createSuccess( CheckStatus $mergedStatus, - StreamHandler $handler, + array $reportData, ): CheckStatus { $status = $mergedStatus->apply(CheckStatus::createSuccess()); - return $this->applyStatusReport($status, $handler); + return $this->applyStatusReport($status, $reportData); } + /** + * @param ReportData $reportData + */ private function applyStatusReport( CheckStatus $status, - StreamHandler $handler + array $reportData ): CheckStatus { /** * @var array{ - * handler: array{ - * logfile: string, - * level: string - * } + * handler: ReportData * } $report */ $report = $status->getReport($this->getScope()); - $report['handler'][] = [ - 'logfile' => $handler->getUrl(), - 'level' => $handler->getLevel()->getName(), - ]; + $report['handler'][] = $reportData; $status->replaceReport($this->getScope(), $report); return $status; } @@ -156,4 +262,18 @@ private function getStreamHandlers( } return $handlers; } + + private function toMemoryStringToInteger(string $memory): int + { + [$number, $suffix] = sscanf($memory, '%u%c') ?? [null, null]; + if (!is_string($suffix)) { + return (int)$memory; + } + + $pos = stripos(' KMG', $suffix); + if (!is_int($pos) || !is_int($number)) { + return 0; + } + return $number * (1024 ** $pos); + } } diff --git a/test/Service/Checker/MonologCheckerTest.php b/test/Service/Checker/MonologCheckerTest.php index 69f38a0..01a1aa8 100644 --- a/test/Service/Checker/MonologCheckerTest.php +++ b/test/Service/Checker/MonologCheckerTest.php @@ -40,7 +40,7 @@ public function setUp(): void public function testCheckWithWrongLogger(): void { $logger = $this->createStub(LoggerInterface::class); - $checker = new MonologChecker($logger); + $checker = new MonologChecker('', '', 0, $logger); $status = $checker->check([]); $expected = CheckStatus::createFailure(); @@ -61,7 +61,7 @@ public function testCheckWithWrongLogger(): void public function testCheckWithoutStreamHandler(): void { $logger = $this->createStub(Logger::class); - $checker = new MonologChecker($logger); + $checker = new MonologChecker('', '', 0, $logger); $status = $checker->check([]); $expected = CheckStatus::createFailure(); @@ -84,7 +84,7 @@ public function testCheckWithoutLogfile(): void $handler->method('getLevel')->willReturn(Level::Warning); $logger->method('getHandlers')->willReturn([$handler]); - $checker = new MonologChecker($logger); + $checker = new MonologChecker('', '', 0, $logger); $status = $checker->check([]); $expected = CheckStatus::createFailure(); @@ -129,7 +129,7 @@ public function testCheckLogfileNotWritable(): void $logger = $this->createStub(Logger::class); $logger->method('getHandlers')->willReturn([$handler]); - $checker = new MonologChecker($logger); + $checker = new MonologChecker('', '', 0, $logger); $status = $checker->check([]); $expected = CheckStatus::createFailure(); @@ -137,7 +137,10 @@ public function testCheckLogfileNotWritable(): void 'handler' => [ [ 'logfile' => $file, - 'level' => 'WARNING' + 'level' => 'WARNING', + 'logfile-size' => 0, + 'logdir-size' => 0, + 'logfile-rotations' => 0 ] ] ]); @@ -170,7 +173,7 @@ public function testCheckDirNotCreateable(): void $logger = $this->createStub(Logger::class); $logger->method('getHandlers')->willReturn([$handler]); - $checker = new MonologChecker($logger); + $checker = new MonologChecker('', '', 0, $logger); $status = $checker->check([]); $expected = CheckStatus::createFailure(); @@ -212,7 +215,7 @@ public function testCheckFileNotCreateable(): void $logger = $this->createStub(Logger::class); $logger->method('getHandlers')->willReturn([$handler]); - $checker = new MonologChecker($logger); + $checker = new MonologChecker('', '', 0, $logger); $status = $checker->check([]); $expected = CheckStatus::createFailure(); @@ -248,7 +251,7 @@ public function testCheckSuccessfully(): void $logger = $this->createStub(Logger::class); $logger->method('getHandlers')->willReturn([$handler]); - $checker = new MonologChecker($logger); + $checker = new MonologChecker('10M', '10X', 0, $logger); $status = $checker->check([]); $expected = CheckStatus::createSuccess(); @@ -256,7 +259,10 @@ public function testCheckSuccessfully(): void 'handler' => [ [ 'logfile' => $file, - 'level' => 'WARNING' + 'level' => 'WARNING', + 'logfile-size' => 18, + 'logdir-size' => 70, + 'logfile-rotations' => 4 ] ] ]); @@ -285,7 +291,7 @@ public function testWithFingersCrossedHandler(): void $logger = $this->createStub(Logger::class); $logger->method('getHandlers')->willReturn([$fingersCrossedHandler]); - $checker = new MonologChecker($logger); + $checker = new MonologChecker('', '', 0, $logger); $status = $checker->check([]); $expected = CheckStatus::createSuccess(); @@ -293,7 +299,10 @@ public function testWithFingersCrossedHandler(): void 'handler' => [ [ 'logfile' => $file, - 'level' => 'WARNING' + 'level' => 'WARNING', + 'logfile-size' => 18, + 'logdir-size' => 70, + 'logfile-rotations' => 4 ] ] ]); @@ -303,4 +312,43 @@ public function testWithFingersCrossedHandler(): void 'Status is not as expected' ); } + + public function testCheckLogRotatingWithErrors(): void + { + $file = $this->resourceDir . '/logging.log'; + $handler = $this->createStub(StreamHandler::class); + $handler->method('getUrl')->willReturn($file); + $handler->method('getLevel')->willReturn(Level::Warning); + $logger = $this->createStub(Logger::class); + $logger->method('getHandlers')->willReturn([$handler]); + + $checker = new MonologChecker('10', '20', 1, $logger); + $status = $checker->check([]); + + $expected = CheckStatus::createFailure(); + $expected->addReport('logging', [ + 'handler' => [ + [ + 'logfile' => $file, + 'level' => 'WARNING', + 'logfile-size' => 18, + 'logdir-size' => 70, + 'logfile-rotations' => 4 + ] + ] + ]); + $expected->addMessages( + 'logging', + [ + 'logfile size exceeds 10 bytes', + 'logdir size exceeds 20 bytes', + 'logfile rotations exceed 1' + ] + ); + $this->assertEquals( + $expected, + $status, + 'Status is not as expected' + ); + } } diff --git a/test/resources/Service/Checker/MonologCheckerTest/logging.log b/test/resources/Service/Checker/MonologCheckerTest/logging.log index e69de29..4e18c57 100644 --- a/test/resources/Service/Checker/MonologCheckerTest/logging.log +++ b/test/resources/Service/Checker/MonologCheckerTest/logging.log @@ -0,0 +1 @@ +one log file entry \ No newline at end of file diff --git a/test/resources/Service/Checker/MonologCheckerTest/logging.log.1 b/test/resources/Service/Checker/MonologCheckerTest/logging.log.1 new file mode 100644 index 0000000..4e18c57 --- /dev/null +++ b/test/resources/Service/Checker/MonologCheckerTest/logging.log.1 @@ -0,0 +1 @@ +one log file entry \ No newline at end of file diff --git a/test/resources/Service/Checker/MonologCheckerTest/logging.log.1.gz b/test/resources/Service/Checker/MonologCheckerTest/logging.log.1.gz new file mode 100644 index 0000000..b824260 --- /dev/null +++ b/test/resources/Service/Checker/MonologCheckerTest/logging.log.1.gz @@ -0,0 +1 @@ +dummy gz \ No newline at end of file diff --git a/test/resources/Service/Checker/MonologCheckerTest/logging.log.2 b/test/resources/Service/Checker/MonologCheckerTest/logging.log.2 new file mode 100644 index 0000000..4e18c57 --- /dev/null +++ b/test/resources/Service/Checker/MonologCheckerTest/logging.log.2 @@ -0,0 +1 @@ +one log file entry \ No newline at end of file diff --git a/test/resources/Service/Checker/MonologCheckerTest/logging.log.2.gz b/test/resources/Service/Checker/MonologCheckerTest/logging.log.2.gz new file mode 100644 index 0000000..b824260 --- /dev/null +++ b/test/resources/Service/Checker/MonologCheckerTest/logging.log.2.gz @@ -0,0 +1 @@ +dummy gz \ No newline at end of file