NEXT-23811 - Implement configurable log handling for plugins, fixes #…

…2766
shopware · Jun 30, 2023 · 4faeb1a · 4faeb1a
1 parent d58f8b9
commit 4faeb1a
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 3 deletions.
diff --git a/changelog/_unreleased/2022-10-13-implement-configurable-plugin-log-handling.md b/changelog/_unreleased/2022-10-13-implement-configurable-plugin-log-handling.md
@@ -0,0 +1,9 @@
+---
+title: Implement consideration of log configuration for plugins
+issue: NEXT-23811
+author: Alexander Wink
+author_email: [email protected]
+author_github: @jinnoflife
+---
+# Core
+* Changed handling of LoggerFactory to consider log configuration of project
diff --git a/src/Core/Framework/DependencyInjection/services.xml b/src/Core/Framework/DependencyInjection/services.xml
@@ -652,6 +652,7 @@ base-uri 'self';
         <!-- Util -->
         <service id="Shopware\Core\Framework\Log\LoggerFactory">
             <argument type="string">%kernel.logs_dir%/%%s_%kernel.environment%.log</argument>
+            <argument type="service" id="logger"/>
             <argument>%shopware.logger.file_rotation_count%</argument>
         </service>
 

diff --git a/src/Core/Framework/Log/LoggerFactory.php b/src/Core/Framework/Log/LoggerFactory.php
@@ -2,6 +2,7 @@
 
 namespace Shopware\Core\Framework\Log;
 
+use Monolog\Handler\NullHandler;
 use Monolog\Handler\RotatingFileHandler;
 use Monolog\Level;
 use Monolog\Logger;
@@ -19,6 +20,7 @@ class LoggerFactory
      */
     public function __construct(
         private readonly string $rotatingFilePathPattern,
+        private readonly LoggerInterface $logger,
         private readonly int $defaultFileRotationCount = 14
     ) {
     }
@@ -28,12 +30,30 @@ public function __construct(
      */
     public function createRotating(string $filePrefix, ?int $fileRotationCount = null, int|Level $loggerLevel = Level::Debug): LoggerInterface
     {
-        $filepath = sprintf($this->rotatingFilePathPattern, $filePrefix);
-
         $result = new Logger($filePrefix);
-        $result->pushHandler(new RotatingFileHandler($filepath, $fileRotationCount ?? $this->defaultFileRotationCount, $loggerLevel));
         $result->pushProcessor(new PsrLogMessageProcessor());
 
+        /**
+         * Use RotatingFileHandler as fallback if Nullhandler or none is given
+         * If RotatingFileHandler is given (default configuration) -> use "default" logic for splitted logs
+         */
+        if (!method_exists($this->logger, 'getHandlers')
+            || (
+                \count($this->logger->getHandlers() ?? 0) === 1
+                && (
+                    current($this->logger->getHandlers()) instanceof NullHandler
+                    || current($this->logger->getHandlers()) instanceof RotatingFileHandler
+                )
+            )
+        ) {
+            $filepath = sprintf($this->rotatingFilePathPattern, $filePrefix);
+            $result->pushHandler(new RotatingFileHandler($filepath, $fileRotationCount ?? $this->defaultFileRotationCount, $loggerLevel));
+
+            return $result;
+        }
+
+        $result->setHandlers($this->logger->getHandlers());
+
         return $result;
     }
 }
diff --git a/tests/unit/php/Core/Framework/Log/LoggerFactoryTest.php b/tests/unit/php/Core/Framework/Log/LoggerFactoryTest.php
@@ -0,0 +1,61 @@
+<?php declare(strict_types=1);
+
+namespace Shopware\Tests\Unit\Core\Framework\Log;
+
+use Monolog\Handler\NullHandler;
+use Monolog\Handler\RotatingFileHandler;
+use Monolog\Logger;
+use Shopware\Core\Framework\Log\LoggerFactory;
+
+/**
+ * @internal
+ *
+ * @covers \Shopware\Core\Framework\Log\LoggerFactory
+ */
+class LoggerFactoryTest extends \PHPUnit\Framework\TestCase
+{
+    public function testNullLogHandler(): void
+    {
+        $providedHandler = [new NullHandler()];
+        $mainLogger = new Logger('test_logger', $providedHandler);
+        $loggerFactory = new LoggerFactory('test_case', $mainLogger);
+
+        /** @var \Monolog\Logger $createdLogger */
+        $createdLogger = $loggerFactory->createRotating('test_file_path');
+        $usedHandler = $createdLogger->getHandlers();
+
+        static::assertCount(1, $usedHandler);
+        static::assertInstanceOf(RotatingFileHandler::class, current($usedHandler), 'Handler differs from excpected');
+    }
+
+    public function testRotatingFileLogHandler(): void
+    {
+        $providedHandler = [new RotatingFileHandler('test')];
+        $mainLogger = new Logger('test_logger', $providedHandler);
+        $loggerFactory = new LoggerFactory('test_case', $mainLogger);
+
+        /** @var \Monolog\Logger $createdLogger */
+        $createdLogger = $loggerFactory->createRotating('test_file_path');
+        $usedHandler = $createdLogger->getHandlers();
+
+        static::assertCount(1, $usedHandler);
+        static::assertInstanceOf(RotatingFileHandler::class, current($usedHandler), 'Handler differs from excpected');
+    }
+
+    public function testMultipleLogHandlers(): void
+    {
+        $providedHandler = [
+            new RotatingFileHandler('test'),
+            new NullHandler(),
+        ];
+        $mainLogger = new Logger('test_logger', $providedHandler);
+        $loggerFactory = new LoggerFactory('test_case', $mainLogger);
+
+        /** @var \Monolog\Logger $createdLogger */
+        $createdLogger = $loggerFactory->createRotating('test_file_path');
+        $usedHandler = $createdLogger->getHandlers();
+
+        static::assertCount(\count($providedHandler), $usedHandler);
+        static::assertSame($providedHandler, $usedHandler, 'Handler differs from excpected');
+    }
+}