Modernize and add test

README.md

@@ -1,6 +1,9 @@
 # Ecs Logging Bundle
 
 ![CI](https://github.com/aubes/ecs-logging-bundle/actions/workflows/php.yml/badge.svg)
+[![Latest Stable Version](https://img.shields.io/packagist/v/aubes/ecs-logging-bundle)](https://packagist.org/packages/aubes/ecs-logging-bundle)
+[![License](https://img.shields.io/packagist/l/aubes/ecs-logging-bundle)](https://packagist.org/packages/aubes/ecs-logging-bundle)
+[![PHP Version](https://img.shields.io/packagist/dependency-v/aubes/ecs-logging-bundle/php)](https://packagist.org/packages/aubes/ecs-logging-bundle)
 
 This Symfony bundle provides the [Ecs](https://www.elastic.co/guide/en/ecs/current/ecs-reference.html) log format for Monolog.
 

composer.json

@@ -13,9 +13,9 @@
         "php": ">=8.1",
         "elastic/ecs-logging": "^2.0.0",
         "monolog/monolog": "^3.0",
-        "symfony/polyfill-php80": "^1.0",
         "symfony/http-foundation": "^6.4 | ^7.0 | ^8.0",
-        "symfony/http-kernel": "^6.4 | ^7.0 | ^8.0"
+        "symfony/http-kernel": "^6.4 | ^7.0 | ^8.0",
+        "symfony/yaml": "^6.4 | ^7.0 | ^8.0"
     },
     "require-dev": {
         "friendsofphp/php-cs-fixer": "^3.1",
@@ -44,6 +44,7 @@
         "fix-cs": "php-cs-fixer fix --allow-risky=yes --config=.php-cs-fixer.php --show-progress=dots --verbose",
         "pmd": "phpmd src text .pmd-ruleset.xml",
         "psalm": "psalm --show-info=true",
-        "test": "phpunit tests"
+        "test": "phpunit tests",
+        "coverage": "phpunit tests --coverage-html coverage/ --coverage-clover coverage/clover.xml"
     }
 }

src/DependencyInjection/Configuration.php

@@ -22,6 +22,7 @@ public function getConfigTreeBuilder(): TreeBuilder
         $rootNode
             ->children()
                 ->arrayNode('monolog')
+                    ->addDefaultsIfNotSet()
                     ->children()
                         ->arrayNode('channels')
                             ->info('Default logging channel list the processors should be pushed to')
@@ -95,7 +96,7 @@ public function getConfigTreeBuilder(): TreeBuilder
     /**
      * @psalm-suppress UndefinedMethod
      */
-    public function addHandlersNode(): NodeDefinition
+    private function addHandlersNode(): NodeDefinition
     {
         $treeBuilder = new TreeBuilder('handlers');
 
@@ -108,7 +109,7 @@ public function addHandlersNode(): NodeDefinition
     /**
      * @psalm-suppress UndefinedMethod
      */
-    public function addChannelsNode(): NodeDefinition
+    private function addChannelsNode(): NodeDefinition
     {
         $treeBuilder = new TreeBuilder('channels');
 

src/DependencyInjection/EcsLoggingExtension.php

@@ -45,7 +45,7 @@ public function load(array $configs, ContainerBuilder $container): void
         }
     }
 
-    protected function configureAutoLabelProcessor(array $config, ContainerBuilder $container): void
+    private function configureAutoLabelProcessor(array $config, ContainerBuilder $container): void
     {
         if (!isset($config['processor']['auto_label']) || !$config['processor']['auto_label']['enabled']) {
             return;
@@ -56,12 +56,12 @@ protected function configureAutoLabelProcessor(array $config, ContainerBuilder $
         $processor = new Definition(AutoLabelProcessor::class);
         $processor->setArgument('$fields', $processorConfig['fields']);
 
-        $this->configureMonologProcessor($config, $processorConfig, $processor);
+        $this->configureMonologProcessor($config, $processorConfig, $processor, -10);
 
         $container->setDefinition('.ecs_logging.processor.auto_label', $processor);
     }
 
-    protected function configureErrorProcessor(array $config, ContainerBuilder $container): void
+    private function configureErrorProcessor(array $config, ContainerBuilder $container): void
     {
         if (!isset($config['processor']['error']) || !$config['processor']['error']['enabled']) {
             return;
@@ -77,7 +77,7 @@ protected function configureErrorProcessor(array $config, ContainerBuilder $cont
         $container->setDefinition('.ecs_logging.processor.error', $processor);
     }
 
-    protected function configureServiceProcessor(array $config, ContainerBuilder $container): void
+    private function configureServiceProcessor(array $config, ContainerBuilder $container): void
     {
         if (!isset($config['processor']['service']) || !$config['processor']['service']['enabled']) {
             return;
@@ -128,7 +128,7 @@ protected function configureServiceProcessor(array $config, ContainerBuilder $co
         $container->setDefinition('.ecs_logging.processor.service', $processor);
     }
 
-    protected function configureTracingProcessor(array $config, ContainerBuilder $container): void
+    private function configureTracingProcessor(array $config, ContainerBuilder $container): void
     {
         if (!isset($config['processor']['tracing']) || !$config['processor']['tracing']['enabled']) {
             return;
@@ -144,7 +144,7 @@ protected function configureTracingProcessor(array $config, ContainerBuilder $co
         $container->setDefinition('.ecs_logging.processor.tracing', $processor);
     }
 
-    protected function configureUserProcessor(array $config, ContainerBuilder $container): void
+    private function configureUserProcessor(array $config, ContainerBuilder $container): void
     {
         if (!isset($config['processor']['user']) || !$config['processor']['user']['enabled']) {
             return;
@@ -168,16 +168,16 @@ protected function configureUserProcessor(array $config, ContainerBuilder $conta
         $container->setDefinition('.ecs_logging.processor.user', $processor);
     }
 
-    protected function configureMonologProcessor(array $config, array $configOverride, Definition $processor): void
+    private function configureMonologProcessor(array $config, array $configOverride, Definition $processor, int $priority = 0): void
     {
         $channels = !empty($configOverride['channels']) ? $configOverride['channels'] : $config['monolog']['channels'];
         foreach ($channels as $channel) {
-            $processor->addTag('monolog.processor', ['channel' => $channel]);
+            $processor->addTag('monolog.processor', ['channel' => $channel, 'priority' => $priority]);
         }
 
         $handlers = !empty($configOverride['handlers']) ? $configOverride['handlers'] : $config['monolog']['handlers'];
         foreach ($handlers as $handler) {
-            $processor->addTag('monolog.processor', ['handler' => $handler]);
+            $processor->addTag('monolog.processor', ['handler' => $handler, 'priority' => $priority]);
         }
     }
 }

src/Logger/AbstractProcessor.php

@@ -8,11 +8,8 @@
 
 abstract class AbstractProcessor
 {
-    protected string $fieldName;
-
-    public function __construct(string $fieldName)
+    public function __construct(protected readonly string $fieldName)
     {
-        $this->fieldName = $fieldName;
     }
 
     abstract public function transformValue(mixed $value): mixed;

src/Logger/AutoLabelProcessor.php

@@ -146,7 +146,7 @@ final class AutoLabelProcessor
         self::FIELD_X509,
     ];
 
-    private array $ecsFields;
+    private readonly array $ecsFields;
 
     public function __construct(array $fields)
     {
@@ -156,13 +156,29 @@ public function __construct(array $fields)
     public function __invoke(LogRecord $record): LogRecord
     {
         $context = $record->context;
+        $nonEcsFields = [];
+
         foreach ($context as $contextName => $contextValue) {
             if (!isset($this->ecsFields[$contextName])) {
-                $context['labels'][$contextName] = $contextValue;
-                unset($context[$contextName]);
+                $nonEcsFields[$contextName] = $contextValue;
             }
         }
 
+        if (empty($nonEcsFields)) {
+            return $record;
+        }
+
+        foreach (\array_keys($nonEcsFields) as $contextName) {
+            unset($context[$contextName]);
+        }
+
+        $existingLabels = $context['labels'] ?? [];
+        if (!\is_array($existingLabels)) {
+            throw new \InvalidArgumentException(\sprintf('The "labels" context field must be an array, "%s" given.', \get_debug_type($existingLabels)));
+        }
+
+        $context['labels'] = \array_merge($existingLabels, $nonEcsFields);
+
         return $record->with(context: $context);
     }
 }

src/Logger/ServiceProcessor.php

@@ -9,11 +9,8 @@
 
 final class ServiceProcessor
 {
-    private Service $service;
-
-    public function __construct(Service $service)
+    public function __construct(private readonly Service $service)
     {
-        $this->service = $service;
     }
 
     public function __invoke(LogRecord $record): LogRecord

src/Logger/UserProcessor.php

@@ -9,13 +9,10 @@
 
 final class UserProcessor
 {
-    private EcsUserProviderInterface $provider;
-    private ?string $domain;
-
-    public function __construct(EcsUserProviderInterface $provider, ?string $domain = null)
-    {
-        $this->provider = $provider;
-        $this->domain = $domain;
+    public function __construct(
+        private readonly EcsUserProviderInterface $provider,
+        private readonly ?string $domain = null,
+    ) {
     }
 
     public function support(LogRecord $record): bool

src/Security/EcsUserProvider.php

@@ -9,25 +9,16 @@
 
 final class EcsUserProvider implements EcsUserProviderInterface
 {
-    private TokenStorageInterface $tokenStorage;
-
-    public function __construct(TokenStorageInterface $tokenStorage)
+    public function __construct(private readonly TokenStorageInterface $tokenStorage)
     {
-        $this->tokenStorage = $tokenStorage;
     }
 
     /**
      * @psalm-suppress InternalMethod
      */
     public function getUser(): ?User
     {
-        $token = $this->tokenStorage->getToken();
-
-        if ($token === null) {
-            return null;
-        }
-
-        $user = $token->getUser();
+        $user = $this->tokenStorage->getToken()?->getUser();
 
         if ($user !== null) {
             $ecsUser = new User();

tests/Logger/AutoLabelProcessorTest.php

@@ -4,15 +4,134 @@
 
 namespace Aubes\EcsLoggingBundle\Tests\Logger;
 
+use Aubes\EcsLoggingBundle\Logger\AutoLabelProcessor;
+use Monolog\Level;
+use Monolog\LogRecord;
 use PHPUnit\Framework\TestCase;
-use Prophecy\PhpUnit\ProphecyTrait;
 
 class AutoLabelProcessorTest extends TestCase
 {
-    use ProphecyTrait;
+    private function createRecord(array $context): LogRecord
+    {
+        return new LogRecord(
+            new \DateTimeImmutable(),
+            'channel',
+            Level::Info,
+            'message',
+            $context
+        );
+    }
+
+    public function testNonEcsFieldsAreMovedToLabels(): void
+    {
+        $processor = new AutoLabelProcessor([]);
+
+        $record = $this->createRecord(['foo' => 'bar', 'baz' => 'qux']);
+        $record = $processor($record);
+
+        $this->assertArrayHasKey('labels', $record->context);
+        $this->assertSame('bar', $record->context['labels']['foo']);
+        $this->assertSame('qux', $record->context['labels']['baz']);
+        $this->assertArrayNotHasKey('foo', $record->context);
+        $this->assertArrayNotHasKey('baz', $record->context);
+    }
+
+    public function testEcsFieldsAreNotMoved(): void
+    {
+        $processor = new AutoLabelProcessor(['service', 'error']);
+
+        $record = $this->createRecord(['service' => 'my-service', 'error' => 'some-error', 'custom' => 'value']);
+        $record = $processor($record);
+
+        $this->assertArrayHasKey('service', $record->context);
+        $this->assertArrayHasKey('error', $record->context);
+        $this->assertArrayHasKey('labels', $record->context);
+        $this->assertSame('value', $record->context['labels']['custom']);
+        $this->assertArrayNotHasKey('custom', $record->context);
+    }
+
+    public function testEmptyContextIsUnchanged(): void
+    {
+        $processor = new AutoLabelProcessor(AutoLabelProcessor::FIELDS_ALL);
+
+        $record = $this->createRecord([]);
+        $record = $processor($record);
+
+        $this->assertSame([], $record->context);
+    }
+
+    public function testAllContextFieldsAreEcsFields(): void
+    {
+        $processor = new AutoLabelProcessor(['foo', 'bar']);
+
+        $record = $this->createRecord(['foo' => 1, 'bar' => 2]);
+        $record = $processor($record);
 
-    public function testProcessor()
+        $this->assertArrayHasKey('foo', $record->context);
+        $this->assertArrayHasKey('bar', $record->context);
+        $this->assertArrayNotHasKey('labels', $record->context);
+    }
+
+    public function testFieldsMinimalConstant(): void
+    {
+        $processor = new AutoLabelProcessor(AutoLabelProcessor::FIELDS_MINIMAL);
+
+        $record = $this->createRecord([
+            'message' => 'text',
+            'service' => 'svc',
+            'custom_field' => 'moved',
+        ]);
+        $record = $processor($record);
+
+        $this->assertArrayHasKey('message', $record->context);
+        $this->assertArrayHasKey('service', $record->context);
+        $this->assertArrayHasKey('labels', $record->context);
+        $this->assertSame('moved', $record->context['labels']['custom_field']);
+        $this->assertArrayNotHasKey('custom_field', $record->context);
+    }
+
+    public function testFieldsBundleConstant(): void
+    {
+        $processor = new AutoLabelProcessor(AutoLabelProcessor::FIELDS_BUNDLE);
+
+        $record = $this->createRecord([
+            'error' => 'some-error',
+            'user' => 'some-user',
+            'unexpected' => 'should-be-labeled',
+        ]);
+        $record = $processor($record);
+
+        $this->assertArrayHasKey('error', $record->context);
+        $this->assertArrayHasKey('user', $record->context);
+        $this->assertSame('should-be-labeled', $record->context['labels']['unexpected']);
+        $this->assertArrayNotHasKey('unexpected', $record->context);
+    }
+
+    public function testInvalidLabelsThrowsException(): void
+    {
+        // 'labels' is declared as an ECS field (stays in context), but holds a non-array value
+        $processor = new AutoLabelProcessor(['labels']);
+
+        $record = $this->createRecord(['labels' => 'not-an-array', 'foo' => 'bar']);
+
+        $this->expectException(\InvalidArgumentException::class);
+        $this->expectExceptionMessage('The "labels" context field must be an array, "string" given.');
+
+        $processor($record);
+    }
+
+    public function testExistingLabelsAreMerged(): void
     {
-        // todo
+        $processor = new AutoLabelProcessor(['labels']);
+
+        $record = $this->createRecord([
+            'labels' => ['existing' => 'value'],
+            'new_field' => 'new_value',
+        ]);
+        $record = $processor($record);
+
+        $this->assertArrayHasKey('labels', $record->context);
+        $this->assertSame('value', $record->context['labels']['existing']);
+        $this->assertSame('new_value', $record->context['labels']['new_field']);
     }
 }