From 19450e13846f5fe4f1abc332b4f70ad17ce468b8 Mon Sep 17 00:00:00 2001 From: aubes <3941035+aubes@users.noreply.github.com> Date: Mon, 16 Mar 2026 22:36:36 +0100 Subject: [PATCH 1/3] Modernize and add test --- README.md | 3 + composer.json | 4 +- src/DependencyInjection/Configuration.php | 4 +- .../EcsLoggingExtension.php | 12 +- src/Logger/AbstractProcessor.php | 5 +- src/Logger/AutoLabelProcessor.php | 2 +- src/Logger/ServiceProcessor.php | 5 +- src/Logger/UserProcessor.php | 11 +- src/Security/EcsUserProvider.php | 13 +- tests/Logger/AutoLabelProcessorTest.php | 113 +++++++++++++++++- tests/Logger/ErrorProcessorTest.php | 22 ++++ tests/Logger/ServiceProcessorTest.php | 61 +++++++++- tests/Logger/TracingProcessorTest.php | 44 +++++++ tests/Logger/UserProcessorTest.php | 71 ++++++++++- tests/Security/EcsUserProviderTest.php | 56 ++++++++- 15 files changed, 380 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 32c3131..e69f2d5 100644 --- a/README.md +++ b/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. diff --git a/composer.json b/composer.json index 640f5ce..855f9d8 100644 --- a/composer.json +++ b/composer.json @@ -13,7 +13,6 @@ "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" }, @@ -44,6 +43,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" } } diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 03eab3c..7d8be29 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -95,7 +95,7 @@ public function getConfigTreeBuilder(): TreeBuilder /** * @psalm-suppress UndefinedMethod */ - public function addHandlersNode(): NodeDefinition + private function addHandlersNode(): NodeDefinition { $treeBuilder = new TreeBuilder('handlers'); @@ -108,7 +108,7 @@ public function addHandlersNode(): NodeDefinition /** * @psalm-suppress UndefinedMethod */ - public function addChannelsNode(): NodeDefinition + private function addChannelsNode(): NodeDefinition { $treeBuilder = new TreeBuilder('channels'); diff --git a/src/DependencyInjection/EcsLoggingExtension.php b/src/DependencyInjection/EcsLoggingExtension.php index 7b91186..fb74dd0 100644 --- a/src/DependencyInjection/EcsLoggingExtension.php +++ b/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; @@ -61,7 +61,7 @@ protected function configureAutoLabelProcessor(array $config, ContainerBuilder $ $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,7 +168,7 @@ 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): void { $channels = !empty($configOverride['channels']) ? $configOverride['channels'] : $config['monolog']['channels']; foreach ($channels as $channel) { diff --git a/src/Logger/AbstractProcessor.php b/src/Logger/AbstractProcessor.php index f385022..78f68b3 100644 --- a/src/Logger/AbstractProcessor.php +++ b/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; diff --git a/src/Logger/AutoLabelProcessor.php b/src/Logger/AutoLabelProcessor.php index e2b2a49..b4a6f98 100644 --- a/src/Logger/AutoLabelProcessor.php +++ b/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) { diff --git a/src/Logger/ServiceProcessor.php b/src/Logger/ServiceProcessor.php index 042f048..f758730 100644 --- a/src/Logger/ServiceProcessor.php +++ b/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 diff --git a/src/Logger/UserProcessor.php b/src/Logger/UserProcessor.php index ec86484..c7fdf06 100644 --- a/src/Logger/UserProcessor.php +++ b/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 diff --git a/src/Security/EcsUserProvider.php b/src/Security/EcsUserProvider.php index 6b66807..cb831c6 100644 --- a/src/Security/EcsUserProvider.php +++ b/src/Security/EcsUserProvider.php @@ -9,11 +9,8 @@ final class EcsUserProvider implements EcsUserProviderInterface { - private TokenStorageInterface $tokenStorage; - - public function __construct(TokenStorageInterface $tokenStorage) + public function __construct(private readonly TokenStorageInterface $tokenStorage) { - $this->tokenStorage = $tokenStorage; } /** @@ -21,13 +18,7 @@ public function __construct(TokenStorageInterface $tokenStorage) */ 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(); diff --git a/tests/Logger/AutoLabelProcessorTest.php b/tests/Logger/AutoLabelProcessorTest.php index 12e6125..1a7e651 100644 --- a/tests/Logger/AutoLabelProcessorTest.php +++ b/tests/Logger/AutoLabelProcessorTest.php @@ -4,6 +4,9 @@ namespace Aubes\EcsLoggingBundle\Tests\Logger; +use Aubes\EcsLoggingBundle\Logger\AutoLabelProcessor; +use Monolog\Level; +use Monolog\LogRecord; use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; @@ -11,8 +14,114 @@ class AutoLabelProcessorTest extends TestCase { use ProphecyTrait; - public function testProcessor() + private function createRecord(array $context): LogRecord { - // todo + 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); + + $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 testExistingLabelsAreMerged(): void + { + $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']); } } diff --git a/tests/Logger/ErrorProcessorTest.php b/tests/Logger/ErrorProcessorTest.php index d59e2c9..d0edf74 100644 --- a/tests/Logger/ErrorProcessorTest.php +++ b/tests/Logger/ErrorProcessorTest.php @@ -96,4 +96,26 @@ public function testWithNonThrowableErrorProcessor() $this->assertArrayHasKey('context', $record); $this->assertArrayNotHasKey('error', $record->context); } + + public function testWithAlreadyTransformedErrorProcessor() + { + $processor = new ErrorProcessor('error'); + + $ecsError = new Error(new \Exception('already transformed')); + + $record = new LogRecord( + new \DateTimeImmutable(), + 'channel', + Level::Info, + 'message', + [ + 'error' => $ecsError, + ] + ); + + $record = $processor($record); + + $this->assertArrayHasKey('error', $record->context); + $this->assertSame($ecsError, $record->context['error']); + } } diff --git a/tests/Logger/ServiceProcessorTest.php b/tests/Logger/ServiceProcessorTest.php index abdc9df..7079685 100644 --- a/tests/Logger/ServiceProcessorTest.php +++ b/tests/Logger/ServiceProcessorTest.php @@ -4,6 +4,10 @@ namespace Aubes\EcsLoggingBundle\Tests\Logger; +use Aubes\EcsLoggingBundle\Logger\ServiceProcessor; +use Elastic\Types\Service; +use Monolog\Level; +use Monolog\LogRecord; use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; @@ -11,8 +15,61 @@ class ServiceProcessorTest extends TestCase { use ProphecyTrait; - public function testProcessor() + private function createRecord(array $context): LogRecord { - // todo + return new LogRecord( + new \DateTimeImmutable(), + 'channel', + Level::Info, + 'message', + $context + ); + } + + public function testServiceIsInjectedWhenAbsent(): void + { + $service = new Service(); + $service->setName('my-app'); + $service->setVersion('1.0.0'); + + $processor = new ServiceProcessor($service); + + $record = $this->createRecord([]); + $record = $processor($record); + + $this->assertArrayHasKey('service', $record->context); + $this->assertSame($service, $record->context['service']); + } + + public function testServiceIsNotOverriddenWhenAlreadyPresent(): void + { + $injectedService = new Service(); + $injectedService->setName('injected'); + + $existingService = new Service(); + $existingService->setName('existing'); + + $processor = new ServiceProcessor($injectedService); + + $record = $this->createRecord(['service' => $existingService]); + $record = $processor($record); + + $this->assertArrayHasKey('service', $record->context); + $this->assertSame($existingService, $record->context['service']); + $this->assertNotSame($injectedService, $record->context['service']); + } + + public function testOtherContextFieldsArePreserved(): void + { + $service = new Service(); + + $processor = new ServiceProcessor($service); + + $record = $this->createRecord(['foo' => 'bar']); + $record = $processor($record); + + $this->assertArrayHasKey('service', $record->context); + $this->assertArrayHasKey('foo', $record->context); + $this->assertSame('bar', $record->context['foo']); } } diff --git a/tests/Logger/TracingProcessorTest.php b/tests/Logger/TracingProcessorTest.php index 082ad71..f04f9fe 100644 --- a/tests/Logger/TracingProcessorTest.php +++ b/tests/Logger/TracingProcessorTest.php @@ -100,4 +100,48 @@ public function testWithTracingWithoutTraceIdProcessor() $this->assertArrayNotHasKey('tracing', $record->context); } + + public function testWithTracingWithoutTransactionIdProcessor() + { + $processor = new TracingProcessor('tracing'); + + $record = new LogRecord( + new \DateTimeImmutable(), + 'channel', + Level::Info, + 'message', + [ + 'tracing' => [ + 'trace_id' => 'abc123', + ], + ] + ); + + $record = $processor($record); + + $this->assertArrayHasKey('tracing', $record->context); + $this->assertInstanceOf(Tracing::class, $record->context['tracing']); + } + + public function testWithAlreadyTransformedTracingProcessor() + { + $processor = new TracingProcessor('tracing'); + + $existingTracing = new Tracing('abc123', 'txn456'); + + $record = new LogRecord( + new \DateTimeImmutable(), + 'channel', + Level::Info, + 'message', + [ + 'tracing' => $existingTracing, + ] + ); + + $record = $processor($record); + + $this->assertArrayHasKey('tracing', $record->context); + $this->assertSame($existingTracing, $record->context['tracing']); + } } diff --git a/tests/Logger/UserProcessorTest.php b/tests/Logger/UserProcessorTest.php index 60894d7..0c9dd10 100644 --- a/tests/Logger/UserProcessorTest.php +++ b/tests/Logger/UserProcessorTest.php @@ -90,8 +90,75 @@ public function testUserDefinedProcessor() $this->assertSame('User Id', $record->context['user']['id']); } - public function testDomain() + public function testDomainFromProviderTakesPriority(): void { - // todo + $user = new User(); + + $provider = $this->prophesize(EcsUserProviderInterface::class); + $provider->getDomain()->willReturn('provider_domain'); + $provider->getUser()->willReturn($user); + + $processor = new UserProcessor($provider->reveal(), 'constructor_domain'); + + $record = new LogRecord( + new \DateTimeImmutable(), + 'channel', + Level::Info, + 'message', + [] + ); + + $record = $processor($record); + + $this->assertArrayHasKey('user', $record->context); + $this->assertSame('provider_domain', $record->context['user']->jsonSerialize()['user']['domain']); + } + + public function testDomainFallbackToConstructor(): void + { + $user = new User(); + + $provider = $this->prophesize(EcsUserProviderInterface::class); + $provider->getDomain()->willReturn(null); + $provider->getUser()->willReturn($user); + + $processor = new UserProcessor($provider->reveal(), 'constructor_domain'); + + $record = new LogRecord( + new \DateTimeImmutable(), + 'channel', + Level::Info, + 'message', + [] + ); + + $record = $processor($record); + + $this->assertArrayHasKey('user', $record->context); + $this->assertSame('constructor_domain', $record->context['user']->jsonSerialize()['user']['domain']); + } + + public function testNoDomainWhenBothAreNull(): void + { + $user = new User(); + + $provider = $this->prophesize(EcsUserProviderInterface::class); + $provider->getDomain()->willReturn(null); + $provider->getUser()->willReturn($user); + + $processor = new UserProcessor($provider->reveal(), null); + + $record = new LogRecord( + new \DateTimeImmutable(), + 'channel', + Level::Info, + 'message', + [] + ); + + $record = $processor($record); + + $this->assertArrayHasKey('user', $record->context); + $this->assertArrayNotHasKey('domain', $record->context['user']->jsonSerialize()['user'] ?? []); } } diff --git a/tests/Security/EcsUserProviderTest.php b/tests/Security/EcsUserProviderTest.php index bea8c39..9ec9e7e 100644 --- a/tests/Security/EcsUserProviderTest.php +++ b/tests/Security/EcsUserProviderTest.php @@ -2,17 +2,67 @@ declare(strict_types=1); -namespace Aubes\EcsLoggingBundle\Tests\Transformer; +namespace Aubes\EcsLoggingBundle\Tests\Security; +use Aubes\EcsLoggingBundle\Security\EcsUserProvider; +use Elastic\Types\User; use PHPUnit\Framework\TestCase; use Prophecy\PhpUnit\ProphecyTrait; +use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\User\UserInterface; class EcsUserProviderTest extends TestCase { use ProphecyTrait; - public function testWithUserTransformer() + public function testGetUserReturnsEcsUserWhenAuthenticated(): void { - // todo + $symfonyUser = $this->prophesize(UserInterface::class); + $symfonyUser->getUserIdentifier()->willReturn('user@example.com'); + + $token = $this->prophesize(TokenInterface::class); + $token->getUser()->willReturn($symfonyUser->reveal()); + + $tokenStorage = $this->prophesize(TokenStorageInterface::class); + $tokenStorage->getToken()->willReturn($token->reveal()); + + $provider = new EcsUserProvider($tokenStorage->reveal()); + $user = $provider->getUser(); + + $this->assertInstanceOf(User::class, $user); + $this->assertSame('user@example.com', $user->jsonSerialize()['user']['id']); + } + + public function testGetUserReturnsNullWhenNoToken(): void + { + $tokenStorage = $this->prophesize(TokenStorageInterface::class); + $tokenStorage->getToken()->willReturn(null); + + $provider = new EcsUserProvider($tokenStorage->reveal()); + + $this->assertNull($provider->getUser()); + } + + public function testGetUserReturnsNullWhenTokenHasNoUser(): void + { + $token = $this->prophesize(TokenInterface::class); + $token->getUser()->willReturn(null); + + $tokenStorage = $this->prophesize(TokenStorageInterface::class); + $tokenStorage->getToken()->willReturn($token->reveal()); + + $provider = new EcsUserProvider($tokenStorage->reveal()); + + $this->assertNull($provider->getUser()); + } + + public function testGetDomainReturnsNull(): void + { + $tokenStorage = $this->prophesize(TokenStorageInterface::class); + + $provider = new EcsUserProvider($tokenStorage->reveal()); + + $this->assertNull($provider->getDomain()); } } From 4b91b6a5736a762a603f3b62d412ad9d6c3acaa4 Mon Sep 17 00:00:00 2001 From: aubes <3941035+aubes@users.noreply.github.com> Date: Tue, 17 Mar 2026 23:06:12 +0100 Subject: [PATCH 2/3] fix: ensure AutoLabelProcessor runs last and validate labels type --- .../EcsLoggingExtension.php | 8 ++++---- src/Logger/AutoLabelProcessor.php | 20 +++++++++++++++++-- tests/Logger/AutoLabelProcessorTest.php | 13 ++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/DependencyInjection/EcsLoggingExtension.php b/src/DependencyInjection/EcsLoggingExtension.php index fb74dd0..2140bb9 100644 --- a/src/DependencyInjection/EcsLoggingExtension.php +++ b/src/DependencyInjection/EcsLoggingExtension.php @@ -56,7 +56,7 @@ private function configureAutoLabelProcessor(array $config, ContainerBuilder $co $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); } @@ -168,16 +168,16 @@ private function configureUserProcessor(array $config, ContainerBuilder $contain $container->setDefinition('.ecs_logging.processor.user', $processor); } - private 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]); } } } diff --git a/src/Logger/AutoLabelProcessor.php b/src/Logger/AutoLabelProcessor.php index b4a6f98..3583648 100644 --- a/src/Logger/AutoLabelProcessor.php +++ b/src/Logger/AutoLabelProcessor.php @@ -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); } } diff --git a/tests/Logger/AutoLabelProcessorTest.php b/tests/Logger/AutoLabelProcessorTest.php index 1a7e651..7f6fe04 100644 --- a/tests/Logger/AutoLabelProcessorTest.php +++ b/tests/Logger/AutoLabelProcessorTest.php @@ -110,6 +110,19 @@ public function testFieldsBundleConstant(): void $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 { $processor = new AutoLabelProcessor(['labels']); From 4bbe29c4384484722829c2a4b8df9a5d589b958e Mon Sep 17 00:00:00 2001 From: aubes <3941035+aubes@users.noreply.github.com> Date: Tue, 17 Mar 2026 23:45:58 +0100 Subject: [PATCH 3/3] Fix configuration error --- composer.json | 3 ++- src/DependencyInjection/Configuration.php | 1 + tests/Logger/AutoLabelProcessorTest.php | 3 --- tests/Logger/ErrorProcessorTest.php | 3 --- tests/Logger/TracingProcessorTest.php | 15 ++++++--------- tests/Logger/UserProcessorTest.php | 10 +++------- 6 files changed, 12 insertions(+), 23 deletions(-) diff --git a/composer.json b/composer.json index 855f9d8..e768828 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,8 @@ "elastic/ecs-logging": "^2.0.0", "monolog/monolog": "^3.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", diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 7d8be29..f993133 100644 --- a/src/DependencyInjection/Configuration.php +++ b/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') diff --git a/tests/Logger/AutoLabelProcessorTest.php b/tests/Logger/AutoLabelProcessorTest.php index 7f6fe04..a78e84d 100644 --- a/tests/Logger/AutoLabelProcessorTest.php +++ b/tests/Logger/AutoLabelProcessorTest.php @@ -8,12 +8,9 @@ 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( diff --git a/tests/Logger/ErrorProcessorTest.php b/tests/Logger/ErrorProcessorTest.php index d0edf74..ba60381 100644 --- a/tests/Logger/ErrorProcessorTest.php +++ b/tests/Logger/ErrorProcessorTest.php @@ -9,12 +9,9 @@ use Monolog\Level; use Monolog\LogRecord; use PHPUnit\Framework\TestCase; -use Prophecy\PhpUnit\ProphecyTrait; class ErrorProcessorTest extends TestCase { - use ProphecyTrait; - public function testWithErrorProcessor() { $processor = new ErrorProcessor('error'); diff --git a/tests/Logger/TracingProcessorTest.php b/tests/Logger/TracingProcessorTest.php index f04f9fe..0414626 100644 --- a/tests/Logger/TracingProcessorTest.php +++ b/tests/Logger/TracingProcessorTest.php @@ -9,13 +9,10 @@ use Monolog\Level; use Monolog\LogRecord; use PHPUnit\Framework\TestCase; -use Prophecy\PhpUnit\ProphecyTrait; class TracingProcessorTest extends TestCase { - use ProphecyTrait; - - public function testWithTracingProcessor() + public function testWithTracingProcessor(): void { $processor = new TracingProcessor('tracing'); @@ -38,7 +35,7 @@ public function testWithTracingProcessor() $this->assertInstanceOf(Tracing::class, $record->context['tracing']); } - public function testWithTracingRenameProcessor() + public function testWithTracingRenameProcessor(): void { $processor = new TracingProcessor('trace_custom'); @@ -62,7 +59,7 @@ public function testWithTracingRenameProcessor() $this->assertInstanceOf(Tracing::class, $record->context['tracing']); } - public function testWithoutTracingProcessor() + public function testWithoutTracingProcessor(): void { $processor = new TracingProcessor('tracing'); @@ -79,7 +76,7 @@ public function testWithoutTracingProcessor() $this->assertArrayNotHasKey('tracing', $record->context); } - public function testWithTracingWithoutTraceIdProcessor() + public function testWithTracingWithoutTraceIdProcessor(): void { $processor = new TracingProcessor('tracing'); @@ -101,7 +98,7 @@ public function testWithTracingWithoutTraceIdProcessor() $this->assertArrayNotHasKey('tracing', $record->context); } - public function testWithTracingWithoutTransactionIdProcessor() + public function testWithTracingWithoutTransactionIdProcessor(): void { $processor = new TracingProcessor('tracing'); @@ -123,7 +120,7 @@ public function testWithTracingWithoutTransactionIdProcessor() $this->assertInstanceOf(Tracing::class, $record->context['tracing']); } - public function testWithAlreadyTransformedTracingProcessor() + public function testWithAlreadyTransformedTracingProcessor(): void { $processor = new TracingProcessor('tracing'); diff --git a/tests/Logger/UserProcessorTest.php b/tests/Logger/UserProcessorTest.php index 0c9dd10..002a49a 100644 --- a/tests/Logger/UserProcessorTest.php +++ b/tests/Logger/UserProcessorTest.php @@ -16,7 +16,7 @@ class UserProcessorTest extends TestCase { use ProphecyTrait; - public function testWithUserProcessor() + public function testWithUserProcessor(): void { $user = $this->prophesize(User::class); @@ -40,7 +40,7 @@ public function testWithUserProcessor() $this->assertInstanceOf(User::class, $record->context['user']); } - public function testWithoutUserProcessor() + public function testWithoutUserProcessor(): void { $provider = $this->prophesize(EcsUserProviderInterface::class); $provider->getDomain()->willReturn('in_memory'); @@ -61,13 +61,9 @@ public function testWithoutUserProcessor() $this->assertArrayNotHasKey('user', $record->context); } - public function testUserDefinedProcessor() + public function testSkipsWhenUserAlreadyInContext(): void { - $user = $this->prophesize(User::class); - $provider = $this->prophesize(EcsUserProviderInterface::class); - $provider->getDomain()->willReturn('in_memory'); - $provider->getUser()->willReturn($user->reveal()); $processor = new UserProcessor($provider->reveal(), 'unknown');