Skip to content

Commit

Permalink
Allow console trace id be set via env vars (#27)
Browse files Browse the repository at this point in the history
  • Loading branch information
frankdekker authored Mar 15, 2024
1 parent 6561a17 commit 7d84ab3
Show file tree
Hide file tree
Showing 17 changed files with 207 additions and 49 deletions.
5 changes: 4 additions & 1 deletion docs/configuration/tracecontext.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ return static function (SymfonyTraceConfig $config): void {
$config->enableMonolog(true);

// Whether to add the request id to console commands, defaults to true
$config->enableConsole(true);
$config->console()->enabled(true);

// Optional, set console trace id based on env var. defaults to null
$config->console()->traceId(env('TRACE_ID'));

// Whether to add the request id to message bus events, defaults to false
$config->enableMessenger(false);
Expand Down
2 changes: 1 addition & 1 deletion phpstan-baseline.neon
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
parameters:
ignoreErrors:
-
message: "#^Parameter \\#1 \\$mergedConfig \\(array\\{traceMode\\: 'tracecontext'\\|'traceid', traceid\\: array\\{request_header\\: string, response_header\\: string, generator_service\\: string\\|null\\}, trust_request_header\\: bool, send_response_header\\: bool, storage_service\\: string\\|null, enable_monolog\\: bool, enable_console\\: bool, enable_messenger\\: bool, \\.\\.\\.\\}\\) of method DR\\\\SymfonyTraceBundle\\\\DependencyInjection\\\\SymfonyTraceExtension\\:\\:loadInternal\\(\\) should be contravariant with parameter \\$mergedConfig \\(array\\) of method Symfony\\\\Component\\\\HttpKernel\\\\DependencyInjection\\\\ConfigurableExtension\\:\\:loadInternal\\(\\)$#"
message: "#^Parameter \\#1 \\$mergedConfig \\(array\\{traceMode\\: 'tracecontext'\\|'traceid', traceid\\: array\\{request_header\\: string, response_header\\: string, generator_service\\: string\\|null\\}, trust_request_header\\: bool, send_response_header\\: bool, storage_service\\: string\\|null, enable_monolog\\: bool, enable_console\\: bool, console\\: array\\{enabled\\: bool, trace_id\\: string\\|null\\}, \\.\\.\\.\\}\\) of method DR\\\\SymfonyTraceBundle\\\\DependencyInjection\\\\SymfonyTraceExtension\\:\\:loadInternal\\(\\) should be contravariant with parameter \\$mergedConfig \\(array\\) of method Symfony\\\\Component\\\\HttpKernel\\\\DependencyInjection\\\\ConfigurableExtension\\:\\:loadInternal\\(\\)$#"
count: 1
path: src/DependencyInjection/SymfonyTraceExtension.php

Expand Down
79 changes: 55 additions & 24 deletions src/DependencyInjection/Configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Sentry\State\HubInterface;
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
use Symfony\Component\Config\Definition\ConfigurationInterface;

Expand All @@ -16,7 +17,7 @@
class Configuration implements ConfigurationInterface
{
public const TRACEMODE_TRACECONTEXT = 'tracecontext';
public const TRACEMODE_TRACEID = 'traceid';
public const TRACEMODE_TRACEID = 'traceid';

public function getConfigTreeBuilder(): TreeBuilder
{
Expand Down Expand Up @@ -69,7 +70,21 @@ public function getConfigTreeBuilder(): TreeBuilder
->end()
->booleanNode('enable_console')
->info('Whether to add the trace id to console commands, defaults to true')
->defaultTrue()
->defaultNull()
->setDeprecated('digitalrevolution/symfony-trace-bundle', '0.6.0')
->end()
->arrayNode('console')
->addDefaultsIfNotSet()
->children()
->booleanNode('enabled')
->info('Whether to add the trace id to console commands, defaults to true')
->defaultTrue()
->end()
->scalarNode('trace_id')
->info('Option to set the `Trace-Id` to use for console commands from env var. Defaults to null.')
->defaultNull()
->end()
->end()
->end()
->booleanNode('enable_messenger')
->info('Whether to add the trace id to message bus events, defaults to false')
Expand All @@ -79,37 +94,53 @@ public function getConfigTreeBuilder(): TreeBuilder
->info('Whether or not to enable the twig `trace_id()` and `transaction_id()` functions. Only works if TwigBundle is present.')
->defaultTrue()
->end()
->arrayNode('http_client')
->addDefaultsIfNotSet()
->children()
->booleanNode('enabled')
->info('Whether or not to enable the trace id aware http client')
->defaultTrue()
->end()
->booleanNode('tag_default_client')
->info('Whether or not to tag the default http client')
->defaultFalse()
->end()
->scalarNode('header')
->info('The header the bundle set in the request in the http client')
->defaultValue('X-Trace-Id')
->end()
->append($this->createHttpClientConfiguration())
->append($this->createSentryConfiguration())
;

return $tree;
}

private function createHttpClientConfiguration(): NodeDefinition
{
$node = (new TreeBuilder('http_client'))->getRootNode();
$node
->addDefaultsIfNotSet()
->children()
->booleanNode('enabled')
->info('Whether or not to enable the trace id aware http client')
->defaultTrue()
->end()
->booleanNode('tag_default_client')
->info('Whether or not to tag the default http client')
->defaultFalse()
->end()
->scalarNode('header')
->info('The header the bundle set in the request in the http client')
->defaultValue('X-Trace-Id')
->end()
->end()
->arrayNode('sentry')
->addDefaultsIfNotSet()
->children()
->booleanNode('enabled')
->end();

return $node;
}

private function createSentryConfiguration(): NodeDefinition
{
$node = (new TreeBuilder('sentry'))->getRootNode();
$node
->addDefaultsIfNotSet()
->children()
->booleanNode('enabled')
->info('Whether or not to enable passing trace and transaction id to Sentry')
->defaultFalse()
->end()
->scalarNode('hub_service')
->info('The service id of the Sentry Hub. Defaults to Sentry\State\HubInterface')
->defaultValue(HubInterface::class)
->end()
->end()
;
->end();

return $tree;
return $node;
}
}
15 changes: 13 additions & 2 deletions src/DependencyInjection/SymfonyTraceExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@
* storage_service: ?string,
* enable_monolog: bool,
* enable_console: bool,
* console: array{
* enabled: bool,
* trace_id: ?string
* },
* enable_messenger: bool,
* enable_twig: bool,
* http_client: array{
Expand Down Expand Up @@ -167,11 +171,18 @@ private function configureMonolog(array $mergedConfig, ContainerBuilder $contain
*/
private function configureConsole(array $mergedConfig, ContainerBuilder $container): void
{
if (class_exists(Application::class) === false || $mergedConfig['enable_console'] === false) {
$enabled = $mergedConfig['enable_console'] ?? $mergedConfig['console']['enabled'];
if (class_exists(Application::class) === false || $enabled === false) {
return;
}
$container->register(CommandSubscriber::class)
->setArguments([new Reference(TraceStorageInterface::class), new Reference(TraceServiceInterface::class)])
->setArguments(
[
new Reference(TraceStorageInterface::class),
new Reference(TraceServiceInterface::class),
$mergedConfig['console']['trace_id']
]
)
->setPublic(false)
->addTag('kernel.event_subscriber');
}
Expand Down
13 changes: 11 additions & 2 deletions src/EventSubscriber/CommandSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@
*/
final class CommandSubscriber implements EventSubscriberInterface
{
public function __construct(private readonly TraceStorageInterface $storage, private readonly TraceServiceInterface $service)
{
public function __construct(
private readonly TraceStorageInterface $storage,
private readonly TraceServiceInterface $service,
private readonly ?string $traceId,
) {
}

/**
Expand All @@ -33,6 +36,12 @@ public function onCommand(): void
return;
}

if ($this->traceId !== null && $this->traceId !== '') {
$this->storage->setTrace($this->service->createTraceFrom($this->traceId));

return;
}

$this->storage->setTrace($this->service->createNewTrace());
}
}
5 changes: 5 additions & 0 deletions src/Service/TraceContext/TraceContextParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@

class TraceContextParser
{
public static function isValid(string $traceParent): bool
{
return preg_match('/^[0-9a-f]{2}-[0-9a-f]{32}-[0-9a-f]{16}-[0-9a-f]{2}$/i', $traceParent) === 1;
}

public static function parseTraceContext(string $traceParent, string $traceState): TraceContext
{
$traceContext = self::parseTraceParent($traceParent);
Expand Down
23 changes: 16 additions & 7 deletions src/Service/TraceContext/TraceContextService.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ public function supports(Request $request): bool
return false;
}

$traceParent = $request->headers->get(self::HEADER_TRACEPARENT, '');

return preg_match('/^[0-9a-f]{2}-[0-9a-f]{32}-[0-9a-f]{16}-[0-9a-f]{2}$/i', $traceParent) === 1;
return TraceContextParser::isValid($request->headers->get(self::HEADER_TRACEPARENT, ''));
}

public function createNewTrace(): TraceContext
Expand All @@ -43,10 +41,22 @@ public function createNewTrace(): TraceContext
return $trace;
}

public function createTraceFrom(string $traceParent): TraceContext
{
if (TraceContextParser::isValid($traceParent) === false) {
return $this->createNewTrace();
}

$trace = TraceContextParser::parseTraceContext($traceParent, '');
$trace->setTransactionId($this->generator->generateTransactionId());

return $trace;
}

public function getRequestTrace(Request $request): TraceContext
{
$traceParent = $request->headers->get(self::HEADER_TRACEPARENT, '');
$traceState = $request->headers->get(self::HEADER_TRACESTATE, '');
$traceParent = $request->headers->get(self::HEADER_TRACEPARENT, '');
$traceState = $request->headers->get(self::HEADER_TRACESTATE, '');

$trace = TraceContextParser::parseTraceContext($traceParent, $traceState);
$trace->setTransactionId($this->generator->generateTransactionId());
Expand All @@ -66,8 +76,7 @@ public function handleResponse(Response $response, TraceContext $context): void

public function handleClientRequest(TraceContext $trace, string $method, string $url, array $options = []): array
{
$traceParent = $this->renderTraceParent($trace);
$options['headers'][self::HEADER_TRACEPARENT] = $traceParent;
$options['headers'][self::HEADER_TRACEPARENT] = $this->renderTraceParent($trace);

$traceState = $this->renderTraceState($trace);
if ($traceState !== '') {
Expand Down
9 changes: 9 additions & 0 deletions src/Service/TraceId/TraceIdService.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ public function createNewTrace(): TraceContext
return $trace;
}

public function createTraceFrom(string $traceId): TraceContext
{
$trace = new TraceContext();
$trace->setTraceId($traceId);
$trace->setTransactionId($this->generator->generateTransactionId());

return $trace;
}

public function getRequestTrace(Request $request): TraceContext
{
$trace = new TraceContext();
Expand Down
4 changes: 4 additions & 0 deletions src/Service/TraceServiceInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,13 @@
interface TraceServiceInterface
{
public function supports(Request $request): bool;

public function createNewTrace(): TraceContext;

public function createTraceFrom(string $traceId): TraceContext;

public function getRequestTrace(Request $request): TraceContext;

public function handleResponse(Response $response, TraceContext $context): void;

public function handleClientRequest(TraceContext $trace, string $method, string $url, array $options = []): array;
Expand Down
1 change: 1 addition & 0 deletions tests/Functional/App/config/config.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
parameters:
secret: dev
locale: en
env(TRACE_ID): null

framework:
test: ~
Expand Down
3 changes: 3 additions & 0 deletions tests/Functional/App/config/traceid.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ symfony_trace:
trust_request_header: true
storage_service: request.id.storage
enable_messenger: true
console:
enabled: true
trace_id: '%env(TRACE_ID)%'
http_client:
enabled: true
header: Trace-Id
Expand Down
23 changes: 20 additions & 3 deletions tests/Functional/ApplicationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
#[CoversNothing]
class ApplicationTest extends AbstractKernelTestCase
{
protected function tearDown(): void
{
parent::tearDown();
putenv('TRACE_ID=');
}

/**
* @throws Exception
*/
Expand All @@ -26,9 +32,7 @@ class ApplicationTest extends AbstractKernelTestCase
#[TestWith([Configuration::TRACEMODE_TRACECONTEXT, 'request.id.storage'])]
public function testCommandShouldSetTrace(string $traceMode, string $storageServiceId): void
{
$application = new Application(
static::bootKernel(['environment' => 'test', 'debug' => false, 'tracemode' => $traceMode])
);
$application = new Application(static::bootKernel(['environment' => 'test', 'debug' => false, 'tracemode' => $traceMode]));

$storage = self::getContainer()->get($storageServiceId);
static::assertInstanceOf(TraceStorageInterface::class, $storage);
Expand All @@ -41,4 +45,17 @@ public function testCommandShouldSetTrace(string $traceMode, string $storageServ
static::assertNotNull($storage->getTraceId());
static::assertNotNull($storage->getTransactionId());
}

public function testCommandShouldTakeTraceIdFromEnv(): void
{
putenv('TRACE_ID=test-trace-id');

$kernel = static::bootKernel(['environment' => 'test', 'debug' => false, 'tracemode' => Configuration::TRACEMODE_TRACEID]);

$storage = self::getContainer()->get('request.id.storage');
static::assertInstanceOf(TraceStorageInterface::class, $storage);

static::assertSame(Command::SUCCESS, (new Application($kernel))->doRun(new ArrayInput(['help']), new NullOutput()));
static::assertSame('test-trace-id', $storage->getTraceId());
}
}
15 changes: 13 additions & 2 deletions tests/Unit/EventSubscriber/CommandSubscriberTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ protected function setUp(): void

public function testOnCommandTrace(): void
{
$subscriber = new CommandSubscriber($this->storage, $this->service);
$subscriber = new CommandSubscriber($this->storage, $this->service, null);

$trace = new TraceContext();
$trace->setTraceId('trace-id');
Expand All @@ -39,14 +39,25 @@ public function testOnCommandTrace(): void

public function testOnCommandStorageHasTrace(): void
{
$subscriber = new CommandSubscriber($this->storage, $this->service);
$subscriber = new CommandSubscriber($this->storage, $this->service, null);

$this->service->expects(static::never())->method('createNewTrace');
$this->storage->expects(static::once())->method('getTraceId')->willReturn("abc123");

$subscriber->onCommand();
}

public function testOnCommandPresetTraceId(): void
{
$subscriber = new CommandSubscriber($this->storage, $this->service, 'test-trace-id');

$this->storage->expects(static::once())->method('getTraceId')->willReturn(null);
$this->service->expects(static::once())->method('createTraceFrom')->with('test-trace-id');
$this->storage->expects(static::once())->method('setTrace');

$subscriber->onCommand();
}

public function testGetSubscribedEvents(): void
{
static::assertSame([ConsoleEvents::COMMAND => ['onCommand', 999]], CommandSubscriber::getSubscribedEvents());
Expand Down
6 changes: 6 additions & 0 deletions tests/Unit/Service/TraceContext/TraceContextParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
#[CoversClass(TraceContextParser::class)]
class TraceContextParserTest extends TestCase
{
public function testIsValid(): void
{
static::assertTrue(TraceContextParser::isValid('00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-00'));
static::assertFalse(TraceContextParser::isValid('invalid'));
}

public function testParseTraceContext(): void
{
$traceContext = TraceContextParser::parseTraceContext(
Expand Down
Loading

0 comments on commit 7d84ab3

Please # to comment.