diff --git a/CHANGELOG.md b/CHANGELOG.md index a53c511..7f6938e 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## 2.3.0 +### Added +- Added sharing correlation ID between services. + ## 2.2.0 ### Added - Added Symfony ^6 support. diff --git a/README.md b/README.md index 668d234..3e2a8d3 100755 --- a/README.md +++ b/README.md @@ -17,7 +17,8 @@ the following features: - adds correlation_id to correlate messages in Sentry with messages in Graylog from the same process; - allows grouping some exceptions by their class, independently from where they were thrown at or what are their message; - removes root prefix from messages (usually included in some exception messages); -- maps context to be available with logged sentry event. +- maps context to be available with logged sentry event. +- allows sharing the same correlation_id across multiple services via an HTTP header Also recommended configuration is given to allow nice synergy between Graylog and Sentry. @@ -96,7 +97,15 @@ sentry: send_attempts: 1 paysera_logging_extra: - application_name: app-something # customise this to know which project message was sent from + application_name: app-something # customize this to know which project message was sent from + fetch_correlation_id_from_request: true # try to fetch correlation id from request header. false by default + +# Enable sharing correlation ID between requests if needed +Paysera\LoggingExtraBundle\Service\CorrelationIdHttpClientDecorator: + decorates: http_client + arguments: + - '@paysera_logging_extra.correlation_id_provider' + - '@.inner' ``` ## Usage @@ -145,7 +154,7 @@ docker-compose up -d docker-compose exec sentry sentry upgrade ``` -You'll find Graylog at [http://localhost:9001/](http://localhost:9001/) and Sentry at +You'll find Graylog at [http://localhost:9001/](http://localhost:9001/) and Sentry at [http://localhost:9002/](http://localhost:9002/). Open Graylog, login with `admin` `admin`, choose `System` -> `Inputs` -> `GELF UDP` -> `Launch new input` -> @@ -166,7 +175,7 @@ php test.php ``` View logged data in Graylog and Sentry instances. Change the code for further -test scenarios or just use Graylog and Sentry to set-up and test your real +test scenarios or just use Graylog and Sentry to set-up and test your real project. Cleanup afterwards: diff --git a/composer.json b/composer.json index a5c5052..adbea65 100755 --- a/composer.json +++ b/composer.json @@ -26,6 +26,7 @@ "symfony/dependency-injection": "^3.4|^4.0|^5.0|^6.0", "symfony/expression-language": "^3.0 || ^4.0 || ^5.0 || ^6.0", "symfony/framework-bundle": "^3.4.26|^4.2.7|^5.0|^6.0", + "symfony/http-client-contracts": "^3.5", "symfony/http-kernel": "^3.4|^4.0|^5.0|^6.0", "symfony/monolog-bundle": "^3.4|^5.0" }, diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index d679554..589b172 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -22,6 +22,7 @@ public function getConfigTreeBuilder() $children = $rootNode->children(); $children->scalarNode('application_name')->isRequired(); $children->arrayNode('grouped_exceptions')->prototype('scalar'); + $children->booleanNode('fetch_correlation_id_from_request')->defaultFalse(); return $treeBuilder; } diff --git a/src/DependencyInjection/PayseraLoggingExtraExtension.php b/src/DependencyInjection/PayseraLoggingExtraExtension.php index aa3e9c1..f741284 100644 --- a/src/DependencyInjection/PayseraLoggingExtraExtension.php +++ b/src/DependencyInjection/PayseraLoggingExtraExtension.php @@ -21,5 +21,6 @@ public function load(array $configs, ContainerBuilder $container) $container->setParameter('paysera_logging_extra.application_name', $config['application_name']); $container->setParameter('paysera_logging_extra.grouped_exceptions', $config['grouped_exceptions']); + $container->setParameter('paysera_logging_extra.fetch_correlation_id_from_request', $config['fetch_correlation_id_from_request']); } } diff --git a/src/Listener/CorrelationIdListener.php b/src/Listener/CorrelationIdListener.php index 55c2077..07b6a1e 100644 --- a/src/Listener/CorrelationIdListener.php +++ b/src/Listener/CorrelationIdListener.php @@ -4,7 +4,7 @@ namespace Paysera\LoggingExtraBundle\Listener; -use Paysera\LoggingExtraBundle\Service\CorrelationIdProvider; +use Paysera\LoggingExtraBundle\Service\CorrelationIdProvider\CorrelationIdProvider; use Symfony\Component\HttpKernel\Event\FilterResponseEvent as LegacyResponseEvent; use Symfony\Component\HttpKernel\Event\ResponseEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; diff --git a/src/Listener/IterationEndListener.php b/src/Listener/IterationEndListener.php index dc55473..4c3e678 100644 --- a/src/Listener/IterationEndListener.php +++ b/src/Listener/IterationEndListener.php @@ -4,14 +4,11 @@ namespace Paysera\LoggingExtraBundle\Listener; -use Paysera\LoggingExtraBundle\Service\CorrelationIdProvider; +use Paysera\LoggingExtraBundle\Service\CorrelationIdProvider\CorrelationIdProvider; use Sentry\ClientInterface; /** * Intended for cases where the same process is reused for separate job or request processing, like in PHPPM. - * - * Changes correlation_id while still maintaining same prefix to be able to find relations in cases of bugs happening - * due to shared state between different processing cycles */ class IterationEndListener { @@ -28,7 +25,7 @@ public function __construct( public function afterIteration() { - $this->correlationIdProvider->incrementIdentifier(); + $this->correlationIdProvider->reset(); if ($this->sentryClient !== null) { $this->sentryClient->flush(); } diff --git a/src/Resources/config/services.xml b/src/Resources/config/services.xml index 1d3035e..5be5bd1 100644 --- a/src/Resources/config/services.xml +++ b/src/Resources/config/services.xml @@ -9,11 +9,6 @@ - - %paysera_logging_extra.application_name% - - diff --git a/src/Resources/config/services/correlation_id_provider.xml b/src/Resources/config/services/correlation_id_provider.xml new file mode 100644 index 0000000..6b50f35 --- /dev/null +++ b/src/Resources/config/services/correlation_id_provider.xml @@ -0,0 +1,26 @@ + + + + + + + %paysera_logging_extra.application_name% + + + + + + + + + + + %paysera_logging_extra.fetch_correlation_id_from_request% + + + diff --git a/src/Service/CorrelationIdHttpClientDecorator.php b/src/Service/CorrelationIdHttpClientDecorator.php new file mode 100644 index 0000000..0e8a6fe --- /dev/null +++ b/src/Service/CorrelationIdHttpClientDecorator.php @@ -0,0 +1,35 @@ +client = $client ?? HttpClient::create(); + $this->correlationIdProvider = $correlationIdProvider; + } + + public function request(string $method, string $url, array $options = []): ResponseInterface + { + $options['headers'] = $options['headers'] ?? []; + $options['headers'][CorrelationIdListener::HEADER_NAME] = $this->correlationIdProvider->getCorrelationId(); + + return $this->client->request($method, $url, $options); + } +} diff --git a/src/Service/CorrelationIdProvider.php b/src/Service/CorrelationIdProvider.php deleted file mode 100644 index e538d2c..0000000 --- a/src/Service/CorrelationIdProvider.php +++ /dev/null @@ -1,31 +0,0 @@ -correlationId = uniqid($systemName, true); - $this->increment = 0; - } - - public function getCorrelationId(): string - { - if ($this->increment === 0) { - return $this->correlationId; - } - - return sprintf('%s_%s', $this->correlationId, $this->increment); - } - - public function incrementIdentifier() - { - $this->increment++; - } -} diff --git a/src/Service/CorrelationIdProvider/CorrelationIdProvider.php b/src/Service/CorrelationIdProvider/CorrelationIdProvider.php new file mode 100644 index 0000000..3572af4 --- /dev/null +++ b/src/Service/CorrelationIdProvider/CorrelationIdProvider.php @@ -0,0 +1,54 @@ +generatedCorrelationIdProvider = $generatedCorrelationIdProvider; + $this->requestHeaderCorrelationIdProvider = $requestHeaderCorrelationIdProvider; + $this->fetchFromRequest = $fetchFromRequest; + $this->isFetchedFromRequest = false; + } + + public function getCorrelationId(): string + { + if ($this->fetchFromRequest) { + $correlationIdFromRequest = $this->requestHeaderCorrelationIdProvider->getCorrelationId(); + + if ($correlationIdFromRequest !== null) { + $this->isFetchedFromRequest = true; + + return $correlationIdFromRequest; + } + } + + return $this->generatedCorrelationIdProvider->getCorrelationId(); + } + + public function reset(): void + { + if ($this->isFetchedFromRequest) { + $this->requestHeaderCorrelationIdProvider->reset(); + $this->isFetchedFromRequest = false; + + return; + } + + $this->generatedCorrelationIdProvider->increment(); + } +} diff --git a/src/Service/CorrelationIdProvider/GeneratedCorrelationIdProvider.php b/src/Service/CorrelationIdProvider/GeneratedCorrelationIdProvider.php new file mode 100644 index 0000000..564793b --- /dev/null +++ b/src/Service/CorrelationIdProvider/GeneratedCorrelationIdProvider.php @@ -0,0 +1,50 @@ +systemName = $systemName; + $this->correlationId = null; + $this->increment = 0; + $this->correlationId = uniqid($this->systemName, true); + } + + public function getCorrelationId(): string + { + if ($this->increment === 0) { + return $this->correlationId; + } + + return $this->buildIncrementedCorrelationId(); + } + + private function buildIncrementedCorrelationId(): string + { + return sprintf('%s_%s', $this->correlationId, $this->increment); + } + + /** + * Changes correlation_id while still maintaining same prefix to be able to find relations in + * cases of bugs happening due to shared state between different processing cycles + */ + public function increment(): void + { + if ($this->increment === PHP_INT_MAX) { + $this->increment = 0; + } + + $this->increment++; + } +} diff --git a/src/Service/CorrelationIdProvider/RequestHeaderCorrelationIdProvider.php b/src/Service/CorrelationIdProvider/RequestHeaderCorrelationIdProvider.php new file mode 100644 index 0000000..98a821e --- /dev/null +++ b/src/Service/CorrelationIdProvider/RequestHeaderCorrelationIdProvider.php @@ -0,0 +1,44 @@ +requestStack = $requestStack; + $this->correlationId = null; + } + + public function getCorrelationId(): ?string + { + if ($this->correlationId !== null) { + return $this->correlationId; + } + + $request = $this->requestStack->getCurrentRequest(); + if ($request === null) { + return null; + } + + $this->correlationId = $request->headers->get(CorrelationIdListener::HEADER_NAME); + + return $this->correlationId; + } + + public function reset(): void + { + $this->correlationId = null; + } +} diff --git a/src/Service/Processor/CorrelationIdProcessor.php b/src/Service/Processor/CorrelationIdProcessor.php index 5541a63..5f09298 100644 --- a/src/Service/Processor/CorrelationIdProcessor.php +++ b/src/Service/Processor/CorrelationIdProcessor.php @@ -5,7 +5,7 @@ namespace Paysera\LoggingExtraBundle\Service\Processor; use Monolog\Processor\ProcessorInterface; -use Paysera\LoggingExtraBundle\Service\CorrelationIdProvider; +use Paysera\LoggingExtraBundle\Service\CorrelationIdProvider\CorrelationIdProvider; class CorrelationIdProcessor implements ProcessorInterface { diff --git a/tests/Functional/FunctionalCorrelationIdListenerTest.php b/tests/Functional/FunctionalCorrelationIdListenerTest.php index fa5bece..8f1cb68 100644 --- a/tests/Functional/FunctionalCorrelationIdListenerTest.php +++ b/tests/Functional/FunctionalCorrelationIdListenerTest.php @@ -5,7 +5,7 @@ namespace Paysera\LoggingExtraBundle\Tests\Functional; use Paysera\LoggingExtraBundle\Listener\CorrelationIdListener; -use Paysera\LoggingExtraBundle\Service\CorrelationIdProvider; +use Paysera\LoggingExtraBundle\Service\CorrelationIdProvider\CorrelationIdProvider; class FunctionalCorrelationIdListenerTest extends FunctionalTestCase { diff --git a/tests/Unit/Service/CorrelationIdHttpClientDecoratorTest.php b/tests/Unit/Service/CorrelationIdHttpClientDecoratorTest.php new file mode 100644 index 0000000..c9ea850 --- /dev/null +++ b/tests/Unit/Service/CorrelationIdHttpClientDecoratorTest.php @@ -0,0 +1,73 @@ +httpClient = $this->createMock(HttpClientInterface::class); + + $correlationIdProvider = $this->createMock(CorrelationIdProvider::class); + $correlationIdProvider->method('getCorrelationId')->willReturn('mock-correlation-id'); + + $this->decorator = new CorrelationIdHttpClientDecorator($correlationIdProvider, $this->httpClient); + } + + public function testSendRequestAddsCustomHeader(): void + { + $url = 'https://api.paysera.com'; + $options = ['headers' => ['Authorization' => 'Bearer mock-token']]; + + $this->httpClient + ->expects($this->once()) + ->method('request') + ->with( + $this->equalTo('GET'), + $this->equalTo($url), + $this->equalTo( + [ + 'headers' => [ + 'Authorization' => 'Bearer mock-token', + 'Paysera-Correlation-Id' => 'mock-correlation-id', + ], + ] + ) + ) + ->willReturn($this->createMock(ResponseInterface::class)); + + $this->decorator->request('GET', $url, $options); + } + + public function testSendRequestAddsCustomHeaderIfHeadersNotExists(): void + { + $url = 'https://api.paysera.com'; + + $this->httpClient + ->expects($this->once()) + ->method('request') + ->with( + $this->equalTo('GET'), + $this->equalTo($url), + $this->equalTo( + ['headers' => ['Paysera-Correlation-Id' => 'mock-correlation-id']] + ) + ) + ->willReturn($this->createMock(ResponseInterface::class)); + + $this->decorator->request('GET', $url, []); + } +} diff --git a/tests/Unit/Service/CorrelationIdProvider/CorrelationIdProviderTest.php b/tests/Unit/Service/CorrelationIdProvider/CorrelationIdProviderTest.php new file mode 100644 index 0000000..6feab88 --- /dev/null +++ b/tests/Unit/Service/CorrelationIdProvider/CorrelationIdProviderTest.php @@ -0,0 +1,183 @@ +defaultGeneratedId = 'generated-correlation-id'; + + $this->generatedProvider = $this->createMock(GeneratedCorrelationIdProvider::class); + $this->generatedProvider + ->method('getCorrelationId') + ->willReturn($this->defaultGeneratedId); + + $this->requestHeaderProvider = $this->createMock(RequestHeaderCorrelationIdProvider::class); + } + + public function testReturnsCorrelationIdFromRequestWhenAvailable(): void + { + // Arrange + $requestCorrelationId = 'request-correlation-id'; + $this->requestHeaderProvider + ->method('getCorrelationId') + ->willReturn($requestCorrelationId); + + $correlationIdProvider = new CorrelationIdProvider( + $this->generatedProvider, + $this->requestHeaderProvider, + true + ); + + // Act + $actualCorrelationId = $correlationIdProvider->getCorrelationId(); + + // Assert + $this->assertEquals($requestCorrelationId, $actualCorrelationId); + } + + public function testFallsBackToGeneratedIdWhenNoCorrelationIdInRequest(): void + { + // Arrange + $this->requestHeaderProvider + ->method('getCorrelationId') + ->willReturn(null); + + $correlationIdProvider = new CorrelationIdProvider( + $this->generatedProvider, + $this->requestHeaderProvider, + true + ); + + // Act + $actualCorrelationId = $correlationIdProvider->getCorrelationId(); + + // Assert + $this->assertEquals($this->defaultGeneratedId, $actualCorrelationId); + } + + public function testAlwaysUsesGeneratedIdWhenFetchFromRequestIsFalse(): void + { + // Arrange + $requestCorrelationId = 'request-correlation-id'; + $this->requestHeaderProvider + ->method('getCorrelationId') + ->willReturn($requestCorrelationId); + + $correlationIdProvider = new CorrelationIdProvider( + $this->generatedProvider, + $this->requestHeaderProvider, + false + ); + + // Act + $actualCorrelationId = $correlationIdProvider->getCorrelationId(); + + // Assert + $this->assertEquals($this->defaultGeneratedId, $actualCorrelationId); + } + + public function testResetClearsRequestProviderWhenIdWasFetchedFromRequest(): void + { + // Arrange + $requestCorrelationId = 'request-correlation-id'; + $this->requestHeaderProvider + ->method('getCorrelationId') + ->willReturn($requestCorrelationId); + + $this->requestHeaderProvider + ->expects($this->once()) + ->method('reset'); + + $this->generatedProvider + ->expects($this->never()) + ->method('increment'); + + $correlationIdProvider = new CorrelationIdProvider( + $this->generatedProvider, + $this->requestHeaderProvider, + true + ); + + // First get the correlation ID to set isFetchedFromRequest = true + $correlationIdProvider->getCorrelationId(); + + // Act + $correlationIdProvider->reset(); + } + + public function testResetIncrementsGeneratedProviderWhenIdWasNotFetchedFromRequest(): void + { + // Arrange + $this->requestHeaderProvider + ->expects($this->never()) + ->method('reset'); + + $this->generatedProvider + ->expects($this->once()) + ->method('increment'); + + $correlationIdProvider = new CorrelationIdProvider( + $this->generatedProvider, + $this->requestHeaderProvider, + false + ); + + // Act + $correlationIdProvider->reset(); + } + + public function testResetIncrementsGeneratedProviderWhenNoCorrelationIdInRequest(): void + { + // Arrange + $this->requestHeaderProvider + ->method('getCorrelationId') + ->willReturn(null); + + $this->requestHeaderProvider + ->expects($this->never()) + ->method('reset'); + + $this->generatedProvider + ->expects($this->once()) + ->method('increment'); + + $correlationIdProvider = new CorrelationIdProvider( + $this->generatedProvider, + $this->requestHeaderProvider, + true + ); + + // First get the correlation ID, but it will use generated since request returns null + $correlationIdProvider->getCorrelationId(); + + // Act + $correlationIdProvider->reset(); + } +} diff --git a/tests/Unit/Service/CorrelationIdProvider/GeneratedCorrelationIdProviderTest.php b/tests/Unit/Service/CorrelationIdProvider/GeneratedCorrelationIdProviderTest.php new file mode 100644 index 0000000..69af0e9 --- /dev/null +++ b/tests/Unit/Service/CorrelationIdProvider/GeneratedCorrelationIdProviderTest.php @@ -0,0 +1,97 @@ +systemName = 'test-system'; + $this->provider = new GeneratedCorrelationIdProvider($this->systemName); + } + + public function testGetCorrelationId(): void + { + // Act + $correlationId = $this->provider->getCorrelationId(); + + // Assert + $this->assertNotEmpty($correlationId); + $this->assertStringStartsWith($this->systemName, $correlationId); + } + + public function testGetCorrelationIdReturnsSameValueBeforeIncrement(): void + { + // Act + $firstCall = $this->provider->getCorrelationId(); + $secondCall = $this->provider->getCorrelationId(); + + // Assert + $this->assertSame($firstCall, $secondCall); + } + + public function testIncrementChangesCorrelationId(): void + { + // Arrange + $initialCorrelationId = $this->provider->getCorrelationId(); + + // Act + $this->provider->increment(); + $incrementedCorrelationId = $this->provider->getCorrelationId(); + + // Assert + $this->assertNotEquals($initialCorrelationId, $incrementedCorrelationId); + $this->assertStringStartsWith($initialCorrelationId . '_', $incrementedCorrelationId); + $this->assertEquals($initialCorrelationId . '_1', $incrementedCorrelationId); + } + + public function testMultipleIncrementsAppendIncrementCounter(): void + { + // Arrange + $initialCorrelationId = $this->provider->getCorrelationId(); + + // Act - increment multiple times + $this->provider->increment(); + $firstIncrementId = $this->provider->getCorrelationId(); + + $this->provider->increment(); + $secondIncrementId = $this->provider->getCorrelationId(); + + $this->provider->increment(); + $thirdIncrementId = $this->provider->getCorrelationId(); + + // Assert + $this->assertEquals($initialCorrelationId . '_1', $firstIncrementId); + $this->assertEquals($initialCorrelationId . '_2', $secondIncrementId); + $this->assertEquals($initialCorrelationId . '_3', $thirdIncrementId); + } + + public function testCorrelationIdFormatFollowsExpectedPattern(): void + { + // Act + $correlationId = $this->provider->getCorrelationId(); + + // Assert + // The format should be: systemName + uniqid (which includes a timestamp and microseconds) + $this->assertRegExp( + sprintf('/^test\-system[0-9a-f]{14}\.[0-9a-f]+$/', preg_quote($this->systemName, '/')), + $correlationId + ); + } +} diff --git a/tests/Unit/Service/CorrelationIdProvider/RequestHeaderCorrelationIdProviderTest.php b/tests/Unit/Service/CorrelationIdProvider/RequestHeaderCorrelationIdProviderTest.php new file mode 100644 index 0000000..74572b9 --- /dev/null +++ b/tests/Unit/Service/CorrelationIdProvider/RequestHeaderCorrelationIdProviderTest.php @@ -0,0 +1,106 @@ +requestStack = new RequestStack(); + $this->provider = new RequestHeaderCorrelationIdProvider($this->requestStack); + } + + public function testGetCorrelationIdFromRequestHeader(): void + { + // Arrange + $correlationId = 'test-correlation-id'; + $request = new Request(); + $request->headers->set(CorrelationIdListener::HEADER_NAME, $correlationId); + $this->requestStack->push($request); + + // Act + $result = $this->provider->getCorrelationId(); + + // Assert + $this->assertEquals($correlationId, $result); + } + + public function testGetCorrelationIdReturnsCachedValue(): void + { + // Arrange + $correlationId = 'test-correlation-id'; + $request = new Request(); + $request->headers->set(CorrelationIdListener::HEADER_NAME, $correlationId); + $this->requestStack->push($request); + + // First call to cache the value + $this->provider->getCorrelationId(); + + // Modify the request header + $request->headers->set(CorrelationIdListener::HEADER_NAME, 'modified-correlation-id'); + + // Act + $result = $this->provider->getCorrelationId(); + + // Assert - should return the cached value, not the modified one + $this->assertEquals($correlationId, $result); + } + + public function testResetClearsCorrelationIdCache(): void + { + // Arrange + $correlationId = 'test-correlation-id'; + $request = new Request(); + $request->headers->set(CorrelationIdListener::HEADER_NAME, $correlationId); + $this->requestStack->push($request); + + // Cache the initial value + $this->provider->getCorrelationId(); + + // Change the header value + $newCorrelationId = 'new-correlation-id'; + $request->headers->set(CorrelationIdListener::HEADER_NAME, $newCorrelationId); + + // Act + $this->provider->reset(); + $result = $this->provider->getCorrelationId(); + + // Assert + $this->assertEquals($newCorrelationId, $result); + } + + public function testGetCorrelationIdReturnsNullWhenNoRequest(): void + { + // No request in the stack + // Act + $result = $this->provider->getCorrelationId(); + + // Assert + $this->assertNull($result); + } + + public function testGetCorrelationIdReturnsNullWhenNoHeader(): void + { + // Arrange - request without the correlation ID header + $request = new Request(); + $this->requestStack->push($request); + + // Act + $result = $this->provider->getCorrelationId(); + + // Assert + $this->assertNull($result); + } +}