From fbfbfa1becc4ddaee2bfc4ec06e880b6c0769505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 8 Nov 2023 03:59:17 +0100 Subject: [PATCH 01/14] Init profiler --- config/services.php | 13 +- src/DataCollector/DriverEventSubscriber.php | 63 +++++++++ src/DataCollector/MongoDBDataCollector.php | 140 +++++++++++++++++++ src/DependencyInjection/MongoDBExtension.php | 5 + src/TraceableClient.php | 48 +++++++ templates/Collector/mongodb.html.twig | 118 ++++++++++++++++ templates/Collector/mongodb.svg | 3 + 7 files changed, 388 insertions(+), 2 deletions(-) create mode 100644 src/DataCollector/DriverEventSubscriber.php create mode 100644 src/DataCollector/MongoDBDataCollector.php create mode 100644 src/TraceableClient.php create mode 100644 templates/Collector/mongodb.html.twig create mode 100644 templates/Collector/mongodb.svg diff --git a/config/services.php b/config/services.php index db2c3f6..8228310 100644 --- a/config/services.php +++ b/config/services.php @@ -21,7 +21,8 @@ namespace Symfony\Component\DependencyInjection\Loader\Configurator; use MongoDB\Bundle\Command\DebugCommand; -use MongoDB\Client; +use MongoDB\Bundle\DataCollector\MongoDBDataCollector; +use MongoDB\Bundle\TraceableClient; return static function (ContainerConfigurator $container): void { // default configuration for services in *this* file @@ -38,9 +39,17 @@ ->tag('console.command'); $services - ->set('mongodb.prototype.client', Client::class) + ->set('mongodb.prototype.client', TraceableClient::class) ->arg('$uri', abstract_arg('Should be defined by pass')) ->arg('$uriOptions', abstract_arg('Should be defined by pass')) ->arg('$driverOptions', abstract_arg('Should be defined by pass')) ->tag('mongodb.client'); + + $services + ->set('data_collector.mongodb', MongoDBDataCollector::class) + ->tag('data_collector', [ + 'template' => '@MongoDB/Collector/mongodb.html.twig', + 'id' => 'mongodb', + 'priority' => 250, + ]); }; diff --git a/src/DataCollector/DriverEventSubscriber.php b/src/DataCollector/DriverEventSubscriber.php new file mode 100644 index 0000000..e53e254 --- /dev/null +++ b/src/DataCollector/DriverEventSubscriber.php @@ -0,0 +1,63 @@ + + */ + private array $events = []; + + public function subscribe(Client $client): void + { + $client->getManager()->addSubscriber($this); + } + + /** + * @return list + */ + public function getEvents(): array + { + return $this->events; + } + + public function commandFailed(CommandFailedEvent $event): void + { + $this->events[] = $event; + } + + public function commandStarted(CommandStartedEvent $event): void + { + $this->events[] = $event; + } + + public function commandSucceeded(CommandSucceededEvent $event): void + { + $this->events[] = $event; + } +} diff --git a/src/DataCollector/MongoDBDataCollector.php b/src/DataCollector/MongoDBDataCollector.php new file mode 100644 index 0000000..a3b4c14 --- /dev/null +++ b/src/DataCollector/MongoDBDataCollector.php @@ -0,0 +1,140 @@ + + */ + private array $clients = []; + + public function addClient(string $name, Client $client): void + { + $this->clients[$name] = $client; + } + + public function collect(Request $request, Response $response, ?\Throwable $exception = null): void + { + foreach ($this->clients as $name => $client) { + $totalTime = 0; + $requestCount = 0; + $errorCount = 0; + $requests = []; + + if ($client instanceof TraceableClient) { + foreach ($client->getEvents() as $event) { + $requestId = $event->getRequestId(); + + $eventData = [ + 'class' => $event::class, + 'commandName' => $event->getCommandName(), + 'server' => $event->getServer()->getInfo(), + 'client' => $name, + ]; + + if ($event instanceof CommandStartedEvent) { + $command = (array) $event->getCommand(); + unset($command['lsid'], $command['$clusterTime']); + + $requests[$requestId] = [ + 'client' => $name, + 'startedAt' => hrtime(true), + 'commandName' => $event->getCommandName(), + 'command' => $command, + // 'server' => $event->getServer()->getInfo(), + 'operationId' => $event->getOperationId(), + 'database' => $event->getDatabaseName(), + 'serviceId' => $event->getServiceId(), + ]; + ++$requestCount; + } elseif ($event instanceof CommandSucceededEvent) { + $requests[$requestId] += [ + // 'reply' => Document::fromPHP($event->getReply()), + 'duration' => $event->getDurationMicros(), + 'endedAt' => hrtime(true), + 'success' => true, + ]; + $totalTime += $event->getDurationMicros(); + } elseif ($event instanceof CommandFailedEvent) { + $requests[$requestId] += [ + // 'reply' => Document::fromPHP($event->getReply()), + 'duration' => $event->getDurationMicros(), + 'error' => $event->getError(), + 'success' => false, + ]; + $totalTime += $event->getDurationMicros(); + ++$errorCount; + } + } + } + + $this->data['clients'][$name] = [ + 'name' => $name, + 'uri' => (string) $client, + 'totalTime' => $totalTime, + 'requestCount' => $requestCount, + 'errorCount' => $errorCount, + 'requests' => $requests, + ]; + } + } + + public function getRequestCount(): int + { + return array_sum(array_column($this->data['clients'], 'requestCount')); + } + + public function getErrorCount(): int + { + return array_sum(array_column($this->data['clients'], 'errorCount')); + } + + public function getTime(): float + { + return array_sum(array_column($this->data['clients'], 'totalTime')); + } + + public function getClients(): array + { + return $this->data['clients']; + } + + public function getName(): string + { + return 'mongodb'; + } + + public function reset(): void + { + // TODO: Implement reset() method. + } +} diff --git a/src/DependencyInjection/MongoDBExtension.php b/src/DependencyInjection/MongoDBExtension.php index a8b06e7..42e76ba 100644 --- a/src/DependencyInjection/MongoDBExtension.php +++ b/src/DependencyInjection/MongoDBExtension.php @@ -24,8 +24,10 @@ use MongoDB\Client; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Extension\Extension; use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; +use Symfony\Component\DependencyInjection\Reference; use function dirname; use function sprintf; @@ -62,6 +64,7 @@ public static function createClientServiceId(string $clientId): string private function createClients(string $defaultClient, array $clients, ContainerBuilder $container): void { $clientPrototype = $container->getDefinition('mongodb.prototype.client'); + $dataCollector = $container->getDefinition('mongodb.data_collector'); foreach ($clients as $client => $configuration) { $serviceId = self::createClientServiceId($client); @@ -73,6 +76,8 @@ private function createClients(string $defaultClient, array $clients, ContainerB $container->setDefinition($serviceId, $clientDefinition); + $dataCollector->addMethodCall('addClient', [$client, new Reference($serviceId)]); + if (isset($configuration['default_database'])) { $container->setParameter(sprintf('%s.default_database', $serviceId), $configuration['default_database']); } diff --git a/src/TraceableClient.php b/src/TraceableClient.php new file mode 100644 index 0000000..8bbd374 --- /dev/null +++ b/src/TraceableClient.php @@ -0,0 +1,48 @@ +subscriber = new DriverEventSubscriber(); + $this->getManager()->addSubscriber($this->subscriber); + } + + /** + * @return list + */ + public function getEvents(): array + { + return $this->subscriber->getEvents(); + } +} diff --git a/templates/Collector/mongodb.html.twig b/templates/Collector/mongodb.html.twig new file mode 100644 index 0000000..9e4fe79 --- /dev/null +++ b/templates/Collector/mongodb.html.twig @@ -0,0 +1,118 @@ +{% extends request.isXmlHttpRequest ? '@WebProfiler/Profiler/ajax_layout.html.twig' : '@WebProfiler/Profiler/layout.html.twig' %} + +{% import _self as helper %} + +{% block toolbar %} + {% if collector.requestCount > 0 %} + + {% set icon %} + {{ include('@MongoDB/Collector/mongodb.svg') }} + + {{ collector.requestCount }} + + in + {{ '%0.2f'|format(collector.time / 1000) }} + ms + + {% endset %} + + {% set text %} +
+ MongoDB Requests + {{ collector.requestCount }} +
+
+ Total errors + {{ collector.errorCount }} +
+
+ Request time + {{ '%0.2f'|format(collector.time / 1000) }} ms +
+ {% endset %} + + + {{ include('@WebProfiler/Profiler/toolbar_item.html.twig', { link: profiler_url, status: status|default('') }) }} + {% endif %} +{% endblock %} + +{% block menu %} + + {{ include('@MongoDB/Collector/mongodb.svg') }} + MongoDB + {% if collector.requestCount %} + + {{ collector.requestCount }} + + {% endif %} + +{% endblock %} + +{% block panel %} + +

Request Metrics

+ +
+
+
+ {{ collector.requestCount }} + Requests +
+ +
+ {{ collector.errorCount }} + Errors +
+ +
+ {{ '%0.2f'|format(collector.time / 1000) }} ms + Request time +
+
+
+ + +
+
+
+ {% if not collector.clients %} +
+

No executed request.

+
+ {% else %} + {% for name, client in collector.clients %} + {% if collector.clients|length > 1 %} +

{{ name }} client

+ {% endif %} + + {% if client.requests is empty %} +
+

No database queries were performed.

+
+ {% else %} + + + + + + + + + + {% for i, request in client.requests %} + + + + + + {% endfor %} + +
#TimeInfo
{{ loop.index }}{{ '%0.2f'|format(request.duration / 1000) }} ms + 
{{ request.command|json_encode(constant('JSON_PRETTY_PRINT')) }}
+
+ {% endif %} + {% endfor %} + {% endif %} +
+
+{% endblock %} diff --git a/templates/Collector/mongodb.svg b/templates/Collector/mongodb.svg new file mode 100644 index 0000000..5346336 --- /dev/null +++ b/templates/Collector/mongodb.svg @@ -0,0 +1,3 @@ + + + From 6655af0e8a4fdc25e38d3dd5be50e1e78537d7c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 8 Nov 2023 22:25:09 +0100 Subject: [PATCH 02/14] Use a compiler pass to register data collector conditionally --- composer.json | 1 + config/services.php | 9 +- src/Client.php | 35 ++++++++ src/DataCollector/MongoDBDataCollector.php | 88 +++++++++---------- .../Compiler/DataCollectorPass.php | 52 +++++++++++ src/DependencyInjection/MongoDBExtension.php | 5 +- src/MongoDBBundle.php | 6 ++ src/TraceableClient.php | 48 ---------- .../MongoDBExtensionTest.php | 6 +- 9 files changed, 142 insertions(+), 108 deletions(-) create mode 100644 src/Client.php create mode 100644 src/DependencyInjection/Compiler/DataCollectorPass.php delete mode 100644 src/TraceableClient.php diff --git a/composer.json b/composer.json index 72f42e8..3d83d13 100644 --- a/composer.json +++ b/composer.json @@ -28,6 +28,7 @@ "symfony/framework-bundle": "^6.3.5 || ^7.0", "symfony/phpunit-bridge": "~6.3.10 || ^6.4.1 || ^7.0.1", "symfony/yaml": "^6.3 || ^7.0", + "symfony/web-profiler-bundle": "^6.3 || ^7.0", "zenstruck/browser": "^1.6" }, "scripts": { diff --git a/config/services.php b/config/services.php index 8228310..2a53e81 100644 --- a/config/services.php +++ b/config/services.php @@ -20,9 +20,9 @@ namespace Symfony\Component\DependencyInjection\Loader\Configurator; +use MongoDB\Bundle\Client; use MongoDB\Bundle\Command\DebugCommand; use MongoDB\Bundle\DataCollector\MongoDBDataCollector; -use MongoDB\Bundle\TraceableClient; return static function (ContainerConfigurator $container): void { // default configuration for services in *this* file @@ -39,14 +39,13 @@ ->tag('console.command'); $services - ->set('mongodb.prototype.client', TraceableClient::class) + ->set('mongodb.prototype.client', Client::class) ->arg('$uri', abstract_arg('Should be defined by pass')) ->arg('$uriOptions', abstract_arg('Should be defined by pass')) - ->arg('$driverOptions', abstract_arg('Should be defined by pass')) - ->tag('mongodb.client'); + ->arg('$driverOptions', abstract_arg('Should be defined by pass')); $services - ->set('data_collector.mongodb', MongoDBDataCollector::class) + ->set('mongodb.data_collector', MongoDBDataCollector::class) ->tag('data_collector', [ 'template' => '@MongoDB/Collector/mongodb.html.twig', 'id' => 'mongodb', diff --git a/src/Client.php b/src/Client.php new file mode 100644 index 0000000..1f3e2ce --- /dev/null +++ b/src/Client.php @@ -0,0 +1,35 @@ +getManager()->addSubscriber($subscriber); + } +} diff --git a/src/DataCollector/MongoDBDataCollector.php b/src/DataCollector/MongoDBDataCollector.php index a3b4c14..57a7180 100644 --- a/src/DataCollector/MongoDBDataCollector.php +++ b/src/DataCollector/MongoDBDataCollector.php @@ -20,9 +20,7 @@ namespace MongoDB\Bundle\DataCollector; -use MongoDB\BSON\Document; -use MongoDB\Bundle\TraceableClient; -use MongoDB\Client; +use MongoDB\Bundle\Client; use MongoDB\Driver\Monitoring\CommandFailedEvent; use MongoDB\Driver\Monitoring\CommandStartedEvent; use MongoDB\Driver\Monitoring\CommandSucceededEvent; @@ -33,67 +31,61 @@ final class MongoDBDataCollector extends DataCollector { /** - * @var list + * @var list */ private array $clients = []; - public function addClient(string $name, Client $client): void + public function addClient(string $name, Client $client, DriverEventSubscriber $subscriber): void { - $this->clients[$name] = $client; + $this->clients[$name] = [ + 'client' => $client, + 'subscriber' => $subscriber, + ]; } public function collect(Request $request, Response $response, ?\Throwable $exception = null): void { - foreach ($this->clients as $name => $client) { + foreach ($this->clients as $name => ['client' => $client, 'subscriber' => $subscriber]) { $totalTime = 0; $requestCount = 0; $errorCount = 0; $requests = []; - if ($client instanceof TraceableClient) { - foreach ($client->getEvents() as $event) { - $requestId = $event->getRequestId(); + foreach ($subscriber->getEvents() as $event) { + $requestId = $event->getRequestId(); - $eventData = [ - 'class' => $event::class, - 'commandName' => $event->getCommandName(), - 'server' => $event->getServer()->getInfo(), + if ($event instanceof CommandStartedEvent) { + $command = (array) $event->getCommand(); + unset($command['lsid'], $command['$clusterTime']); + + $requests[$requestId] = [ 'client' => $name, + 'startedAt' => hrtime(true), + 'commandName' => $event->getCommandName(), + 'command' => $command, + // 'server' => $event->getServer()->getInfo(), + 'operationId' => $event->getOperationId(), + 'database' => $event->getDatabaseName(), + 'serviceId' => $event->getServiceId(), ]; - - if ($event instanceof CommandStartedEvent) { - $command = (array) $event->getCommand(); - unset($command['lsid'], $command['$clusterTime']); - - $requests[$requestId] = [ - 'client' => $name, - 'startedAt' => hrtime(true), - 'commandName' => $event->getCommandName(), - 'command' => $command, - // 'server' => $event->getServer()->getInfo(), - 'operationId' => $event->getOperationId(), - 'database' => $event->getDatabaseName(), - 'serviceId' => $event->getServiceId(), - ]; - ++$requestCount; - } elseif ($event instanceof CommandSucceededEvent) { - $requests[$requestId] += [ - // 'reply' => Document::fromPHP($event->getReply()), - 'duration' => $event->getDurationMicros(), - 'endedAt' => hrtime(true), - 'success' => true, - ]; - $totalTime += $event->getDurationMicros(); - } elseif ($event instanceof CommandFailedEvent) { - $requests[$requestId] += [ - // 'reply' => Document::fromPHP($event->getReply()), - 'duration' => $event->getDurationMicros(), - 'error' => $event->getError(), - 'success' => false, - ]; - $totalTime += $event->getDurationMicros(); - ++$errorCount; - } + ++$requestCount; + } elseif ($event instanceof CommandSucceededEvent) { + $requests[$requestId] += [ + // 'reply' => Document::fromPHP($event->getReply()), + 'duration' => $event->getDurationMicros(), + 'endedAt' => hrtime(true), + 'success' => true, + ]; + $totalTime += $event->getDurationMicros(); + } elseif ($event instanceof CommandFailedEvent) { + $requests[$requestId] += [ + // 'reply' => Document::fromPHP($event->getReply()), + 'duration' => $event->getDurationMicros(), + 'error' => $event->getError(), + 'success' => false, + ]; + $totalTime += $event->getDurationMicros(); + ++$errorCount; } } diff --git a/src/DependencyInjection/Compiler/DataCollectorPass.php b/src/DependencyInjection/Compiler/DataCollectorPass.php new file mode 100644 index 0000000..9ca2c5f --- /dev/null +++ b/src/DependencyInjection/Compiler/DataCollectorPass.php @@ -0,0 +1,52 @@ +has('profiler')) { + return; + } + + $dataCollector = $container->getDefinition('mongodb.data_collector'); + + // Add a subscriber to each client to collect driver events, and register the client to the data collector. + foreach ($container->findTaggedServiceIds('mongodb.client', true) as $clientId => $attributes) { + $subscriberId = sprintf('%s.subscriber', $clientId); + $container->setDefinition($subscriberId, new Definition(DriverEventSubscriber::class)); + $container->getDefinition($clientId)->addMethodCall('addSubscriber', [new Reference($subscriberId)]); + $dataCollector->addMethodCall('addClient', [ + $attributes[0]['name'] ?? $clientId, + new Reference($clientId), + new Reference($subscriberId), + ]); + } + } +} diff --git a/src/DependencyInjection/MongoDBExtension.php b/src/DependencyInjection/MongoDBExtension.php index 42e76ba..3982e60 100644 --- a/src/DependencyInjection/MongoDBExtension.php +++ b/src/DependencyInjection/MongoDBExtension.php @@ -27,7 +27,6 @@ use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Extension\Extension; use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; -use Symfony\Component\DependencyInjection\Reference; use function dirname; use function sprintf; @@ -73,11 +72,9 @@ private function createClients(string $defaultClient, array $clients, ContainerB $clientDefinition->setArgument('$uri', $configuration['uri']); $clientDefinition->setArgument('$uriOptions', $configuration['uri_options'] ?? []); $clientDefinition->setArgument('$driverOptions', $configuration['driver_options'] ?? []); - + $clientDefinition->addTag('mongodb.client', ['name' => $client]); $container->setDefinition($serviceId, $clientDefinition); - $dataCollector->addMethodCall('addClient', [$client, new Reference($serviceId)]); - if (isset($configuration['default_database'])) { $container->setParameter(sprintf('%s.default_database', $serviceId), $configuration['default_database']); } diff --git a/src/MongoDBBundle.php b/src/MongoDBBundle.php index 127c1bc..5dac19a 100644 --- a/src/MongoDBBundle.php +++ b/src/MongoDBBundle.php @@ -21,6 +21,7 @@ namespace MongoDB\Bundle; use MongoDB\Bundle\DependencyInjection\MongoDBExtension; +use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Extension\ExtensionInterface; use Symfony\Component\HttpKernel\Bundle\AbstractBundle; @@ -30,4 +31,9 @@ public function getContainerExtension(): ?ExtensionInterface { return new MongoDBExtension(); } + + public function build(ContainerBuilder $container): void + { + $container->addCompilerPass(new DependencyInjection\Compiler\DataCollectorPass()); + } } diff --git a/src/TraceableClient.php b/src/TraceableClient.php deleted file mode 100644 index 8bbd374..0000000 --- a/src/TraceableClient.php +++ /dev/null @@ -1,48 +0,0 @@ -subscriber = new DriverEventSubscriber(); - $this->getManager()->addSubscriber($this->subscriber); - } - - /** - * @return list - */ - public function getEvents(): array - { - return $this->subscriber->getEvents(); - } -} diff --git a/tests/Unit/DependencyInjection/MongoDBExtensionTest.php b/tests/Unit/DependencyInjection/MongoDBExtensionTest.php index c0cb90f..20f5767 100644 --- a/tests/Unit/DependencyInjection/MongoDBExtensionTest.php +++ b/tests/Unit/DependencyInjection/MongoDBExtensionTest.php @@ -59,7 +59,7 @@ public function testLoadWithSingleClient(): void // Check service definition $definition = $container->getDefinition('mongodb.client.default'); - $this->assertSame(Client::class, $definition->getClass()); + $this->assertTrue(is_a($definition->getClass(), Client::class, true), sprintf('Expected "%s" to be a "%s"', $definition->getClass(), Client::class)); $this->assertSame('mongodb://localhost:27017', $definition->getArgument('$uri')); // Check alias definition @@ -93,12 +93,12 @@ public function testLoadWithMultipleClients(): void // Check service definitions $definition = $container->getDefinition('mongodb.client.default'); - $this->assertSame(Client::class, $definition->getClass()); + $this->assertTrue(is_a($definition->getClass(), Client::class, true), sprintf('Expected "%s" to be a "%s"', $definition->getClass(), Client::class)); $this->assertSame('mongodb://localhost:27017', $definition->getArgument('$uri')); $this->assertSame(['readPreference' => 'primary'], $definition->getArgument('$uriOptions')); $definition = $container->getDefinition('mongodb.client.secondary'); - $this->assertSame(Client::class, $definition->getClass()); + $this->assertTrue(is_a($definition->getClass(), Client::class, true), sprintf('Expected "%s" to be a "%s"', $definition->getClass(), Client::class)); $this->assertSame('mongodb://localhost:27018', $definition->getArgument('$uri')); $this->assertEquals(['serverApi' => new ServerApi((string) ServerApi::V1)], $definition->getArgument('$driverOptions')); } From c61cdad884d3fa40a06d084e205da11c7adec588 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 8 Nov 2023 23:10:28 +0100 Subject: [PATCH 03/14] Add tests on data collector services --- .../MongoDBExtensionTest.php | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/tests/Unit/DependencyInjection/MongoDBExtensionTest.php b/tests/Unit/DependencyInjection/MongoDBExtensionTest.php index 20f5767..cfd7d5c 100644 --- a/tests/Unit/DependencyInjection/MongoDBExtensionTest.php +++ b/tests/Unit/DependencyInjection/MongoDBExtensionTest.php @@ -21,11 +21,13 @@ namespace MongoDB\Bundle\Tests\Unit\DependencyInjection; use InvalidArgumentException; +use MongoDB\Bundle\DependencyInjection\Compiler\DataCollectorPass; use MongoDB\Bundle\DependencyInjection\MongoDBExtension; use MongoDB\Client; use MongoDB\Driver\ServerApi; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag; /** @covers \MongoDB\Bundle\DependencyInjection\MongoDBExtension */ @@ -50,7 +52,8 @@ public function testLoadWithSingleClient(): void 'clients' => [ ['id' => 'default', 'uri' => 'mongodb://localhost:27017'], ], - ], + ]], [ + 'profiler' => new Definition(\stdClass::class), ]); $this->assertTrue($container->hasDefinition('mongodb.client.default')); @@ -61,10 +64,33 @@ public function testLoadWithSingleClient(): void $definition = $container->getDefinition('mongodb.client.default'); $this->assertTrue(is_a($definition->getClass(), Client::class, true), sprintf('Expected "%s" to be a "%s"', $definition->getClass(), Client::class)); $this->assertSame('mongodb://localhost:27017', $definition->getArgument('$uri')); + $this->assertTrue($definition->hasMethodCall('addSubscriber')); // Check alias definition $alias = $container->getAlias(Client::class); $this->assertSame('mongodb.client.default', (string) $alias); + + // Check data collector + $definition = $container->getDefinition('mongodb.data_collector'); + $this->assertTrue($definition->hasMethodCall('addClient')); + $this->assertSame('default', $definition->getMethodCalls()[0][1][0]); + } + + public function testLoadWithoutProfiler(): void + { + $container = $this->getContainer([[ + 'clients' => [ + ['id' => 'default', 'uri' => 'mongodb://localhost:27017'], + ], + ]]); + + // Check service definition + $definition = $container->getDefinition('mongodb.client.default'); + $this->assertFalse($definition->hasMethodCall('addSubscriber')); + + // Check data collector + $definition = $container->getDefinition('mongodb.data_collector'); + $this->assertFalse($definition->hasMethodCall('addClient')); } public function testLoadWithMultipleClients(): void @@ -83,7 +109,8 @@ public function testLoadWithMultipleClients(): void 'driver_options' => ['serverApi' => new ServerApi((string) ServerApi::V1)], ], ], - ], + ]], [ + 'profiler' => new Definition(\stdClass::class), ]); $this->assertTrue($container->hasDefinition('mongodb.client.default')); @@ -96,11 +123,19 @@ public function testLoadWithMultipleClients(): void $this->assertTrue(is_a($definition->getClass(), Client::class, true), sprintf('Expected "%s" to be a "%s"', $definition->getClass(), Client::class)); $this->assertSame('mongodb://localhost:27017', $definition->getArgument('$uri')); $this->assertSame(['readPreference' => 'primary'], $definition->getArgument('$uriOptions')); + $this->assertTrue($definition->hasMethodCall('addSubscriber')); $definition = $container->getDefinition('mongodb.client.secondary'); $this->assertTrue(is_a($definition->getClass(), Client::class, true), sprintf('Expected "%s" to be a "%s"', $definition->getClass(), Client::class)); $this->assertSame('mongodb://localhost:27018', $definition->getArgument('$uri')); $this->assertEquals(['serverApi' => new ServerApi((string) ServerApi::V1)], $definition->getArgument('$driverOptions')); + $this->assertTrue($definition->hasMethodCall('addSubscriber')); + + // Check data collector + $definition = $container->getDefinition('mongodb.data_collector'); + $this->assertCount(2, $definition->getMethodCalls()); + $this->assertTrue($definition->hasMethodCall('addClient')); + $this->assertSame(['default', 'secondary'], array_column(array_column($definition->getMethodCalls(), 1), 0)); } private function getContainer(array $config = [], array $thirdPartyDefinitions = []): ContainerBuilder @@ -113,6 +148,7 @@ private function getContainer(array $config = [], array $thirdPartyDefinitions = $container->getCompilerPassConfig()->setOptimizationPasses([]); $container->getCompilerPassConfig()->setRemovingPasses([]); + $container->addCompilerPass(new DataCollectorPass()); $loader = new MongoDBExtension(); $loader->load($config, $container); From ade9e8764e55c53d516573c0eb48ca6fe61fc221 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 8 Nov 2023 23:21:29 +0100 Subject: [PATCH 04/14] Implement DataCollector::reset() --- src/DataCollector/DriverEventSubscriber.php | 9 ++++++++- src/DataCollector/MongoDBDataCollector.php | 10 ++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/DataCollector/DriverEventSubscriber.php b/src/DataCollector/DriverEventSubscriber.php index e53e254..e73492c 100644 --- a/src/DataCollector/DriverEventSubscriber.php +++ b/src/DataCollector/DriverEventSubscriber.php @@ -25,8 +25,10 @@ use MongoDB\Driver\Monitoring\CommandStartedEvent; use MongoDB\Driver\Monitoring\CommandSubscriber; use MongoDB\Driver\Monitoring\CommandSucceededEvent; +use Symfony\Contracts\Service\ResetInterface; -final class DriverEventSubscriber implements CommandSubscriber +/** @internal */ +final class DriverEventSubscriber implements CommandSubscriber, ResetInterface { /** * @var list @@ -60,4 +62,9 @@ public function commandSucceeded(CommandSucceededEvent $event): void { $this->events[] = $event; } + + public function reset(): void + { + $this->events = []; + } } diff --git a/src/DataCollector/MongoDBDataCollector.php b/src/DataCollector/MongoDBDataCollector.php index 57a7180..203a0fa 100644 --- a/src/DataCollector/MongoDBDataCollector.php +++ b/src/DataCollector/MongoDBDataCollector.php @@ -28,6 +28,7 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\DataCollector\DataCollector; +/** @internal */ final class MongoDBDataCollector extends DataCollector { /** @@ -63,7 +64,6 @@ public function collect(Request $request, Response $response, ?\Throwable $excep 'startedAt' => hrtime(true), 'commandName' => $event->getCommandName(), 'command' => $command, - // 'server' => $event->getServer()->getInfo(), 'operationId' => $event->getOperationId(), 'database' => $event->getDatabaseName(), 'serviceId' => $event->getServiceId(), @@ -71,7 +71,6 @@ public function collect(Request $request, Response $response, ?\Throwable $excep ++$requestCount; } elseif ($event instanceof CommandSucceededEvent) { $requests[$requestId] += [ - // 'reply' => Document::fromPHP($event->getReply()), 'duration' => $event->getDurationMicros(), 'endedAt' => hrtime(true), 'success' => true, @@ -79,7 +78,6 @@ public function collect(Request $request, Response $response, ?\Throwable $excep $totalTime += $event->getDurationMicros(); } elseif ($event instanceof CommandFailedEvent) { $requests[$requestId] += [ - // 'reply' => Document::fromPHP($event->getReply()), 'duration' => $event->getDurationMicros(), 'error' => $event->getError(), 'success' => false, @@ -127,6 +125,10 @@ public function getName(): string public function reset(): void { - // TODO: Implement reset() method. + $this->data = []; + + foreach ($this->clients as ['subscriber' => $subscriber]) { + $subscriber->reset(); + } } } From af0a0496ec703138ccf612ff165c5a1cd0b69753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 9 Nov 2023 00:06:44 +0100 Subject: [PATCH 05/14] Rework MongoDB logo --- templates/Collector/mongodb.svg | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/Collector/mongodb.svg b/templates/Collector/mongodb.svg index 5346336..83a00a9 100644 --- a/templates/Collector/mongodb.svg +++ b/templates/Collector/mongodb.svg @@ -1,3 +1,3 @@ - - + + From 3a453fa4b3d5eb6ebb5e375ef8fcddd23407f4b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Thu, 9 Nov 2023 00:46:46 +0100 Subject: [PATCH 06/14] Add stopwatch --- src/DataCollector/DriverEventSubscriber.php | 27 ++++++++++++++++--- .../Compiler/DataCollectorPass.php | 9 ++++++- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/DataCollector/DriverEventSubscriber.php b/src/DataCollector/DriverEventSubscriber.php index e73492c..4e33695 100644 --- a/src/DataCollector/DriverEventSubscriber.php +++ b/src/DataCollector/DriverEventSubscriber.php @@ -20,11 +20,11 @@ namespace MongoDB\Bundle\DataCollector; -use MongoDB\Client; use MongoDB\Driver\Monitoring\CommandFailedEvent; use MongoDB\Driver\Monitoring\CommandStartedEvent; use MongoDB\Driver\Monitoring\CommandSubscriber; use MongoDB\Driver\Monitoring\CommandSucceededEvent; +use Symfony\Component\Stopwatch\Stopwatch; use Symfony\Contracts\Service\ResetInterface; /** @internal */ @@ -34,10 +34,12 @@ final class DriverEventSubscriber implements CommandSubscriber, ResetInterface * @var list */ private array $events = []; + private array $stopwatchEvents = []; - public function subscribe(Client $client): void - { - $client->getManager()->addSubscriber($this); + public function __construct( + private string $clientName, + private ?Stopwatch $stopwatch = null, + ) { } /** @@ -51,16 +53,33 @@ public function getEvents(): array public function commandFailed(CommandFailedEvent $event): void { $this->events[] = $event; + + if (isset($this->stopwatchEvents[$event->getRequestId()])) { + $this->stopwatchEvents[$event->getRequestId()]->stop(); + unset($this->stopwatchEvents[$event->getRequestId()]); + } } public function commandStarted(CommandStartedEvent $event): void { $this->events[] = $event; + + if ($this->stopwatch) { + $this->stopwatchEvents[$event->getRequestId()] = $this->stopwatch->start( + 'mongodb.'.$this->clientName.'.'.$event->getCommandName(), + 'mongodb', + ); + } } public function commandSucceeded(CommandSucceededEvent $event): void { $this->events[] = $event; + + if (isset($this->stopwatchEvents[$event->getRequestId()])) { + $this->stopwatchEvents[$event->getRequestId()]->stop(); + unset($this->stopwatchEvents[$event->getRequestId()]); + } } public function reset(): void diff --git a/src/DependencyInjection/Compiler/DataCollectorPass.php b/src/DependencyInjection/Compiler/DataCollectorPass.php index 9ca2c5f..40230e8 100644 --- a/src/DependencyInjection/Compiler/DataCollectorPass.php +++ b/src/DependencyInjection/Compiler/DataCollectorPass.php @@ -23,6 +23,7 @@ use MongoDB\Bundle\DataCollector\DriverEventSubscriber; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; @@ -40,7 +41,13 @@ public function process(ContainerBuilder $container): void // Add a subscriber to each client to collect driver events, and register the client to the data collector. foreach ($container->findTaggedServiceIds('mongodb.client', true) as $clientId => $attributes) { $subscriberId = sprintf('%s.subscriber', $clientId); - $container->setDefinition($subscriberId, new Definition(DriverEventSubscriber::class)); + $subscriber = new Definition(DriverEventSubscriber::class); + $subscriber->setArguments([ + $attributes[0]['name'] ?? $clientId, + new Reference('debug.stopwatch', ContainerInterface::NULL_ON_INVALID_REFERENCE), + ]); + $container->setDefinition($subscriberId, $subscriber); + $container->getDefinition($clientId)->addMethodCall('addSubscriber', [new Reference($subscriberId)]); $dataCollector->addMethodCall('addClient', [ $attributes[0]['name'] ?? $clientId, From e54a8f58be741b798eb7f616fae194aeb99fb568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Sat, 11 Nov 2023 00:29:05 +0100 Subject: [PATCH 07/14] Collect backtrace --- composer.json | 1 + config/services.php | 14 +- src/DataCollector/DriverEventSubscriber.php | 91 +++++++----- src/DataCollector/MongoDBDataCollector.php | 130 ++++++++---------- .../Compiler/DataCollectorPass.php | 20 +-- src/DependencyInjection/MongoDBExtension.php | 9 +- templates/Collector/mongodb.html.twig | 108 ++++++++++++--- 7 files changed, 225 insertions(+), 148 deletions(-) diff --git a/composer.json b/composer.json index 3d83d13..c2f1ffa 100644 --- a/composer.json +++ b/composer.json @@ -27,6 +27,7 @@ "symfony/filesystem": "^6.3 || ^7.0", "symfony/framework-bundle": "^6.3.5 || ^7.0", "symfony/phpunit-bridge": "~6.3.10 || ^6.4.1 || ^7.0.1", + "symfony/stopwatch": "^6.3 || ^7.0", "symfony/yaml": "^6.3 || ^7.0", "symfony/web-profiler-bundle": "^6.3 || ^7.0", "zenstruck/browser": "^1.6" diff --git a/config/services.php b/config/services.php index 2a53e81..f280af1 100644 --- a/config/services.php +++ b/config/services.php @@ -22,6 +22,7 @@ use MongoDB\Bundle\Client; use MongoDB\Bundle\Command\DebugCommand; +use MongoDB\Bundle\DataCollector\DriverEventSubscriber; use MongoDB\Bundle\DataCollector\MongoDBDataCollector; return static function (ContainerConfigurator $container): void { @@ -39,13 +40,22 @@ ->tag('console.command'); $services - ->set('mongodb.prototype.client', Client::class) + ->set('mongodb.abstract.client', Client::class) ->arg('$uri', abstract_arg('Should be defined by pass')) ->arg('$uriOptions', abstract_arg('Should be defined by pass')) - ->arg('$driverOptions', abstract_arg('Should be defined by pass')); + ->arg('$driverOptions', abstract_arg('Should be defined by pass')) + ->abstract(); + + $services + ->set('mongodb.abstract.driver_event_subscriber', DriverEventSubscriber::class) + ->arg('$clientName', abstract_arg('Should be defined by pass')) + ->arg('$dataCollector', service('mongodb.data_collector')) + ->arg('$stopwatch', service('debug.stopwatch')->nullOnInvalid()) + ->abstract(); $services ->set('mongodb.data_collector', MongoDBDataCollector::class) + ->arg('$clients', tagged_iterator('mongodb.client', 'name')) ->tag('data_collector', [ 'template' => '@MongoDB/Collector/mongodb.html.twig', 'id' => 'mongodb', diff --git a/src/DataCollector/DriverEventSubscriber.php b/src/DataCollector/DriverEventSubscriber.php index 4e33695..2591e6a 100644 --- a/src/DataCollector/DriverEventSubscriber.php +++ b/src/DataCollector/DriverEventSubscriber.php @@ -25,65 +25,80 @@ use MongoDB\Driver\Monitoring\CommandSubscriber; use MongoDB\Driver\Monitoring\CommandSucceededEvent; use Symfony\Component\Stopwatch\Stopwatch; -use Symfony\Contracts\Service\ResetInterface; +use Symfony\Component\Stopwatch\StopwatchEvent; + +use function debug_backtrace; + +use const DEBUG_BACKTRACE_IGNORE_ARGS; /** @internal */ -final class DriverEventSubscriber implements CommandSubscriber, ResetInterface +final class DriverEventSubscriber implements CommandSubscriber { - /** - * @var list - */ - private array $events = []; + /** @var array */ private array $stopwatchEvents = []; public function __construct( - private string $clientName, - private ?Stopwatch $stopwatch = null, + private readonly string $clientName, + private readonly MongoDBDataCollector $dataCollector, + private readonly ?Stopwatch $stopwatch = null, ) { } - /** - * @return list - */ - public function getEvents(): array + public function commandStarted(CommandStartedEvent $event): void { - return $this->events; - } + $requestId = $event->getRequestId(); - public function commandFailed(CommandFailedEvent $event): void - { - $this->events[] = $event; + $command = (array) $event->getCommand(); + unset($command['lsid'], $command['$clusterTime']); - if (isset($this->stopwatchEvents[$event->getRequestId()])) { - $this->stopwatchEvents[$event->getRequestId()]->stop(); - unset($this->stopwatchEvents[$event->getRequestId()]); - } + $this->dataCollector->collectCommandEvent($this->clientName, $requestId, [ + 'clientName' => $this->clientName, + 'databaseName' => $event->getDatabaseName(), + 'commandName' => $event->getCommandName(), + 'command' => $command, + 'operationId' => $event->getOperationId(), + 'serviceId' => $event->getServiceId(), + 'backtrace' => $this->filterBacktrace(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS)), + ]); + + $this->stopwatchEvents[$requestId] = $this->stopwatch?->start( + 'mongodb.' . $this->clientName . '.' . $event->getCommandName(), + 'mongodb', + ); } - public function commandStarted(CommandStartedEvent $event): void + public function commandSucceeded(CommandSucceededEvent $event): void { - $this->events[] = $event; - - if ($this->stopwatch) { - $this->stopwatchEvents[$event->getRequestId()] = $this->stopwatch->start( - 'mongodb.'.$this->clientName.'.'.$event->getCommandName(), - 'mongodb', - ); - } + $requestId = $event->getRequestId(); + + $this->stopwatchEvents[$requestId]?->stop(); + unset($this->stopwatchEvents[$requestId]); + + $this->dataCollector->collectCommandEvent($this->clientName, $requestId, [ + 'clientName' => $this->clientName, + 'durationMicros' => $event->getDurationMicros(), + ]); } - public function commandSucceeded(CommandSucceededEvent $event): void + public function commandFailed(CommandFailedEvent $event): void { - $this->events[] = $event; + $requestId = $event->getRequestId(); + + $this->stopwatchEvents[$requestId]?->stop(); + unset($this->stopwatchEvents[$requestId]); - if (isset($this->stopwatchEvents[$event->getRequestId()])) { - $this->stopwatchEvents[$event->getRequestId()]->stop(); - unset($this->stopwatchEvents[$event->getRequestId()]); - } + $this->dataCollector->collectCommandEvent($this->clientName, $requestId, [ + 'clientName' => $this->clientName, + 'durationMicros' => $event->getDurationMicros(), + 'error' => $event->getError(), + ]); } - public function reset(): void + private function filterBacktrace(array $backtrace): array { - $this->events = []; + // skip first since it's always the current method + array_shift($backtrace); + + return $backtrace; } } diff --git a/src/DataCollector/MongoDBDataCollector.php b/src/DataCollector/MongoDBDataCollector.php index 203a0fa..d745e40 100644 --- a/src/DataCollector/MongoDBDataCollector.php +++ b/src/DataCollector/MongoDBDataCollector.php @@ -20,97 +20,88 @@ namespace MongoDB\Bundle\DataCollector; -use MongoDB\Bundle\Client; -use MongoDB\Driver\Monitoring\CommandFailedEvent; -use MongoDB\Driver\Monitoring\CommandStartedEvent; -use MongoDB\Driver\Monitoring\CommandSucceededEvent; +use MongoDB\Client; +use MongoDB\Driver\Command; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\DataCollector\DataCollector; +use Symfony\Component\HttpKernel\DataCollector\LateDataCollectorInterface; +use Throwable; + +use function array_column; +use function array_diff_key; +use function array_map; +use function array_sum; +use function count; +use function debug_backtrace; +use function dump; +use function iterator_to_array; + +use const DEBUG_BACKTRACE_IGNORE_ARGS; /** @internal */ -final class MongoDBDataCollector extends DataCollector +final class MongoDBDataCollector extends DataCollector implements LateDataCollectorInterface { /** - * @var list + * The list of request by client name is built with driver event data. + * + * @var array> */ - private array $clients = []; + private array $requests = []; - public function addClient(string $name, Client $client, DriverEventSubscriber $subscriber): void - { - $this->clients[$name] = [ - 'client' => $client, - 'subscriber' => $subscriber, - ]; + public function __construct( + /** @var iterable */ + private readonly iterable $clients = [], + ) { } - public function collect(Request $request, Response $response, ?\Throwable $exception = null): void + public function collectCommandEvent(string $clientName, string $requestId, array $data): void { - foreach ($this->clients as $name => ['client' => $client, 'subscriber' => $subscriber]) { - $totalTime = 0; - $requestCount = 0; - $errorCount = 0; - $requests = []; - - foreach ($subscriber->getEvents() as $event) { - $requestId = $event->getRequestId(); - - if ($event instanceof CommandStartedEvent) { - $command = (array) $event->getCommand(); - unset($command['lsid'], $command['$clusterTime']); - - $requests[$requestId] = [ - 'client' => $name, - 'startedAt' => hrtime(true), - 'commandName' => $event->getCommandName(), - 'command' => $command, - 'operationId' => $event->getOperationId(), - 'database' => $event->getDatabaseName(), - 'serviceId' => $event->getServiceId(), - ]; - ++$requestCount; - } elseif ($event instanceof CommandSucceededEvent) { - $requests[$requestId] += [ - 'duration' => $event->getDurationMicros(), - 'endedAt' => hrtime(true), - 'success' => true, - ]; - $totalTime += $event->getDurationMicros(); - } elseif ($event instanceof CommandFailedEvent) { - $requests[$requestId] += [ - 'duration' => $event->getDurationMicros(), - 'error' => $event->getError(), - 'success' => false, - ]; - $totalTime += $event->getDurationMicros(); - ++$errorCount; - } - } - - $this->data['clients'][$name] = [ - 'name' => $name, - 'uri' => (string) $client, - 'totalTime' => $totalTime, - 'requestCount' => $requestCount, - 'errorCount' => $errorCount, - 'requests' => $requests, - ]; + if (isset($this->requests[$clientName][$requestId])) { + $this->requests[$clientName][$requestId] += $data; + } else { + $this->requests[$clientName][$requestId] = $data; } } + public function collect(Request $request, Response $response, ?Throwable $exception = null): void + { + } + + public function lateCollect(): void + { + $this->data = [ + 'clients' => array_map(static fn (Client $client) => [ + 'serverBuildInfo' => $client->getManager()->executeCommand('admin', new Command(['buildInfo' => 1]))->toArray()[0], + 'clientInfo' => array_diff_key($client->__debugInfo(), ['manager' => 1]), + ], iterator_to_array($this->clients)), + 'requests' => $this->requests, + 'requestCount' => array_sum(array_map(count(...), $this->requests)), + 'errorCount' => array_sum(array_map(static fn (array $requests) => count(array_column($requests, 'error')), $this->requests)), + 'durationMicros' => array_sum(array_map(static fn (array $requests) => array_sum(array_column($requests, 'durationMicros')), $this->requests)), + ]; + + dump($this->data, array_column(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS), 'class')); + } + public function getRequestCount(): int { - return array_sum(array_column($this->data['clients'], 'requestCount')); + return $this->data['requestCount']; } public function getErrorCount(): int { - return array_sum(array_column($this->data['clients'], 'errorCount')); + return $this->data['errorCount']; + } + + public function getTime(): int + { + return $this->data['durationMicros']; } - public function getTime(): float + public function getRequests(): array { - return array_sum(array_column($this->data['clients'], 'totalTime')); + return $this->data['requests']; } public function getClients(): array @@ -125,10 +116,7 @@ public function getName(): string public function reset(): void { + $this->requests = []; $this->data = []; - - foreach ($this->clients as ['subscriber' => $subscriber]) { - $subscriber->reset(); - } } } diff --git a/src/DependencyInjection/Compiler/DataCollectorPass.php b/src/DependencyInjection/Compiler/DataCollectorPass.php index 40230e8..551ab4c 100644 --- a/src/DependencyInjection/Compiler/DataCollectorPass.php +++ b/src/DependencyInjection/Compiler/DataCollectorPass.php @@ -21,39 +21,31 @@ namespace MongoDB\Bundle\DependencyInjection\Compiler; use MongoDB\Bundle\DataCollector\DriverEventSubscriber; +use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; +use function sprintf; + /** @internal */ final class DataCollectorPass implements CompilerPassInterface { public function process(ContainerBuilder $container): void { - if (!$container->has('profiler')) { + if (! $container->has('profiler')) { return; } - $dataCollector = $container->getDefinition('mongodb.data_collector'); - // Add a subscriber to each client to collect driver events, and register the client to the data collector. foreach ($container->findTaggedServiceIds('mongodb.client', true) as $clientId => $attributes) { $subscriberId = sprintf('%s.subscriber', $clientId); - $subscriber = new Definition(DriverEventSubscriber::class); - $subscriber->setArguments([ - $attributes[0]['name'] ?? $clientId, - new Reference('debug.stopwatch', ContainerInterface::NULL_ON_INVALID_REFERENCE), - ]); + $subscriber = new ChildDefinition('mongodb.abstract.driver_event_subscriber'); + $subscriber->replaceArgument('$clientName', $attributes[0]['name'] ?? $clientId); $container->setDefinition($subscriberId, $subscriber); - $container->getDefinition($clientId)->addMethodCall('addSubscriber', [new Reference($subscriberId)]); - $dataCollector->addMethodCall('addClient', [ - $attributes[0]['name'] ?? $clientId, - new Reference($clientId), - new Reference($subscriberId), - ]); } } } diff --git a/src/DependencyInjection/MongoDBExtension.php b/src/DependencyInjection/MongoDBExtension.php index 3982e60..8572ab4 100644 --- a/src/DependencyInjection/MongoDBExtension.php +++ b/src/DependencyInjection/MongoDBExtension.php @@ -23,6 +23,7 @@ use InvalidArgumentException; use MongoDB\Client; use Symfony\Component\Config\FileLocator; +use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Extension\Extension; @@ -62,13 +63,10 @@ public static function createClientServiceId(string $clientId): string private function createClients(string $defaultClient, array $clients, ContainerBuilder $container): void { - $clientPrototype = $container->getDefinition('mongodb.prototype.client'); - $dataCollector = $container->getDefinition('mongodb.data_collector'); - foreach ($clients as $client => $configuration) { $serviceId = self::createClientServiceId($client); - $clientDefinition = clone $clientPrototype; + $clientDefinition = new ChildDefinition('mongodb.abstract.client'); $clientDefinition->setArgument('$uri', $configuration['uri']); $clientDefinition->setArgument('$uriOptions', $configuration['uri_options'] ?? []); $clientDefinition->setArgument('$driverOptions', $configuration['driver_options'] ?? []); @@ -92,8 +90,5 @@ private function createClients(string $defaultClient, array $clients, ContainerB $clients[$defaultClient]['default_database'], ); } - - // Remove the prototype definition as it's tagged as client - $container->removeDefinition('mongodb.prototype.client'); } } diff --git a/templates/Collector/mongodb.html.twig b/templates/Collector/mongodb.html.twig index 9e4fe79..cb333fb 100644 --- a/templates/Collector/mongodb.html.twig +++ b/templates/Collector/mongodb.html.twig @@ -71,40 +71,116 @@
-
-
+
+

+ Requests +

+
- {% if not collector.clients %} + {% if collector.requests is empty %}
-

No executed request.

+

No executed requests.

{% else %} - {% for name, client in collector.clients %} + {% for client, requests in collector.requests %} {% if collector.clients|length > 1 %} -

{{ name }} client

+

{{ client }} client

{% endif %} - {% if client.requests is empty %} + {% if requests is empty %}
-

No database queries were performed.

+

No database requests were performed.

{% else %} - +
- - + + - - {% for i, request in client.requests %} - + + {% for i, request in requests %} + - + {% endfor %} From 47ad9915134984a06478cf5dff19170c06ae7109 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Sun, 12 Nov 2023 19:43:09 +0100 Subject: [PATCH 08/14] Pretty render backtrace --- src/DataCollector/MongoDBDataCollector.php | 4 - templates/Collector/mongodb.html.twig | 119 +++++++++++---------- 2 files changed, 64 insertions(+), 59 deletions(-) diff --git a/src/DataCollector/MongoDBDataCollector.php b/src/DataCollector/MongoDBDataCollector.php index d745e40..12d619e 100644 --- a/src/DataCollector/MongoDBDataCollector.php +++ b/src/DataCollector/MongoDBDataCollector.php @@ -33,8 +33,6 @@ use function array_map; use function array_sum; use function count; -use function debug_backtrace; -use function dump; use function iterator_to_array; use const DEBUG_BACKTRACE_IGNORE_ARGS; @@ -80,8 +78,6 @@ public function lateCollect(): void 'errorCount' => array_sum(array_map(static fn (array $requests) => count(array_column($requests, 'error')), $this->requests)), 'durationMicros' => array_sum(array_map(static fn (array $requests) => array_sum(array_column($requests, 'durationMicros')), $this->requests)), ]; - - dump($this->data, array_column(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS), 'class')); } public function getRequestCount(): int diff --git a/templates/Collector/mongodb.html.twig b/templates/Collector/mongodb.html.twig index cb333fb..58953e2 100644 --- a/templates/Collector/mongodb.html.twig +++ b/templates/Collector/mongodb.html.twig @@ -2,6 +2,35 @@ {% import _self as helper %} +{% block head %} + {{ parent() }} + + +{% endblock %} + {% block toolbar %} {% if collector.requestCount > 0 %} @@ -112,19 +141,8 @@
View formatted request - {# - {% if request.runnable %} -    - View runnable request - {% endif %} - - {% if request.explainable %} -    - Explain request - {% endif %} - #} - {% if request.backtrace is defined %} +    View request backtrace {% endif %}
@@ -134,51 +152,42 @@ - {# - {% if request.runnable %} - - {% endif %} - - {% if request.explainable %} -
- {% endif %} - #} - {% if request.backtrace is defined %} -
-
#Time#▲Time Info
{{ loop.index }}{{ '%0.2f'|format(request.duration / 1000) }} ms{{ '%0.2f'|format(request.durationMicros / 1000) }} ms - 
{{ request.command|json_encode(constant('JSON_PRETTY_PRINT')) }}
+ {{ dump(request.command) }} + +
+ View formatted request + + {# + {% if request.runnable %} +    + View runnable request + {% endif %} + + {% if request.explainable %} +    + Explain request + {% endif %} + #} + + {% if request.backtrace is defined %} + View request backtrace + {% endif %} +
+ + + + {# + {% if request.runnable %} + + {% endif %} + + {% if request.explainable %} +
+ {% endif %} + #} + + {% if request.backtrace is defined %} +
+ + + + + + + + + {% for trace in request.backtrace %} + + + + + {% endfor %} + +
#File/Call
{{ loop.index }} + + {% set line_number = trace.line|default(1) %} + {% if trace.file is defined %} + + {% endif %} + {{- trace.class|default ~ (trace.class is defined ? trace.type|default('::')) -}} + {{ trace.function }} + {% if trace.file is defined %} + + {% endif %} + (line {{ line_number }}) + +
+
+ {% endif %}
- - - - - - - - {% for trace in request.backtrace %} - - - - - {% endfor %} - -
#File/Call
{{ loop.index }} - - {% set line_number = trace.line|default(1) %} - {% if trace.file is defined %} - - {% endif %} - {{- trace.class|default ~ (trace.class is defined ? trace.type|default('::')) -}} - {{ trace.function }} +
+ + {% endif %} + {{- trace.file|file_relative|default('n/a') -}} + {% if trace.file is defined %} + + {% endif %} + -> {{ trace.function }} + (line {{ line_number }}) + +
+
+ {{ trace.file|file_excerpt(trace.line)|replace({ + '#DD0000': 'var(--highlight-string)', + '#007700': 'var(--highlight-keyword)', + '#0000BB': 'var(--highlight-default)', + '#FF8000': 'var(--highlight-comment)' + })|raw }} +
+
+ {% endfor %}
{% endif %} From 08aa322bdaa3a3cfff28bd3ce30524be0e30a6b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Sun, 12 Nov 2023 23:30:03 +0100 Subject: [PATCH 09/14] Fix tests and CS --- src/Client.php | 4 +-- src/DataCollector/DriverEventSubscriber.php | 1 + src/DataCollector/MongoDBDataCollector.php | 2 -- .../Compiler/DataCollectorPass.php | 3 -- src/DependencyInjection/MongoDBExtension.php | 13 +++---- .../MongoDBExtensionTest.php | 36 ++++++++++++------- 6 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/Client.php b/src/Client.php index 1f3e2ce..3665f20 100644 --- a/src/Client.php +++ b/src/Client.php @@ -25,9 +25,7 @@ final class Client extends MongoDBClient { - /** - * @internal - */ + /** @internal */ public function addSubscriber(Subscriber $subscriber): void { $this->getManager()->addSubscriber($subscriber); diff --git a/src/DataCollector/DriverEventSubscriber.php b/src/DataCollector/DriverEventSubscriber.php index 2591e6a..03baff1 100644 --- a/src/DataCollector/DriverEventSubscriber.php +++ b/src/DataCollector/DriverEventSubscriber.php @@ -27,6 +27,7 @@ use Symfony\Component\Stopwatch\Stopwatch; use Symfony\Component\Stopwatch\StopwatchEvent; +use function array_shift; use function debug_backtrace; use const DEBUG_BACKTRACE_IGNORE_ARGS; diff --git a/src/DataCollector/MongoDBDataCollector.php b/src/DataCollector/MongoDBDataCollector.php index 12d619e..a12e94d 100644 --- a/src/DataCollector/MongoDBDataCollector.php +++ b/src/DataCollector/MongoDBDataCollector.php @@ -35,8 +35,6 @@ use function count; use function iterator_to_array; -use const DEBUG_BACKTRACE_IGNORE_ARGS; - /** @internal */ final class MongoDBDataCollector extends DataCollector implements LateDataCollectorInterface { diff --git a/src/DependencyInjection/Compiler/DataCollectorPass.php b/src/DependencyInjection/Compiler/DataCollectorPass.php index 551ab4c..29d14f0 100644 --- a/src/DependencyInjection/Compiler/DataCollectorPass.php +++ b/src/DependencyInjection/Compiler/DataCollectorPass.php @@ -20,12 +20,9 @@ namespace MongoDB\Bundle\DependencyInjection\Compiler; -use MongoDB\Bundle\DataCollector\DriverEventSubscriber; use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\ContainerInterface; -use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; use function sprintf; diff --git a/src/DependencyInjection/MongoDBExtension.php b/src/DependencyInjection/MongoDBExtension.php index 8572ab4..3b4ce85 100644 --- a/src/DependencyInjection/MongoDBExtension.php +++ b/src/DependencyInjection/MongoDBExtension.php @@ -25,7 +25,6 @@ use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Extension\Extension; use Symfony\Component\DependencyInjection\Loader\PhpFileLoader; @@ -84,11 +83,13 @@ private function createClients(string $defaultClient, array $clients, ContainerB // Register an autowiring alias for the default client $container->setAlias(Client::class, self::createClientServiceId($defaultClient)); - if (isset($clients[$defaultClient]['default_database'])) { - $container->setParameter( - sprintf('%s.default_database', Client::class), - $clients[$defaultClient]['default_database'], - ); + if (! isset($clients[$defaultClient]['default_database'])) { + return; } + + $container->setParameter( + sprintf('%s.default_database', Client::class), + $clients[$defaultClient]['default_database'], + ); } } diff --git a/tests/Unit/DependencyInjection/MongoDBExtensionTest.php b/tests/Unit/DependencyInjection/MongoDBExtensionTest.php index cfd7d5c..933e9a4 100644 --- a/tests/Unit/DependencyInjection/MongoDBExtensionTest.php +++ b/tests/Unit/DependencyInjection/MongoDBExtensionTest.php @@ -26,10 +26,16 @@ use MongoDB\Client; use MongoDB\Driver\ServerApi; use PHPUnit\Framework\TestCase; +use stdClass; +use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag; +use function array_column; +use function is_a; +use function sprintf; + /** @covers \MongoDB\Bundle\DependencyInjection\MongoDBExtension */ final class MongoDBExtensionTest extends TestCase { @@ -52,8 +58,9 @@ public function testLoadWithSingleClient(): void 'clients' => [ ['id' => 'default', 'uri' => 'mongodb://localhost:27017'], ], - ]], [ - 'profiler' => new Definition(\stdClass::class), + ], + ], [ + 'profiler' => new Definition(stdClass::class), ]); $this->assertTrue($container->hasDefinition('mongodb.client.default')); @@ -62,18 +69,17 @@ public function testLoadWithSingleClient(): void // Check service definition $definition = $container->getDefinition('mongodb.client.default'); - $this->assertTrue(is_a($definition->getClass(), Client::class, true), sprintf('Expected "%s" to be a "%s"', $definition->getClass(), Client::class)); $this->assertSame('mongodb://localhost:27017', $definition->getArgument('$uri')); $this->assertTrue($definition->hasMethodCall('addSubscriber')); + $this->assertInstanceOf(ChildDefinition::class, $definition); + $this->assertSame('mongodb.abstract.client', $definition->getParent()); + $parentDefinition = $container->getDefinition($definition->getParent()); + $this->assertTrue(is_a($parentDefinition->getClass(), Client::class, true), sprintf('Expected "%s" to be a "%s"', $definition->getClass(), Client::class)); + $this->assertTrue($parentDefinition->isAbstract()); // Check alias definition $alias = $container->getAlias(Client::class); $this->assertSame('mongodb.client.default', (string) $alias); - - // Check data collector - $definition = $container->getDefinition('mongodb.data_collector'); - $this->assertTrue($definition->hasMethodCall('addClient')); - $this->assertSame('default', $definition->getMethodCalls()[0][1][0]); } public function testLoadWithoutProfiler(): void @@ -82,7 +88,8 @@ public function testLoadWithoutProfiler(): void 'clients' => [ ['id' => 'default', 'uri' => 'mongodb://localhost:27017'], ], - ]]); + ], + ]); // Check service definition $definition = $container->getDefinition('mongodb.client.default'); @@ -109,8 +116,9 @@ public function testLoadWithMultipleClients(): void 'driver_options' => ['serverApi' => new ServerApi((string) ServerApi::V1)], ], ], - ]], [ - 'profiler' => new Definition(\stdClass::class), + ], + ], [ + 'profiler' => new Definition(stdClass::class), ]); $this->assertTrue($container->hasDefinition('mongodb.client.default')); @@ -120,13 +128,15 @@ public function testLoadWithMultipleClients(): void // Check service definitions $definition = $container->getDefinition('mongodb.client.default'); - $this->assertTrue(is_a($definition->getClass(), Client::class, true), sprintf('Expected "%s" to be a "%s"', $definition->getClass(), Client::class)); + $this->assertInstanceOf(ChildDefinition::class, $definition); + $this->assertSame('mongodb.abstract.client', $definition->getParent()); $this->assertSame('mongodb://localhost:27017', $definition->getArgument('$uri')); $this->assertSame(['readPreference' => 'primary'], $definition->getArgument('$uriOptions')); $this->assertTrue($definition->hasMethodCall('addSubscriber')); $definition = $container->getDefinition('mongodb.client.secondary'); - $this->assertTrue(is_a($definition->getClass(), Client::class, true), sprintf('Expected "%s" to be a "%s"', $definition->getClass(), Client::class)); + $this->assertInstanceOf(ChildDefinition::class, $definition); + $this->assertSame('mongodb.abstract.client', $definition->getParent()); $this->assertSame('mongodb://localhost:27018', $definition->getArgument('$uri')); $this->assertEquals(['serverApi' => new ServerApi((string) ServerApi::V1)], $definition->getArgument('$driverOptions')); $this->assertTrue($definition->hasMethodCall('addSubscriber')); From e07a1677c3eb33bdc47b753a6b6b4fb062084b38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 13 Nov 2023 10:02:17 +0100 Subject: [PATCH 10/14] Add client info --- src/DataCollector/MongoDBDataCollector.php | 42 +++++++++---- templates/Collector/mongodb.html.twig | 69 +++++++++++++++++----- 2 files changed, 83 insertions(+), 28 deletions(-) diff --git a/src/DataCollector/MongoDBDataCollector.php b/src/DataCollector/MongoDBDataCollector.php index a12e94d..623c6b3 100644 --- a/src/DataCollector/MongoDBDataCollector.php +++ b/src/DataCollector/MongoDBDataCollector.php @@ -28,12 +28,7 @@ use Symfony\Component\HttpKernel\DataCollector\LateDataCollectorInterface; use Throwable; -use function array_column; use function array_diff_key; -use function array_map; -use function array_sum; -use function count; -use function iterator_to_array; /** @internal */ final class MongoDBDataCollector extends DataCollector implements LateDataCollectorInterface @@ -66,15 +61,36 @@ public function collect(Request $request, Response $response, ?Throwable $except public function lateCollect(): void { + $requests = $this->requests; + $requestCount = 0; + $errorCount = 0; + $durationMicros = 0; + + foreach ($requests as $clientName => $requestsByClient) { + foreach ($requestsByClient as $requestId => $request) { + $requestCount++; + $durationMicros += $request['durationMicros'] ?? 0; + $errorCount += isset($request['error']) ? 1 : 0; + } + } + + $clients = []; + foreach ($this->clients as $name => $client) { + $clients[$name] = [ + 'serverBuildInfo' => array_diff_key( + (array) $client->getManager()->executeCommand('admin', new Command(['buildInfo' => 1]))->toArray()[0], + ['versionArray' => 0, 'ok' => 0], + ), + 'clientInfo' => array_diff_key($client->__debugInfo(), ['manager' => 0]), + ]; + } + $this->data = [ - 'clients' => array_map(static fn (Client $client) => [ - 'serverBuildInfo' => $client->getManager()->executeCommand('admin', new Command(['buildInfo' => 1]))->toArray()[0], - 'clientInfo' => array_diff_key($client->__debugInfo(), ['manager' => 1]), - ], iterator_to_array($this->clients)), - 'requests' => $this->requests, - 'requestCount' => array_sum(array_map(count(...), $this->requests)), - 'errorCount' => array_sum(array_map(static fn (array $requests) => count(array_column($requests, 'error')), $this->requests)), - 'durationMicros' => array_sum(array_map(static fn (array $requests) => array_sum(array_column($requests, 'durationMicros')), $this->requests)), + 'clients' => $clients, + 'requests' => $requests, + 'requestCount' => $requestCount, + 'errorCount' => $errorCount, + 'durationMicros' => $durationMicros, ]; } diff --git a/templates/Collector/mongodb.html.twig b/templates/Collector/mongodb.html.twig index 58953e2..dbbe840 100644 --- a/templates/Collector/mongodb.html.twig +++ b/templates/Collector/mongodb.html.twig @@ -79,26 +79,26 @@ {% block panel %} -

Request Metrics

+

Request Metrics

-
-
-
- {{ collector.requestCount }} - Requests -
+
+
+
+ {{ collector.requestCount }} + Requests +
-
- {{ collector.errorCount }} - Errors -
+
+ {{ collector.errorCount }} + Errors +
-
- {{ '%0.2f'|format(collector.time / 1000) }} ms - Request time -
+
+ {{ '%0.2f'|format(collector.time / 1000) }} ms + Request time
+
@@ -200,4 +200,43 @@ {% endif %}
+ +
+

+ Clients + {{ collector.clients|length }} +

+ + +
+ {% if collector.clients is empty %} +
+

No clients were used.

+
+ {% endif %} + {% for clientName, client in collector.clients %} +

Client {{ clientName }}

+ + + + + + + + + + + + + + + + + + +
KeyValue
Client Debug Info{{ dump(client.clientInfo) }}
Server Build Info{{ dump(client.serverBuildInfo) }}
+ {% endfor %} +
+
+
{% endblock %} From ecf00630d06b30b0b43ede125c9de900db4e15ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Mon, 13 Nov 2023 15:42:39 +0100 Subject: [PATCH 11/14] Simplify DI using configurator --- config/services.php | 11 +----- src/Client.php | 33 ---------------- src/DataCollector/DriverEventSubscriber.php | 13 +++---- src/DataCollector/MongoDBDataCollector.php | 39 ++++++++++++------- .../Compiler/DataCollectorPass.php | 9 +---- .../MongoDBExtensionTest.php | 12 ++---- 6 files changed, 37 insertions(+), 80 deletions(-) delete mode 100644 src/Client.php diff --git a/config/services.php b/config/services.php index f280af1..4b6a596 100644 --- a/config/services.php +++ b/config/services.php @@ -20,10 +20,9 @@ namespace Symfony\Component\DependencyInjection\Loader\Configurator; -use MongoDB\Bundle\Client; use MongoDB\Bundle\Command\DebugCommand; -use MongoDB\Bundle\DataCollector\DriverEventSubscriber; use MongoDB\Bundle\DataCollector\MongoDBDataCollector; +use MongoDB\Client; return static function (ContainerConfigurator $container): void { // default configuration for services in *this* file @@ -46,15 +45,9 @@ ->arg('$driverOptions', abstract_arg('Should be defined by pass')) ->abstract(); - $services - ->set('mongodb.abstract.driver_event_subscriber', DriverEventSubscriber::class) - ->arg('$clientName', abstract_arg('Should be defined by pass')) - ->arg('$dataCollector', service('mongodb.data_collector')) - ->arg('$stopwatch', service('debug.stopwatch')->nullOnInvalid()) - ->abstract(); - $services ->set('mongodb.data_collector', MongoDBDataCollector::class) + ->arg('$stopwatch', service('debug.stopwatch')->nullOnInvalid()) ->arg('$clients', tagged_iterator('mongodb.client', 'name')) ->tag('data_collector', [ 'template' => '@MongoDB/Collector/mongodb.html.twig', diff --git a/src/Client.php b/src/Client.php deleted file mode 100644 index 3665f20..0000000 --- a/src/Client.php +++ /dev/null @@ -1,33 +0,0 @@ -getManager()->addSubscriber($subscriber); - } -} diff --git a/src/DataCollector/DriverEventSubscriber.php b/src/DataCollector/DriverEventSubscriber.php index 03baff1..7ffde24 100644 --- a/src/DataCollector/DriverEventSubscriber.php +++ b/src/DataCollector/DriverEventSubscriber.php @@ -39,7 +39,7 @@ final class DriverEventSubscriber implements CommandSubscriber private array $stopwatchEvents = []; public function __construct( - private readonly string $clientName, + private readonly int $clientId, private readonly MongoDBDataCollector $dataCollector, private readonly ?Stopwatch $stopwatch = null, ) { @@ -52,8 +52,7 @@ public function commandStarted(CommandStartedEvent $event): void $command = (array) $event->getCommand(); unset($command['lsid'], $command['$clusterTime']); - $this->dataCollector->collectCommandEvent($this->clientName, $requestId, [ - 'clientName' => $this->clientName, + $this->dataCollector->collectCommandEvent($this->clientId, $requestId, [ 'databaseName' => $event->getDatabaseName(), 'commandName' => $event->getCommandName(), 'command' => $command, @@ -63,7 +62,7 @@ public function commandStarted(CommandStartedEvent $event): void ]); $this->stopwatchEvents[$requestId] = $this->stopwatch?->start( - 'mongodb.' . $this->clientName . '.' . $event->getCommandName(), + 'mongodb.' . $event->getCommandName(), 'mongodb', ); } @@ -75,8 +74,7 @@ public function commandSucceeded(CommandSucceededEvent $event): void $this->stopwatchEvents[$requestId]?->stop(); unset($this->stopwatchEvents[$requestId]); - $this->dataCollector->collectCommandEvent($this->clientName, $requestId, [ - 'clientName' => $this->clientName, + $this->dataCollector->collectCommandEvent($this->clientId, $requestId, [ 'durationMicros' => $event->getDurationMicros(), ]); } @@ -88,8 +86,7 @@ public function commandFailed(CommandFailedEvent $event): void $this->stopwatchEvents[$requestId]?->stop(); unset($this->stopwatchEvents[$requestId]); - $this->dataCollector->collectCommandEvent($this->clientName, $requestId, [ - 'clientName' => $this->clientName, + $this->dataCollector->collectCommandEvent($this->clientId, $requestId, [ 'durationMicros' => $event->getDurationMicros(), 'error' => $event->getError(), ]); diff --git a/src/DataCollector/MongoDBDataCollector.php b/src/DataCollector/MongoDBDataCollector.php index 623c6b3..ff787f2 100644 --- a/src/DataCollector/MongoDBDataCollector.php +++ b/src/DataCollector/MongoDBDataCollector.php @@ -20,15 +20,18 @@ namespace MongoDB\Bundle\DataCollector; +use LogicException; use MongoDB\Client; use MongoDB\Driver\Command; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\DataCollector\DataCollector; use Symfony\Component\HttpKernel\DataCollector\LateDataCollectorInterface; +use Symfony\Component\Stopwatch\Stopwatch; use Throwable; use function array_diff_key; +use function spl_object_id; /** @internal */ final class MongoDBDataCollector extends DataCollector implements LateDataCollectorInterface @@ -41,17 +44,23 @@ final class MongoDBDataCollector extends DataCollector implements LateDataCollec private array $requests = []; public function __construct( + private readonly ?Stopwatch $stopwatch = null, /** @var iterable */ private readonly iterable $clients = [], ) { } - public function collectCommandEvent(string $clientName, string $requestId, array $data): void + public function configureClient(Client $client): void { - if (isset($this->requests[$clientName][$requestId])) { - $this->requests[$clientName][$requestId] += $data; + $client->getManager()->addSubscriber(new DriverEventSubscriber(spl_object_id($client), $this, $this->stopwatch)); + } + + public function collectCommandEvent(int $clientId, string $requestId, array $data): void + { + if (isset($this->requests[$clientId][$requestId])) { + $this->requests[$clientId][$requestId] += $data; } else { - $this->requests[$clientName][$requestId] = $data; + $this->requests[$clientId][$requestId] = $data; } } @@ -61,21 +70,14 @@ public function collect(Request $request, Response $response, ?Throwable $except public function lateCollect(): void { - $requests = $this->requests; $requestCount = 0; $errorCount = 0; $durationMicros = 0; - foreach ($requests as $clientName => $requestsByClient) { - foreach ($requestsByClient as $requestId => $request) { - $requestCount++; - $durationMicros += $request['durationMicros'] ?? 0; - $errorCount += isset($request['error']) ? 1 : 0; - } - } - $clients = []; + $clientIdMap = []; foreach ($this->clients as $name => $client) { + $clientIdMap[spl_object_id($client)] = $name; $clients[$name] = [ 'serverBuildInfo' => array_diff_key( (array) $client->getManager()->executeCommand('admin', new Command(['buildInfo' => 1]))->toArray()[0], @@ -85,6 +87,17 @@ public function lateCollect(): void ]; } + $requests = []; + foreach ($this->requests as $clientId => $requestsByClientId) { + $clientName = $clientIdMap[$clientId] ?? throw new LogicException('Client not found'); + foreach ($requestsByClientId as $requestId => $request) { + $requests[$clientName][$requestId] = $request; + $requestCount++; + $durationMicros += $request['durationMicros'] ?? 0; + $errorCount += isset($request['error']) ? 1 : 0; + } + } + $this->data = [ 'clients' => $clients, 'requests' => $requests, diff --git a/src/DependencyInjection/Compiler/DataCollectorPass.php b/src/DependencyInjection/Compiler/DataCollectorPass.php index 29d14f0..f058324 100644 --- a/src/DependencyInjection/Compiler/DataCollectorPass.php +++ b/src/DependencyInjection/Compiler/DataCollectorPass.php @@ -20,13 +20,10 @@ namespace MongoDB\Bundle\DependencyInjection\Compiler; -use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; -use function sprintf; - /** @internal */ final class DataCollectorPass implements CompilerPassInterface { @@ -38,11 +35,7 @@ public function process(ContainerBuilder $container): void // Add a subscriber to each client to collect driver events, and register the client to the data collector. foreach ($container->findTaggedServiceIds('mongodb.client', true) as $clientId => $attributes) { - $subscriberId = sprintf('%s.subscriber', $clientId); - $subscriber = new ChildDefinition('mongodb.abstract.driver_event_subscriber'); - $subscriber->replaceArgument('$clientName', $attributes[0]['name'] ?? $clientId); - $container->setDefinition($subscriberId, $subscriber); - $container->getDefinition($clientId)->addMethodCall('addSubscriber', [new Reference($subscriberId)]); + $container->getDefinition($clientId)->setConfigurator([new Reference('mongodb.data_collector'), 'configureClient']); } } } diff --git a/tests/Unit/DependencyInjection/MongoDBExtensionTest.php b/tests/Unit/DependencyInjection/MongoDBExtensionTest.php index 933e9a4..78e72af 100644 --- a/tests/Unit/DependencyInjection/MongoDBExtensionTest.php +++ b/tests/Unit/DependencyInjection/MongoDBExtensionTest.php @@ -70,7 +70,7 @@ public function testLoadWithSingleClient(): void // Check service definition $definition = $container->getDefinition('mongodb.client.default'); $this->assertSame('mongodb://localhost:27017', $definition->getArgument('$uri')); - $this->assertTrue($definition->hasMethodCall('addSubscriber')); + $this->assertNotNull($definition->getConfigurator()); $this->assertInstanceOf(ChildDefinition::class, $definition); $this->assertSame('mongodb.abstract.client', $definition->getParent()); $parentDefinition = $container->getDefinition($definition->getParent()); @@ -132,20 +132,14 @@ public function testLoadWithMultipleClients(): void $this->assertSame('mongodb.abstract.client', $definition->getParent()); $this->assertSame('mongodb://localhost:27017', $definition->getArgument('$uri')); $this->assertSame(['readPreference' => 'primary'], $definition->getArgument('$uriOptions')); - $this->assertTrue($definition->hasMethodCall('addSubscriber')); + $this->assertNotNull($definition->getConfigurator()); $definition = $container->getDefinition('mongodb.client.secondary'); $this->assertInstanceOf(ChildDefinition::class, $definition); $this->assertSame('mongodb.abstract.client', $definition->getParent()); $this->assertSame('mongodb://localhost:27018', $definition->getArgument('$uri')); $this->assertEquals(['serverApi' => new ServerApi((string) ServerApi::V1)], $definition->getArgument('$driverOptions')); - $this->assertTrue($definition->hasMethodCall('addSubscriber')); - - // Check data collector - $definition = $container->getDefinition('mongodb.data_collector'); - $this->assertCount(2, $definition->getMethodCalls()); - $this->assertTrue($definition->hasMethodCall('addClient')); - $this->assertSame(['default', 'secondary'], array_column(array_column($definition->getMethodCalls(), 1), 0)); + $this->assertNotNull($definition->getConfigurator()); } private function getContainer(array $config = [], array $thirdPartyDefinitions = []): ContainerBuilder From f9e288ce2cc37c2ca103e03d17da1260f4ff9641 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Tue, 14 Nov 2023 15:08:33 +0100 Subject: [PATCH 12/14] Add tests to collector --- src/DataCollector/CommandEventCollector.php | 11 ++ src/DataCollector/DriverEventSubscriber.php | 30 +++-- src/DataCollector/MongoDBDataCollector.php | 4 +- .../Compiler/DataCollectorPass.php | 6 +- templates/Collector/mongodb.html.twig | 14 +-- .../DriverEventSubscriberTest.php | 94 ++++++++++++++++ .../MongoDBDataCollectorTest.php | 105 ++++++++++++++++++ .../Stubs/CommandEventCollectorStub.php | 17 +++ .../MongoDBExtensionTest.php | 1 - 9 files changed, 260 insertions(+), 22 deletions(-) create mode 100644 src/DataCollector/CommandEventCollector.php create mode 100644 tests/Functional/DataCollector/DriverEventSubscriberTest.php create mode 100644 tests/Functional/DataCollector/MongoDBDataCollectorTest.php create mode 100644 tests/Functional/DataCollector/Stubs/CommandEventCollectorStub.php diff --git a/src/DataCollector/CommandEventCollector.php b/src/DataCollector/CommandEventCollector.php new file mode 100644 index 0000000..cb05e21 --- /dev/null +++ b/src/DataCollector/CommandEventCollector.php @@ -0,0 +1,11 @@ +getCommand(); unset($command['lsid'], $command['$clusterTime']); - $this->dataCollector->collectCommandEvent($this->clientId, $requestId, [ + $data = [ 'databaseName' => $event->getDatabaseName(), 'commandName' => $event->getCommandName(), 'command' => $command, 'operationId' => $event->getOperationId(), 'serviceId' => $event->getServiceId(), 'backtrace' => $this->filterBacktrace(debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS)), - ]); + ]; + + if ($event->getCommandName() === 'getMore') { + $data['cursorId'] = $event->getCommand()->getMore; + } + + $this->collector->collectCommandEvent($this->clientId, $requestId, $data); $this->stopwatchEvents[$requestId] = $this->stopwatch?->start( 'mongodb.' . $event->getCommandName(), @@ -74,9 +80,15 @@ public function commandSucceeded(CommandSucceededEvent $event): void $this->stopwatchEvents[$requestId]?->stop(); unset($this->stopwatchEvents[$requestId]); - $this->dataCollector->collectCommandEvent($this->clientId, $requestId, [ + $data = [ 'durationMicros' => $event->getDurationMicros(), - ]); + ]; + + if (isset($event->getReply()->cursor)) { + $data['cursorId'] = $event->getReply()->cursor->id; + } + + $this->collector->collectCommandEvent($this->clientId, $requestId, $data); } public function commandFailed(CommandFailedEvent $event): void @@ -86,10 +98,12 @@ public function commandFailed(CommandFailedEvent $event): void $this->stopwatchEvents[$requestId]?->stop(); unset($this->stopwatchEvents[$requestId]); - $this->dataCollector->collectCommandEvent($this->clientId, $requestId, [ + $data = [ 'durationMicros' => $event->getDurationMicros(), - 'error' => $event->getError(), - ]); + 'error' => (string) $event->getError(), + ]; + + $this->collector->collectCommandEvent($this->clientId, $requestId, $data); } private function filterBacktrace(array $backtrace): array diff --git a/src/DataCollector/MongoDBDataCollector.php b/src/DataCollector/MongoDBDataCollector.php index ff787f2..a14bcd4 100644 --- a/src/DataCollector/MongoDBDataCollector.php +++ b/src/DataCollector/MongoDBDataCollector.php @@ -34,10 +34,10 @@ use function spl_object_id; /** @internal */ -final class MongoDBDataCollector extends DataCollector implements LateDataCollectorInterface +final class MongoDBDataCollector extends DataCollector implements LateDataCollectorInterface, CommandEventCollector { /** - * The list of request by client name is built with driver event data. + * The list of request by client ID is built with driver event data. * * @var array> */ diff --git a/src/DependencyInjection/Compiler/DataCollectorPass.php b/src/DependencyInjection/Compiler/DataCollectorPass.php index f058324..8fdd230 100644 --- a/src/DependencyInjection/Compiler/DataCollectorPass.php +++ b/src/DependencyInjection/Compiler/DataCollectorPass.php @@ -33,7 +33,11 @@ public function process(ContainerBuilder $container): void return; } - // Add a subscriber to each client to collect driver events, and register the client to the data collector. + /** + * Add a subscriber to each client to collect driver events. + * + * @see \MongoDB\Bundle\DataCollector\MongoDBDataCollector::configureClient() + */ foreach ($container->findTaggedServiceIds('mongodb.client', true) as $clientId => $attributes) { $container->getDefinition($clientId)->setConfigurator([new Reference('mongodb.data_collector'), 'configureClient']); } diff --git a/templates/Collector/mongodb.html.twig b/templates/Collector/mongodb.html.twig index dbbe840..f756322 100644 --- a/templates/Collector/mongodb.html.twig +++ b/templates/Collector/mongodb.html.twig @@ -6,15 +6,10 @@ {{ parent() }} {% endblock %} @@ -139,15 +133,15 @@ {{ dump(request.command) }}
- View formatted request + View formatted command {% if request.backtrace is defined %}    - View request backtrace + View command backtrace {% endif %}
-