From b83827f836586ec01c51ee4fe272c39ecaa5abaa Mon Sep 17 00:00:00 2001 From: Tobias Bachert Date: Sat, 13 Aug 2022 21:33:20 +0200 Subject: [PATCH 1/8] Bump otel version to 0.0.14 --- composer.json | 2 +- .../Symfony/OtelSdkBundle/Mock/SpanExporter.php | 11 +++++++---- .../Symfony/OtelSdkBundle/OtelSdkBundleTest.php | 5 ++++- .../OtelSdkBundle/Trace/ExporterFactoryTest.php | 11 +++++++---- 4 files changed, 19 insertions(+), 10 deletions(-) diff --git a/composer.json b/composer.json index 666ebfb4..aad5d473 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,7 @@ "require": { "php": "^7.4 || ^8.0", "ext-json": "*", - "open-telemetry/opentelemetry": "^0.0.13", + "open-telemetry/opentelemetry": "^0.0.14", "php-http/discovery": "^1.14", "php-http/message": "^1.12" }, diff --git a/tests/Integration/Symfony/OtelSdkBundle/Mock/SpanExporter.php b/tests/Integration/Symfony/OtelSdkBundle/Mock/SpanExporter.php index d6056d9c..97c8f226 100644 --- a/tests/Integration/Symfony/OtelSdkBundle/Mock/SpanExporter.php +++ b/tests/Integration/Symfony/OtelSdkBundle/Mock/SpanExporter.php @@ -4,6 +4,9 @@ namespace OpenTelemetry\Test\Integration\Symfony\OtelSdkBundle\Mock; +use OpenTelemetry\SDK\Common\Future\CancellationInterface; +use OpenTelemetry\SDK\Common\Future\CompletedFuture; +use OpenTelemetry\SDK\Common\Future\FutureInterface; use OpenTelemetry\SDK\Trace\SpanExporterInterface; class SpanExporter implements SpanExporterInterface @@ -22,17 +25,17 @@ public static function fromConnectionString(string $endpointUrl, string $name, s return new self($name, $args); } - public function export(iterable $spans): int + public function export(iterable $spans, ?CancellationInterface $cancellation = null): FutureInterface { - return 1; + return new CompletedFuture(1); } - public function shutdown(): bool + public function shutdown(?CancellationInterface $cancellation = null): bool { return true; } - public function forceFlush(): bool + public function forceFlush(?CancellationInterface $cancellation = null): bool { return true; } diff --git a/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php b/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php index 2a4e8465..a8c55713 100644 --- a/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php +++ b/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php @@ -87,7 +87,7 @@ public function testWithMinimalConfig(): void public function testTracerProviderWithSimpleConfig(): void { $this->loadTestData('simple'); - + $this->assertInstanceOf( SDK\Trace\TracerProvider::class, $this->getTracerProvider() @@ -211,6 +211,9 @@ public function testTracingWithSimpleConfig(): void */ public function testTracingWithAlwaysOffSampler(): void { + $this->markTestSkipped('Requires BatchSpanProcessor update to not force-flush empty batch.'); + + /** @phpstan-ignore-next-line */ $this->load( self::wrapConfig([ 'resource' => [ diff --git a/tests/Unit/Symfony/OtelSdkBundle/Trace/ExporterFactoryTest.php b/tests/Unit/Symfony/OtelSdkBundle/Trace/ExporterFactoryTest.php index ff351a85..2e86e8a1 100644 --- a/tests/Unit/Symfony/OtelSdkBundle/Trace/ExporterFactoryTest.php +++ b/tests/Unit/Symfony/OtelSdkBundle/Trace/ExporterFactoryTest.php @@ -5,6 +5,9 @@ namespace OpenTelemetry\Test\Unit\Symfony\OtelSdkBundle\Trace; use OpenTelemetry\Contrib; +use OpenTelemetry\SDK\Common\Future\CancellationInterface; +use OpenTelemetry\SDK\Common\Future\CompletedFuture; +use OpenTelemetry\SDK\Common\Future\FutureInterface; use OpenTelemetry\SDK\Trace\SpanExporterInterface; use OpenTelemetry\Symfony\OtelSdkBundle\Trace\ExporterFactory; use PHPUnit\Framework\TestCase; @@ -223,17 +226,17 @@ public static function fromConnectionString(string $endpointUrl, string $name, s { } - public function export(iterable $spans): int + public function export(iterable $spans, ?CancellationInterface $cancellation = null): FutureInterface { - return 1; + return new CompletedFuture(1); } - public function shutdown(): bool + public function shutdown(?CancellationInterface $cancellation = null): bool { return true; } - public function forceFlush(): bool + public function forceFlush(?CancellationInterface $cancellation = null): bool { return true; } From ab29f33ed4967c551f04786dcc5dcb3ff390cdf5 Mon Sep 17 00:00:00 2001 From: Tobias Bachert Date: Sat, 13 Aug 2022 22:33:19 +0200 Subject: [PATCH 2/8] Add symfony request listener --- composer.json | 1 + .../SetAliasIfNotDefinedCompilerPass.php | 29 ++ .../DependencyInjection/Configuration.php | 58 ++++ .../DependencyInjection/OtelExtension.php | 48 ++++ .../HttpKernel/HeadersPropagator.php | 38 +++ .../OtelBundle/HttpKernel/RequestListener.php | 271 ++++++++++++++++++ src/Symfony/OtelBundle/OtelBundle.php | 46 +++ .../OtelBundle/Resources/services_kernel.php | 24 ++ src/Symfony/OtelBundle/composer.json | 24 ++ .../HttpKernel/HeadersPropagatorTest.php | 70 +++++ .../HttpKernel/RequestListenerTest.php | 218 ++++++++++++++ 11 files changed, 827 insertions(+) create mode 100644 src/Symfony/OtelBundle/DependencyInjection/Compiler/SetAliasIfNotDefinedCompilerPass.php create mode 100644 src/Symfony/OtelBundle/DependencyInjection/Configuration.php create mode 100644 src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php create mode 100644 src/Symfony/OtelBundle/HttpKernel/HeadersPropagator.php create mode 100644 src/Symfony/OtelBundle/HttpKernel/RequestListener.php create mode 100644 src/Symfony/OtelBundle/OtelBundle.php create mode 100644 src/Symfony/OtelBundle/Resources/services_kernel.php create mode 100644 src/Symfony/OtelBundle/composer.json create mode 100644 tests/Unit/Symfony/OtelBundle/HttpKernel/HeadersPropagatorTest.php create mode 100644 tests/Unit/Symfony/OtelBundle/HttpKernel/RequestListenerTest.php diff --git a/composer.json b/composer.json index aad5d473..5623f265 100644 --- a/composer.json +++ b/composer.json @@ -19,6 +19,7 @@ "php-http/message": "^1.12" }, "replace": { + "open-telemetry/contrib-symfony-instrumentation-bundle": "self.version", "open-telemetry/contrib-sdk-bundle": "self.version", "open-telemetry/contrib-aws": "self.version" }, diff --git a/src/Symfony/OtelBundle/DependencyInjection/Compiler/SetAliasIfNotDefinedCompilerPass.php b/src/Symfony/OtelBundle/DependencyInjection/Compiler/SetAliasIfNotDefinedCompilerPass.php new file mode 100644 index 00000000..03f59ed7 --- /dev/null +++ b/src/Symfony/OtelBundle/DependencyInjection/Compiler/SetAliasIfNotDefinedCompilerPass.php @@ -0,0 +1,29 @@ +service = $service; + $this->aliasService = $aliasService; + } + + public function process(ContainerBuilder $container): void + { + if ($container->has($this->service)) { + return; + } + + $container->setAlias($this->service, $this->aliasService); + } +} diff --git a/src/Symfony/OtelBundle/DependencyInjection/Configuration.php b/src/Symfony/OtelBundle/DependencyInjection/Configuration.php new file mode 100644 index 00000000..1e791da6 --- /dev/null +++ b/src/Symfony/OtelBundle/DependencyInjection/Configuration.php @@ -0,0 +1,58 @@ +getRootNode() + ->addDefaultsIfNotSet() + ->children() + ->arrayNode('tracing') + ->addDefaultsIfNotSet(); + + if (class_exists(HttpKernel::class)) { + $tracing->children()->append($this->kernelTracingNode()); + } + + return $builder; + } + + private function kernelTracingNode(): NodeDefinition + { + ($kernel = new ArrayNodeDefinition('kernel')) + ->addDefaultsIfNotSet() + ->canBeDisabled() + ->fixXmlConfig('requestHeader') + ->fixXmlConfig('responseHeader') + ->children() + ->booleanNode('extractRemoteContext')->defaultTrue()->end() + ->arrayNode('requestHeaders') + ->beforeNormalization()->castToArray()->end() + ->scalarPrototype()->cannotBeEmpty()->end() + ->end() + ->arrayNode('responseHeaders') + ->beforeNormalization()->castToArray()->end() + ->scalarPrototype()->cannotBeEmpty()->end() + ->end() + ->end() + ; + + return $kernel; + } +} diff --git a/src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php b/src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php new file mode 100644 index 00000000..5c6b4282 --- /dev/null +++ b/src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php @@ -0,0 +1,48 @@ +processConfiguration($this->getConfiguration($configs, $container), $configs); + + $loader = new PhpFileLoader($container, new FileLocator()); + + if ($config['tracing']['kernel']['enabled'] ?? false) { + $loader->load(__DIR__ . '/../Resources/services_kernel.php'); + $this->loadKernelTracing($config['tracing']['kernel'], $container); + } + } + + private function loadKernelTracing(array $config, ContainerBuilder $container): void + { + $container->getDefinition(RequestListener::class) + ->setArgument('$requestHeaders', $config['requestHeaders']) + ->setArgument('$responseHeaders', $config['responseHeaders']) + ; + + if (!$config['extractRemoteContext']) { + $container->getDefinition(RequestListener::class) + ->setArgument('$propagator', new Reference(NoopTextMapPropagator::class)) + ; + } + } + + public function getConfiguration(array $config, ContainerBuilder $container): ConfigurationInterface + { + return new Configuration(); + } +} diff --git a/src/Symfony/OtelBundle/HttpKernel/HeadersPropagator.php b/src/Symfony/OtelBundle/HttpKernel/HeadersPropagator.php new file mode 100644 index 00000000..dcc2b68b --- /dev/null +++ b/src/Symfony/OtelBundle/HttpKernel/HeadersPropagator.php @@ -0,0 +1,38 @@ +headers->keys(); + } + + /** + * @param Request $carrier + */ + public function get($carrier, string $key): ?string + { + /** @psalm-suppress InvalidArgument */ + return count($carrier->headers->all($key)) > 1 + /** @phpstan-ignore-next-line */ + ? implode(',', $carrier->headers->all($key)) + : $carrier->headers->get($key); + } +} diff --git a/src/Symfony/OtelBundle/HttpKernel/RequestListener.php b/src/Symfony/OtelBundle/HttpKernel/RequestListener.php new file mode 100644 index 00000000..eebd8580 --- /dev/null +++ b/src/Symfony/OtelBundle/HttpKernel/RequestListener.php @@ -0,0 +1,271 @@ + $requestHeaders + * @param iterable $responseHeaders + */ + public function __construct( + TracerProviderInterface $tracerProvider, + TextMapPropagatorInterface $propagator, + iterable $requestHeaders = [], + iterable $responseHeaders = [] + ) { + $this->tracer = $tracerProvider->getTracer( + OtelBundle::instrumentationName(), + OtelBundle::instrumentationVersion(), + TraceAttributes::SCHEMA_URL, + ); + $this->propagator = $propagator; + $this->propagationGetter = new HeadersPropagator(); + $this->requestHeaderAttributes = $this->createHeaderAttributeMapping('request', $requestHeaders); + $this->responseHeaderAttributes = $this->createHeaderAttributeMapping('response', $responseHeaders); + } + + public static function getSubscribedEvents(): array + { + return [ + KernelEvents::REQUEST => [ + ['startRequest', 10000], + ['recordRoute', 31], // after RouterListener + ], + KernelEvents::EXCEPTION => [ + ['recordException'], + ], + KernelEvents::RESPONSE => [ + ['recordResponse', -10000], + ], + KernelEvents::FINISH_REQUEST => [ + ['endScope', -10000], + ['endRequest', -10000], + ], + KernelEvents::TERMINATE => [ + ['terminateRequest', 10000], + ], + ]; + } + + public function startRequest(RequestEvent $event): void + { + $request = $event->getRequest(); + + /** @psalm-suppress ArgumentTypeCoercion */ + $spanBuilder = $this->tracer + /** @phan-suppress-next-line PhanTypeMismatchArgument */ + ->spanBuilder(sprintf('HTTP %s', $request->getMethod())) + ->setAttributes($this->requestAttributes($request)) + ->setAttributes($this->headerAttributes($request->headers, $this->requestHeaderAttributes)) + ; + + $parent = Context::getCurrent(); + + if ($event->isMainRequest()) { + $spanBuilder->setSpanKind(SpanKind::KIND_SERVER); + $parent = $this->propagator->extract( + $request, + $this->propagationGetter, + $parent, + ); + + if ($requestTime = $request->server->get('REQUEST_TIME_FLOAT')) { + $spanBuilder->setStartTimestamp((int) ($requestTime * 1_000_000_000)); + } + } + + $span = $spanBuilder->setParent($parent)->startSpan(); + $scope = $span->storeInContext($parent)->activate(); + + $request->attributes->set(self::REQUEST_ATTRIBUTE_SPAN, $span); + $request->attributes->set(self::REQUEST_ATTRIBUTE_SCOPE, $scope); + } + + public function recordRoute(RequestEvent $event): void + { + if (!$span = $this->fetchRequestSpan($event->getRequest())) { + return; + } + + if (($routeName = $event->getRequest()->attributes->get('_route', '')) === '') { + return; + } + + /** @phan-suppress-next-line PhanTypeMismatchArgument */ + $span->updateName($routeName); + $span->setAttribute(TraceAttributes::HTTP_ROUTE, $routeName); + } + + public function recordException(ExceptionEvent $event): void + { + if (!$span = $this->fetchRequestSpan($event->getRequest())) { + return; + } + + $span->recordException($event->getThrowable()); + $event->getRequest()->attributes->set(self::REQUEST_ATTRIBUTE_EXCEPTION, $event->getThrowable()); + } + + public function recordResponse(ResponseEvent $event): void + { + if (!$span = $this->fetchRequestSpan($event->getRequest())) { + return; + } + + $event->getRequest()->attributes->remove(self::REQUEST_ATTRIBUTE_EXCEPTION); + + if (!$span->isRecording()) { + return; + } + + $response = $event->getResponse(); + $span->setAttribute(TraceAttributes::HTTP_RESPONSE_CONTENT_LENGTH, $response->headers->get('Content-Length')); + $span->setAttribute(TraceAttributes::HTTP_STATUS_CODE, $response->getStatusCode()); + if ($response->getStatusCode() >= 500 && $response->getStatusCode() < 600) { + $span->setStatus(StatusCode::STATUS_ERROR); + } + + $span->setAttributes($this->headerAttributes($response->headers, $this->responseHeaderAttributes)); + } + + public function endScope(FinishRequestEvent $event): void + { + if (!$scope = $this->fetchRequestScope($event->getRequest())) { + return; + } + + $scope->detach(); + } + + public function endRequest(FinishRequestEvent $event): void + { + if (!$span = $this->fetchRequestSpan($event->getRequest())) { + return; + } + + if ($exception = $this->fetchRequestException($event->getRequest())) { + $span->setStatus(StatusCode::STATUS_ERROR, $exception->getMessage()); + } elseif ($event->isMainRequest()) { + // End span on ::terminateRequest() instead + return; + } + + $span->end(); + } + + public function terminateRequest(TerminateEvent $event): void + { + if (!$span = $this->fetchRequestSpan($event->getRequest())) { + return; + } + + $span->end(); + } + + private function fetchRequestSpan(Request $request): ?SpanInterface + { + return $this->fetchRequestAttribute($request, self::REQUEST_ATTRIBUTE_SPAN, SpanInterface::class); + } + + private function fetchRequestScope(Request $request): ?ScopeInterface + { + return $this->fetchRequestAttribute($request, self::REQUEST_ATTRIBUTE_SCOPE, ScopeInterface::class); + } + + private function fetchRequestException(Request $request): ?Throwable + { + return $this->fetchRequestAttribute($request, self::REQUEST_ATTRIBUTE_EXCEPTION, Throwable::class); + } + + /** + * @psalm-template T of object + * @psalm-param class-string $type + * @psalm-return T|null + */ + private function fetchRequestAttribute(Request $request, string $key, string $type): ?object + { + return ($object = $request->attributes->get($key)) instanceof $type + ? $object + : null; + } + + private function requestAttributes(Request $request): iterable + { + return [ + TraceAttributes::HTTP_METHOD => $request->getMethod(), + TraceAttributes::HTTP_TARGET => $request->getRequestUri(), + TraceAttributes::HTTP_HOST => $request->getHttpHost(), + TraceAttributes::HTTP_SCHEME => $request->getScheme(), + TraceAttributes::HTTP_FLAVOR => ($protocolVersion = $request->getProtocolVersion()) !== null + ? strtr($protocolVersion, ['HTTP/' => '']) + : null, + TraceAttributes::HTTP_USER_AGENT => $request->headers->get('User-Agent'), + TraceAttributes::HTTP_REQUEST_CONTENT_LENGTH => $request->headers->get('Content-Length'), + TraceAttributes::HTTP_CLIENT_IP => $request->getClientIp(), + + TraceAttributes::NET_PEER_IP => $request->server->get('REMOTE_ADDR'), + TraceAttributes::NET_PEER_PORT => $request->server->get('REMOTE_PORT'), + TraceAttributes::NET_PEER_NAME => $request->server->get('REMOTE_HOST'), + TraceAttributes::NET_HOST_IP => $request->server->get('SERVER_ADDR'), + TraceAttributes::NET_HOST_PORT => $request->server->get('SERVER_PORT'), + TraceAttributes::NET_HOST_NAME => $request->server->get('SERVER_NAME'), + ]; + } + + private function headerAttributes(HeaderBag $headerBag, array $headers): iterable + { + foreach ($headers as $header => $attribute) { + if ($headerBag->has($header)) { + yield $attribute => $headerBag->all($header); + } + } + } + + private function createHeaderAttributeMapping(string $type, iterable $headers): array + { + $headerAttributes = []; + foreach ($headers as $header) { + $lcHeader = strtolower($header); + $headerAttributes[$lcHeader] = sprintf('http.%s.header.%s', $type, strtr($lcHeader, ['-' => '_'])); + } + + return $headerAttributes; + } +} diff --git a/src/Symfony/OtelBundle/OtelBundle.php b/src/Symfony/OtelBundle/OtelBundle.php new file mode 100644 index 00000000..5006c7ff --- /dev/null +++ b/src/Symfony/OtelBundle/OtelBundle.php @@ -0,0 +1,46 @@ +addCompilerPass(new SetAliasIfNotDefinedCompilerPass(TextMapPropagatorInterface::class, NoopTextMapPropagator::class)); + $container->addCompilerPass(new SetAliasIfNotDefinedCompilerPass(TracerProviderInterface::class, NoopTracerProvider::class)); + $container->addCompilerPass(new SetAliasIfNotDefinedCompilerPass(MeterProviderInterface::class, NoopMeterProvider::class)); + } + + public static function instrumentationName(): string + { + return 'open-telemetry/contrib-symfony-instrumentation-bundle'; + } + + public static function instrumentationVersion(): ?string + { + if (!class_exists(InstalledVersions::class)) { + return null; + } + + try { + return InstalledVersions::getPrettyVersion(self::instrumentationName()); + } catch (OutOfBoundsException $e) { + return null; + } + } +} diff --git a/src/Symfony/OtelBundle/Resources/services_kernel.php b/src/Symfony/OtelBundle/Resources/services_kernel.php new file mode 100644 index 00000000..aaa91da7 --- /dev/null +++ b/src/Symfony/OtelBundle/Resources/services_kernel.php @@ -0,0 +1,24 @@ +services()->set(RequestListener::class) + ->autoconfigure() + ->arg(0, service(TracerProviderInterface::class)) + ->arg(1, service(TextMapPropagatorInterface::class)) + ; + + $configurator->services()->set(NoopTextMapPropagator::class); + $configurator->services()->set(NoopTracerProvider::class); + $configurator->services()->set(NoopMeterProvider::class); +}; diff --git a/src/Symfony/OtelBundle/composer.json b/src/Symfony/OtelBundle/composer.json new file mode 100644 index 00000000..c2c3927e --- /dev/null +++ b/src/Symfony/OtelBundle/composer.json @@ -0,0 +1,24 @@ +{ + "name": "open-telemetry/contrib-symfony-instrumentation-bundle", + "description": "Symfony instrumentation bundle", + "keywords": ["opentelemetry", "otel", "symfony", "bundle", "tracing"], + "type": "symfony-bundle", + "license": "Apache-2.0", + "require": { + "php": "^7.4 || ^8.0", + "composer-runtime-api": "^2.0", + "open-telemetry/api": "^0.0.14", + "open-telemetry/context": "^0.0.14", + "open-telemetry/sem-conv": "^0.0.14", + "symfony/config": "^4.4 || ^5.4 || ^6.1", + "symfony/dependency-injection": "^4.4 || ^5.4 || ^6.1" + }, + "require-dev": { + "symfony/http-kernel": "^4.4 || ^5.4 || ^6.1" + }, + "autoload": { + "psr-4": { + "OpenTelemetry\\Symfony\\OtelBundle\\": "" + } + } +} diff --git a/tests/Unit/Symfony/OtelBundle/HttpKernel/HeadersPropagatorTest.php b/tests/Unit/Symfony/OtelBundle/HttpKernel/HeadersPropagatorTest.php new file mode 100644 index 00000000..37e63a08 --- /dev/null +++ b/tests/Unit/Symfony/OtelBundle/HttpKernel/HeadersPropagatorTest.php @@ -0,0 +1,70 @@ +headers->set('a', 'value-a'); + $request->headers->set('b', 'value-b'); + + $this->assertSame( + ['a', 'b'], + (new HeadersPropagator())->keys($request), + ); + } + + public function testHeadersPropagatorReturnsValueForKey(): void + { + $request = new Request(); + $request->headers->set('a', 'value-a'); + + $this->assertSame( + 'value-a', + (new HeadersPropagator())->get($request, 'a'), + ); + } + + public function testHeadersPropagatorReturnsValueForKeyCaseInsensitive(): void + { + $request = new Request(); + $request->headers->set('a', 'value-a'); + + $this->assertSame( + 'value-a', + (new HeadersPropagator())->get($request, 'A'), + ); + } + + public function testHeadersPropagatorReturnsConcatenatedValueForKey(): void + { + $request = new Request(); + $request->headers->set('a', ['value-a-1', 'value-a-2']); + + $this->assertSame( + 'value-a-1,value-a-2', + (new HeadersPropagator())->get($request, 'a'), + ); + } + + public function testHeadersPropagatorReturnsNullForNotExistingKey(): void + { + $request = new Request(); + $request->headers->set('a', 'value-a'); + + $this->assertNull( + (new HeadersPropagator())->get($request, 'b'), + ); + } +} diff --git a/tests/Unit/Symfony/OtelBundle/HttpKernel/RequestListenerTest.php b/tests/Unit/Symfony/OtelBundle/HttpKernel/RequestListenerTest.php new file mode 100644 index 00000000..c4f6bef6 --- /dev/null +++ b/tests/Unit/Symfony/OtelBundle/HttpKernel/RequestListenerTest.php @@ -0,0 +1,218 @@ +createMock(HttpKernelInterface::class); + $request = new Request(); + + $scope = Context::storage()->scope(); + + $listener->startRequest(new RequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + $this->assertNotSame($scope, Context::storage()->scope()); + + $listener->endScope(new FinishRequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + $this->assertSame($scope, Context::storage()->scope()); + } + + public function testPropagatorIsCalledForMainRequest(): void + { + $propagator = $this->createMock(TextMapPropagatorInterface::class); + $propagator->expects($this->once())->method('extract')->willReturnArgument(2); + $listener = new RequestListener( + new NoopTracerProvider(), + $propagator, + ); + $kernel = $this->createMock(HttpKernelInterface::class); + $request = new Request(); + + $listener->startRequest(new RequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + $listener->endScope(new FinishRequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + } + + public function testPropagatorIsNotCalledForSubRequest(): void + { + $propagator = $this->createMock(TextMapPropagatorInterface::class); + $propagator->expects($this->never())->method('extract'); + $listener = new RequestListener( + new NoopTracerProvider(), + $propagator, + ); + $kernel = $this->createMock(HttpKernelInterface::class); + $request = new Request(); + + $listener->startRequest(new RequestEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST)); + $listener->endScope(new FinishRequestEvent($kernel, $request, HttpKernelInterface::SUB_REQUEST)); + } + + /** + * @dataProvider httpMethodProvider + */ + public function testNameUsesHttpMethod(string $method): void + { + $exporter = new InMemoryExporter(); + $tracerProvider = new TracerProvider([new SimpleSpanProcessor($exporter)]); + + $listener = new RequestListener($tracerProvider, new NoopTextMapPropagator()); + $request = new Request(); + $request->setMethod($method); + + $this->callListener($listener, $request); + + $this->assertSame("HTTP $method", $exporter->getSpans()[0]->getName()); + } + + public function httpMethodProvider(): iterable + { + yield ['GET']; + yield ['POST']; + } + + public function testRouteNameIsRecorded(): void + { + $exporter = new InMemoryExporter(); + $tracerProvider = new TracerProvider([new SimpleSpanProcessor($exporter)]); + + $listener = new RequestListener($tracerProvider, new NoopTextMapPropagator()); + $request = new Request(); + $request->attributes->set('_route', 'route-name'); + + $this->callListener($listener, $request); + + $this->assertSame('route-name', $exporter->getSpans()[0]->getName()); + $this->assertSame('route-name', $exporter->getSpans()[0]->getAttributes()->get('http.route')); + } + + public function testStatusCode5xxSetsStatusError(): void + { + $exporter = new InMemoryExporter(); + $tracerProvider = new TracerProvider([new SimpleSpanProcessor($exporter)]); + + $listener = new RequestListener($tracerProvider, new NoopTextMapPropagator()); + $request = new Request(); + $response = new Response(); + $response->setStatusCode(503); + + $this->callListener($listener, $request, $response); + + $this->assertSame(StatusCode::STATUS_ERROR, $exporter->getSpans()[0]->getStatus()->getCode()); + } + + public function testRequestHeaderAttributeIsSet(): void + { + $exporter = new InMemoryExporter(); + $tracerProvider = new TracerProvider([new SimpleSpanProcessor($exporter)]); + + $listener = new RequestListener($tracerProvider, new NoopTextMapPropagator(), ['x-test']); + $request = new Request(); + $request->headers->set('x-test', 'value'); + $request->headers->set('x-test2', 'value2'); + + $this->callListener($listener, $request); + + $this->assertSame(['value'], $exporter->getSpans()[0]->getAttributes()->get('http.request.header.x_test')); + } + + public function testResponseHeaderAttributeIsSet(): void + { + $exporter = new InMemoryExporter(); + $tracerProvider = new TracerProvider([new SimpleSpanProcessor($exporter)]); + + $listener = new RequestListener($tracerProvider, new NoopTextMapPropagator(), [], ['x-test']); + $request = new Request(); + $response = new Response(); + $response->headers->set('x-test', 'value'); + $response->headers->set('x-test2', 'value2'); + + $this->callListener($listener, $request, $response); + + $this->assertSame(['value'], $exporter->getSpans()[0]->getAttributes()->get('http.response.header.x_test')); + } + + public function testExceptionIsRecorded(): void + { + $exporter = new InMemoryExporter(); + $tracerProvider = new TracerProvider([new SimpleSpanProcessor($exporter)]); + + $listener = new RequestListener($tracerProvider, new NoopTextMapPropagator(), [], ['x-test']); + $request = new Request(); + $response = new Response(); + $response->headers->set('x-test', 'value'); + + $this->callListenerException($listener, $request, new Exception('exception')); + + $this->assertSame('exception', $exporter->getSpans()[0]->getEvents()[0]->getName()); + $this->assertSame(StatusCode::STATUS_ERROR, $exporter->getSpans()[0]->getStatus()->getCode()); + } + + public function testRemoteContextIsExtracted(): void + { + $exporter = new InMemoryExporter(); + $tracerProvider = new TracerProvider([new SimpleSpanProcessor($exporter)]); + + $listener = new RequestListener($tracerProvider, new TraceContextPropagator()); + $request = new Request(); + $request->headers->set('traceparent', '00-0af7651916cd43dd8448eb211c80319c-b9c7c989f97918e1-01'); + + $this->callListener($listener, $request); + + $this->assertSame('0af7651916cd43dd8448eb211c80319c', $exporter->getSpans()[0]->getContext()->getTraceId()); + } + + private function callListener(RequestListener $listener, ?Request $request, ?Response $response = null): void + { + $request ??= new Request(); + $response ??= new Response(); + + $kernel = $this->createMock(HttpKernelInterface::class); + $listener->startRequest(new RequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + $listener->recordRoute(new RequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + $listener->recordResponse(new ResponseEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $response)); + $listener->endScope(new FinishRequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + $listener->endRequest(new FinishRequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + $listener->terminateRequest(new TerminateEvent($kernel, $request, $response)); + } + + private function callListenerException(RequestListener $listener, ?Request $request, Throwable $exception): void + { + $request ??= new Request(); + + $kernel = $this->createMock(HttpKernelInterface::class); + $listener->startRequest(new RequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + $listener->recordRoute(new RequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + $listener->recordException(new ExceptionEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $exception)); + $listener->endScope(new FinishRequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + $listener->endRequest(new FinishRequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST)); + } +} From fe67c0530ae5281aa4a49190755bfe1c47c8bcb3 Mon Sep 17 00:00:00 2001 From: Tobias Bachert Date: Sun, 14 Aug 2022 10:35:57 +0200 Subject: [PATCH 3/8] Add container extension tests --- composer.json | 3 +- phpunit.xml.dist | 24 ++--- .../OtelBundle/Resources/services_kernel.php | 4 +- .../SetAliasIfNotDefinedCompilerPassTest.php | 35 +++++++ .../DependencyInjection/ConfigurationTest.php | 57 ++++++++++++ .../DependencyInjection/Fixtures/config.xml | 11 +++ .../DependencyInjection/Fixtures/config.yaml | 8 ++ .../DependencyInjection/OtelExtensionTest.php | 91 +++++++++++++++++++ .../Symfony/OtelBundle/OtelBundleTest.php | 30 ++++++ .../Util/ContainerConfiguratorHelperTest.php | 6 -- tests/bootstrap.php | 9 ++ 11 files changed, 257 insertions(+), 21 deletions(-) create mode 100644 tests/Unit/Symfony/OtelBundle/DependencyInjection/Compiler/SetAliasIfNotDefinedCompilerPassTest.php create mode 100644 tests/Unit/Symfony/OtelBundle/DependencyInjection/ConfigurationTest.php create mode 100644 tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.xml create mode 100644 tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.yaml create mode 100644 tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php create mode 100644 tests/Unit/Symfony/OtelBundle/OtelBundleTest.php create mode 100644 tests/bootstrap.php diff --git a/composer.json b/composer.json index 5623f265..cd26fecc 100644 --- a/composer.json +++ b/composer.json @@ -40,6 +40,7 @@ "guzzlehttp/guzzle": "^7.3", "guzzlehttp/psr7": "^2.0@RC", "kriswallsmith/buzz": "^1.2", + "matthiasnoback/symfony-dependency-injection-test": "^4.3", "mikey179/vfsstream": "^1.6", "nyholm/psr7": "^1.4", "open-telemetry/dev-tools": "dev-main", @@ -49,6 +50,7 @@ "phpstan/phpstan-symfony": "^1.1", "phpunit/phpunit": "^9.5", "psalm/plugin-phpunit": "^0.13.0", + "qossmic/deptrac-shim": "^0.22.1", "symfony/config": "^4.4|^5.0|^6.0", "symfony/http-client": "^5.3", "symfony/http-kernel": "^4.4|^5.3|^6.0", @@ -56,7 +58,6 @@ "symfony/polyfill-php80": "^1.16", "symfony/proxy-manager-bridge": "^4.4|^5.3|^6.0", "symfony/yaml": "^4.4|^5.3|^6.0", - "qossmic/deptrac-shim": "^0.22.1", "vimeo/psalm": "^4.0" }, "suggest": { diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 6e031c59..6d349144 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,23 +1,23 @@ -services()->set(RequestListener::class) ->autoconfigure() - ->arg(0, service(TracerProviderInterface::class)) - ->arg(1, service(TextMapPropagatorInterface::class)) + ->arg('$tracerProvider', service(TracerProviderInterface::class)) + ->arg('$propagator', service(TextMapPropagatorInterface::class)) ; $configurator->services()->set(NoopTextMapPropagator::class); diff --git a/tests/Unit/Symfony/OtelBundle/DependencyInjection/Compiler/SetAliasIfNotDefinedCompilerPassTest.php b/tests/Unit/Symfony/OtelBundle/DependencyInjection/Compiler/SetAliasIfNotDefinedCompilerPassTest.php new file mode 100644 index 00000000..59e731a6 --- /dev/null +++ b/tests/Unit/Symfony/OtelBundle/DependencyInjection/Compiler/SetAliasIfNotDefinedCompilerPassTest.php @@ -0,0 +1,35 @@ +addCompilerPass(new SetAliasIfNotDefinedCompilerPass('service', 'aliasService')); + } + + public function testSetsServiceAliasIfServiceNotSet(): void + { + $this->container->register('aliasService', 'AliasService'); + $this->compile(); + + $this->assertContainerBuilderHasService('service', 'AliasService'); + $this->assertContainerBuilderHasAlias('service', 'aliasService'); + } + + public function testDoesNotSetServiceAliasIfServiceSet(): void + { + $this->container->register('aliasService', 'AliasService'); + $this->container->register('service', 'Service'); + $this->compile(); + + $this->assertContainerBuilderHasService('service', 'Service'); + } +} diff --git a/tests/Unit/Symfony/OtelBundle/DependencyInjection/ConfigurationTest.php b/tests/Unit/Symfony/OtelBundle/DependencyInjection/ConfigurationTest.php new file mode 100644 index 00000000..46822b01 --- /dev/null +++ b/tests/Unit/Symfony/OtelBundle/DependencyInjection/ConfigurationTest.php @@ -0,0 +1,57 @@ + [ + 'kernel' => [ + 'enabled' => true, + 'extractRemoteContext' => true, + 'requestHeaders' => [ + 'Content-Length', + 'date', + ], + 'responseHeaders' => [ + 'link', + ], + ], + ], + ]; + + $this->assertProcessedConfigurationEquals($expectedConfiguration, $sources); + } + + public function sourcesProvider(): iterable + { + yield [[__DIR__ . '/Fixtures/config.yaml']]; + yield [[__DIR__ . '/Fixtures/config.xml']]; + } +} diff --git a/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.xml b/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.xml new file mode 100644 index 00000000..b230e9ca --- /dev/null +++ b/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.xml @@ -0,0 +1,11 @@ + + + + + Content-Length + date + link + + + + diff --git a/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.yaml b/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.yaml new file mode 100644 index 00000000..fecff0f0 --- /dev/null +++ b/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.yaml @@ -0,0 +1,8 @@ +otel: + tracing: + kernel: + requestHeaders: + - Content-Length + - date + responseHeaders: + - link diff --git a/tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php b/tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php new file mode 100644 index 00000000..e50915b3 --- /dev/null +++ b/tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php @@ -0,0 +1,91 @@ +load(); + + $this->assertContainerBuilderHasService(RequestListener::class); + } + + public function testKernelTracingCanBeDisabled(): void + { + $this->load([ + 'tracing' => [ + 'kernel' => false, + ], + ]); + + $this->assertContainerBuilderNotHasService(RequestListener::class); + } + + public function testExtractRemoteContextTrueUsesDefaultTextMapPropagator(): void + { + $this->load(); + + $this->assertContainerBuilderHasServiceDefinitionWithArgument(RequestListener::class, '$propagator', new Reference(TextMapPropagatorInterface::class)); + } + + public function testExtractRemoteContextFalseUsesNoopTextMapPropagator(): void + { + $this->load([ + 'tracing' => [ + 'kernel' => [ + 'extractRemoteContext' => false, + ], + ], + ]); + + $this->assertContainerBuilderHasServiceDefinitionWithArgument(RequestListener::class, '$propagator', new Reference(NoopTextMapPropagator::class)); + } + + public function testRequestHeadersSetsHeadersToExtract(): void + { + $this->load([ + 'tracing' => [ + 'kernel' => [ + 'requestHeaders' => ['a', 'b'], + ], + ], + ]); + + $this->assertContainerBuilderHasServiceDefinitionWithArgument(RequestListener::class, '$requestHeaders', ['a', 'b']); + $this->assertContainerBuilderHasServiceDefinitionWithArgument(RequestListener::class, '$responseHeaders', []); + } + + public function testResponseHeadersSetsHeadersToExtract(): void + { + $this->load([ + 'tracing' => [ + 'kernel' => [ + 'responseHeaders' => ['a', 'b'], + ], + ], + ]); + + $this->assertContainerBuilderHasServiceDefinitionWithArgument(RequestListener::class, '$requestHeaders', []); + $this->assertContainerBuilderHasServiceDefinitionWithArgument(RequestListener::class, '$responseHeaders', ['a', 'b']); + } +} diff --git a/tests/Unit/Symfony/OtelBundle/OtelBundleTest.php b/tests/Unit/Symfony/OtelBundle/OtelBundleTest.php new file mode 100644 index 00000000..ed1b940d --- /dev/null +++ b/tests/Unit/Symfony/OtelBundle/OtelBundleTest.php @@ -0,0 +1,30 @@ +getFileName()) . '/composer.json')) { + $this->fail(); + } + + $expectedName = json_decode($content)->name ?? null; + + $this->assertSame($expectedName, OtelBundle::instrumentationName()); + } +} diff --git a/tests/Unit/Symfony/OtelSdkBundle/Util/ContainerConfiguratorHelperTest.php b/tests/Unit/Symfony/OtelSdkBundle/Util/ContainerConfiguratorHelperTest.php index a9c0fe4f..48010312 100644 --- a/tests/Unit/Symfony/OtelSdkBundle/Util/ContainerConfiguratorHelperTest.php +++ b/tests/Unit/Symfony/OtelSdkBundle/Util/ContainerConfiguratorHelperTest.php @@ -4,7 +4,6 @@ namespace OpenTelemetry\Test\Unit\Symfony\OtelSdkBundle\Util; -use DG\BypassFinals; use OpenTelemetry\Symfony\OtelSdkBundle\Util\ContainerConfiguratorHelper; use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; @@ -13,11 +12,6 @@ class ContainerConfiguratorHelperTest extends TestCase { - public function setUp(): void - { - BypassFinals::enable(); - } - public function testCreate() { $this->assertInstanceOf( diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 00000000..6408be93 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,9 @@ + Date: Mon, 15 Aug 2022 10:17:18 +0200 Subject: [PATCH 4/8] Register noop services unconditionally --- .../DependencyInjection/OtelExtension.php | 1 + src/Symfony/OtelBundle/Resources/services.php | 15 +++++++++++++++ .../OtelBundle/Resources/services_kernel.php | 7 ------- .../DependencyInjection/OtelExtensionTest.php | 15 +++++++++++++++ 4 files changed, 31 insertions(+), 7 deletions(-) create mode 100644 src/Symfony/OtelBundle/Resources/services.php diff --git a/src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php b/src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php index 5c6b4282..27f98807 100644 --- a/src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php +++ b/src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php @@ -20,6 +20,7 @@ public function load(array $configs, ContainerBuilder $container) $config = $this->processConfiguration($this->getConfiguration($configs, $container), $configs); $loader = new PhpFileLoader($container, new FileLocator()); + $loader->load(__DIR__ . '/../Resources/services.php'); if ($config['tracing']['kernel']['enabled'] ?? false) { $loader->load(__DIR__ . '/../Resources/services_kernel.php'); diff --git a/src/Symfony/OtelBundle/Resources/services.php b/src/Symfony/OtelBundle/Resources/services.php new file mode 100644 index 00000000..412980fc --- /dev/null +++ b/src/Symfony/OtelBundle/Resources/services.php @@ -0,0 +1,15 @@ +services()->set(NoopTextMapPropagator::class); + $configurator->services()->set(NoopTracerProvider::class); + $configurator->services()->set(NoopMeterProvider::class); +}; diff --git a/src/Symfony/OtelBundle/Resources/services_kernel.php b/src/Symfony/OtelBundle/Resources/services_kernel.php index daae3153..39422ea3 100644 --- a/src/Symfony/OtelBundle/Resources/services_kernel.php +++ b/src/Symfony/OtelBundle/Resources/services_kernel.php @@ -4,10 +4,7 @@ namespace Symfony\Component\DependencyInjection\Loader\Configurator; -use OpenTelemetry\API\Metrics\Noop\NoopMeterProvider; -use OpenTelemetry\API\Trace\NoopTracerProvider; use OpenTelemetry\API\Trace\TracerProviderInterface; -use OpenTelemetry\Context\Propagation\NoopTextMapPropagator; use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; use OpenTelemetry\Symfony\OtelBundle\HttpKernel\RequestListener; @@ -17,8 +14,4 @@ ->arg('$tracerProvider', service(TracerProviderInterface::class)) ->arg('$propagator', service(TextMapPropagatorInterface::class)) ; - - $configurator->services()->set(NoopTextMapPropagator::class); - $configurator->services()->set(NoopTracerProvider::class); - $configurator->services()->set(NoopMeterProvider::class); }; diff --git a/tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php b/tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php index e50915b3..bf249e41 100644 --- a/tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php +++ b/tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php @@ -5,6 +5,8 @@ namespace OpenTelemetry\Test\Unit\Symfony\OtelBundle\DependencyInjection; use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractExtensionTestCase; +use OpenTelemetry\API\Metrics\Noop\NoopMeterProvider; +use OpenTelemetry\API\Trace\NoopTracerProvider; use OpenTelemetry\Context\Propagation\NoopTextMapPropagator; use OpenTelemetry\Context\Propagation\TextMapPropagatorInterface; use OpenTelemetry\Symfony\OtelBundle\DependencyInjection\OtelExtension; @@ -41,6 +43,19 @@ public function testKernelTracingCanBeDisabled(): void $this->assertContainerBuilderNotHasService(RequestListener::class); } + public function testNoopOtelServicesAreAlwaysRegistered(): void + { + $this->load([ + 'tracing' => [ + 'kernel' => false, + ], + ]); + + $this->assertContainerBuilderHasService(NoopTextMapPropagator::class); + $this->assertContainerBuilderHasService(NoopTracerProvider::class); + $this->assertContainerBuilderHasService(NoopMeterProvider::class); + } + public function testExtractRemoteContextTrueUsesDefaultTextMapPropagator(): void { $this->load(); From f6bee3fbba0072ef758755792378933fdd631179 Mon Sep 17 00:00:00 2001 From: Tobias Bachert Date: Sat, 20 Aug 2022 14:21:57 +0200 Subject: [PATCH 5/8] Move http header capturing configuration out of kernel node --- .../DependencyInjection/Configuration.php | 47 ++++++++++++++----- .../DependencyInjection/OtelExtension.php | 8 ++-- .../OtelBundle/Resources/services_kernel.php | 2 + .../DependencyInjection/ConfigurationTest.php | 18 ++++--- .../DependencyInjection/Fixtures/config.xml | 12 +++-- .../DependencyInjection/Fixtures/config.yaml | 13 ++--- .../DependencyInjection/OtelExtensionTest.php | 20 ++++---- 7 files changed, 76 insertions(+), 44 deletions(-) diff --git a/src/Symfony/OtelBundle/DependencyInjection/Configuration.php b/src/Symfony/OtelBundle/DependencyInjection/Configuration.php index 1e791da6..60705f23 100644 --- a/src/Symfony/OtelBundle/DependencyInjection/Configuration.php +++ b/src/Symfony/OtelBundle/DependencyInjection/Configuration.php @@ -26,6 +26,8 @@ public function getConfigTreeBuilder(): TreeBuilder ->arrayNode('tracing') ->addDefaultsIfNotSet(); + $tracing->children()->append($this->httpTracingNode()); + if (class_exists(HttpKernel::class)) { $tracing->children()->append($this->kernelTracingNode()); } @@ -33,26 +35,45 @@ public function getConfigTreeBuilder(): TreeBuilder return $builder; } + private function httpTracingNode(): NodeDefinition + { + return (new ArrayNodeDefinition('http')) + ->addDefaultsIfNotSet() + ->children() + ->arrayNode('server') + ->addDefaultsIfNotSet() + ->fixXmlConfig('requestHeader') + ->fixXmlConfig('responseHeader') + ->children() + ->arrayNode('requestHeaders') + ->info('Request headers to capture as span attributes.') + ->example(['Content-Type', 'X-Forwarded-For']) + ->beforeNormalization()->castToArray()->end() + ->scalarPrototype()->cannotBeEmpty()->end() + ->end() + ->arrayNode('responseHeaders') + ->info('Response headers to capture as span attributes.') + ->example(['Content-Type']) + ->beforeNormalization()->castToArray()->end() + ->scalarPrototype()->cannotBeEmpty()->end() + ->end() + ->end() + ->end() + ->end() + ; + } + private function kernelTracingNode(): NodeDefinition { - ($kernel = new ArrayNodeDefinition('kernel')) + return (new ArrayNodeDefinition('kernel')) ->addDefaultsIfNotSet() ->canBeDisabled() - ->fixXmlConfig('requestHeader') - ->fixXmlConfig('responseHeader') ->children() - ->booleanNode('extractRemoteContext')->defaultTrue()->end() - ->arrayNode('requestHeaders') - ->beforeNormalization()->castToArray()->end() - ->scalarPrototype()->cannotBeEmpty()->end() - ->end() - ->arrayNode('responseHeaders') - ->beforeNormalization()->castToArray()->end() - ->scalarPrototype()->cannotBeEmpty()->end() + ->booleanNode('extractRemoteContext') + ->info('Set to `false` if the kernel runs in a runtime that extracts the remote context before passing the request to the kernel.') + ->defaultTrue() ->end() ->end() ; - - return $kernel; } } diff --git a/src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php b/src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php index 27f98807..fe12c94b 100644 --- a/src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php +++ b/src/Symfony/OtelBundle/DependencyInjection/OtelExtension.php @@ -22,6 +22,9 @@ public function load(array $configs, ContainerBuilder $container) $loader = new PhpFileLoader($container, new FileLocator()); $loader->load(__DIR__ . '/../Resources/services.php'); + $container->setParameter('otel.tracing.http.server.request_headers', $config['tracing']['http']['server']['requestHeaders']); + $container->setParameter('otel.tracing.http.server.response_headers', $config['tracing']['http']['server']['responseHeaders']); + if ($config['tracing']['kernel']['enabled'] ?? false) { $loader->load(__DIR__ . '/../Resources/services_kernel.php'); $this->loadKernelTracing($config['tracing']['kernel'], $container); @@ -30,11 +33,6 @@ public function load(array $configs, ContainerBuilder $container) private function loadKernelTracing(array $config, ContainerBuilder $container): void { - $container->getDefinition(RequestListener::class) - ->setArgument('$requestHeaders', $config['requestHeaders']) - ->setArgument('$responseHeaders', $config['responseHeaders']) - ; - if (!$config['extractRemoteContext']) { $container->getDefinition(RequestListener::class) ->setArgument('$propagator', new Reference(NoopTextMapPropagator::class)) diff --git a/src/Symfony/OtelBundle/Resources/services_kernel.php b/src/Symfony/OtelBundle/Resources/services_kernel.php index 39422ea3..19c4a017 100644 --- a/src/Symfony/OtelBundle/Resources/services_kernel.php +++ b/src/Symfony/OtelBundle/Resources/services_kernel.php @@ -13,5 +13,7 @@ ->autoconfigure() ->arg('$tracerProvider', service(TracerProviderInterface::class)) ->arg('$propagator', service(TextMapPropagatorInterface::class)) + ->arg('$requestHeaders', param('otel.tracing.http.server.request_headers')) + ->arg('$responseHeaders', param('otel.tracing.http.server.response_headers')) ; }; diff --git a/tests/Unit/Symfony/OtelBundle/DependencyInjection/ConfigurationTest.php b/tests/Unit/Symfony/OtelBundle/DependencyInjection/ConfigurationTest.php index 46822b01..60630bc1 100644 --- a/tests/Unit/Symfony/OtelBundle/DependencyInjection/ConfigurationTest.php +++ b/tests/Unit/Symfony/OtelBundle/DependencyInjection/ConfigurationTest.php @@ -32,16 +32,20 @@ public function testConfiguration(array $sources): void { $expectedConfiguration = [ 'tracing' => [ + 'http' => [ + 'server' => [ + 'requestHeaders' => [ + 'Content-Length', + 'date', + ], + 'responseHeaders' => [ + 'link', + ], + ], + ], 'kernel' => [ 'enabled' => true, 'extractRemoteContext' => true, - 'requestHeaders' => [ - 'Content-Length', - 'date', - ], - 'responseHeaders' => [ - 'link', - ], ], ], ]; diff --git a/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.xml b/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.xml index b230e9ca..c0cb9761 100644 --- a/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.xml +++ b/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.xml @@ -1,11 +1,13 @@ - - Content-Length - date - link - + + + Content-Length + date + link + + diff --git a/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.yaml b/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.yaml index fecff0f0..0b43cb95 100644 --- a/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.yaml +++ b/tests/Unit/Symfony/OtelBundle/DependencyInjection/Fixtures/config.yaml @@ -1,8 +1,9 @@ otel: tracing: - kernel: - requestHeaders: - - Content-Length - - date - responseHeaders: - - link + http: + server: + requestHeaders: + - Content-Length + - date + responseHeaders: + - link diff --git a/tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php b/tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php index bf249e41..6cbe10e3 100644 --- a/tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php +++ b/tests/Unit/Symfony/OtelBundle/DependencyInjection/OtelExtensionTest.php @@ -80,27 +80,31 @@ public function testRequestHeadersSetsHeadersToExtract(): void { $this->load([ 'tracing' => [ - 'kernel' => [ - 'requestHeaders' => ['a', 'b'], + 'http' => [ + 'server' => [ + 'requestHeaders' => ['a', 'b'], + ], ], ], ]); - $this->assertContainerBuilderHasServiceDefinitionWithArgument(RequestListener::class, '$requestHeaders', ['a', 'b']); - $this->assertContainerBuilderHasServiceDefinitionWithArgument(RequestListener::class, '$responseHeaders', []); + $this->assertContainerBuilderHasParameter('otel.tracing.http.server.request_headers', ['a', 'b']); + $this->assertContainerBuilderHasParameter('otel.tracing.http.server.response_headers', []); } public function testResponseHeadersSetsHeadersToExtract(): void { $this->load([ 'tracing' => [ - 'kernel' => [ - 'responseHeaders' => ['a', 'b'], + 'http' => [ + 'server' => [ + 'responseHeaders' => ['a', 'b'], + ], ], ], ]); - $this->assertContainerBuilderHasServiceDefinitionWithArgument(RequestListener::class, '$requestHeaders', []); - $this->assertContainerBuilderHasServiceDefinitionWithArgument(RequestListener::class, '$responseHeaders', ['a', 'b']); + $this->assertContainerBuilderHasParameter('otel.tracing.http.server.request_headers', []); + $this->assertContainerBuilderHasParameter('otel.tracing.http.server.response_headers', ['a', 'b']); } } From aba8ce3388042ee057f7cf9a63c57405cb99899c Mon Sep 17 00:00:00 2001 From: Tobias Bachert Date: Sat, 20 Aug 2022 14:25:24 +0200 Subject: [PATCH 6/8] Create `TracerProviderInterface` service aliases --- src/Symfony/OtelSdkBundle/Resources/config/sdk.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Symfony/OtelSdkBundle/Resources/config/sdk.php b/src/Symfony/OtelSdkBundle/Resources/config/sdk.php index 1bb3ffbe..2233eb94 100644 --- a/src/Symfony/OtelSdkBundle/Resources/config/sdk.php +++ b/src/Symfony/OtelSdkBundle/Resources/config/sdk.php @@ -4,6 +4,8 @@ namespace OpenTelemetry\Symfony\OtelSdkBundle\Resources; +use OpenTelemetry\API; +use OpenTelemetry\SDK; use OpenTelemetry\SDK\Common\Attribute\Attributes; use OpenTelemetry\SDK\Common\Time\SystemClock; use OpenTelemetry\SDK\Resource; @@ -113,6 +115,9 @@ ConfigHelper::createReferenceFromClass(Trace\RandomIdGenerator::class), ]); + $containerConfigurator->services()->alias(API\Trace\TracerProviderInterface::class, Trace\TracerProvider::class); + $containerConfigurator->services()->alias(SDK\Trace\TracerProviderInterface::class, Trace\TracerProvider::class); + $helper->setService(Trace\Tracer::class) ->factory([ ConfigHelper::createReferenceFromClass(Trace\TracerProvider::class), From 023962ffcc548c97b062d3d21802894428412f52 Mon Sep 17 00:00:00 2001 From: Tobias Bachert Date: Sat, 20 Aug 2022 14:29:58 +0200 Subject: [PATCH 7/8] Remove default `Tracer` --- .../DependencyInjection/Tracer.php | 10 ---------- .../OtelSdkBundle/Resources/config/sdk.php | 8 -------- .../Symfony/OtelSdkBundle/OtelSdkBundleTest.php | 17 ----------------- 3 files changed, 35 deletions(-) delete mode 100644 src/Symfony/OtelSdkBundle/DependencyInjection/Tracer.php diff --git a/src/Symfony/OtelSdkBundle/DependencyInjection/Tracer.php b/src/Symfony/OtelSdkBundle/DependencyInjection/Tracer.php deleted file mode 100644 index 0b20ebc3..00000000 --- a/src/Symfony/OtelSdkBundle/DependencyInjection/Tracer.php +++ /dev/null @@ -1,10 +0,0 @@ -services()->alias(API\Trace\TracerProviderInterface::class, Trace\TracerProvider::class); $containerConfigurator->services()->alias(SDK\Trace\TracerProviderInterface::class, Trace\TracerProvider::class); - $helper->setService(Trace\Tracer::class) - ->factory([ - ConfigHelper::createReferenceFromClass(Trace\TracerProvider::class), - 'getTracer', - ]) - ->args([Tracer::DEFAULT_KEY]); - /** * @codeCoverageIgnoreEnd */ diff --git a/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php b/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php index a8c55713..61eea3d5 100644 --- a/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php +++ b/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php @@ -94,23 +94,6 @@ public function testTracerProviderWithSimpleConfig(): void ); } - /** - * @test - * @depends testTracerProviderWithSimpleConfig - * @throws Exception - */ - public function testTracerWithSimpleConfig(): void - { - $this->loadTestData('simple'); - - $this->assertInstanceOf( - SDK\Trace\Tracer::class, - $this->get( - ServiceHelper::classToId(SDK\Trace\Tracer::class) - ) - ); - } - /** * @test * @depends testTracerProviderWithSimpleConfig From 572d59203bf3e046ded135e58b6c989db0d75339 Mon Sep 17 00:00:00 2001 From: Tobias Bachert Date: Thu, 25 Aug 2022 19:30:19 +0200 Subject: [PATCH 8/8] Bump otel version to 0.0.15 --- composer.json | 2 +- tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php | 3 --- .../Symfony/OtelSdkBundle/Resources/SdkConfigTest.php | 2 -- 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/composer.json b/composer.json index cd26fecc..c8f1e4bf 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,7 @@ "require": { "php": "^7.4 || ^8.0", "ext-json": "*", - "open-telemetry/opentelemetry": "^0.0.14", + "open-telemetry/opentelemetry": "^0.0.15", "php-http/discovery": "^1.14", "php-http/message": "^1.12" }, diff --git a/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php b/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php index 61eea3d5..975fa796 100644 --- a/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php +++ b/tests/Integration/Symfony/OtelSdkBundle/OtelSdkBundleTest.php @@ -194,9 +194,6 @@ public function testTracingWithSimpleConfig(): void */ public function testTracingWithAlwaysOffSampler(): void { - $this->markTestSkipped('Requires BatchSpanProcessor update to not force-flush empty batch.'); - - /** @phpstan-ignore-next-line */ $this->load( self::wrapConfig([ 'resource' => [ diff --git a/tests/Integration/Symfony/OtelSdkBundle/Resources/SdkConfigTest.php b/tests/Integration/Symfony/OtelSdkBundle/Resources/SdkConfigTest.php index 7b4a43c1..5ea804a2 100644 --- a/tests/Integration/Symfony/OtelSdkBundle/Resources/SdkConfigTest.php +++ b/tests/Integration/Symfony/OtelSdkBundle/Resources/SdkConfigTest.php @@ -97,8 +97,6 @@ public function testSpan() { $this->assertServiceSetup(Trace\SpanLimitsBuilder::class); $this->assertServiceSetup(Trace\SpanLimits::class); - $this->assertServiceSetup(SpanProcessor\SimpleSpanProcessor::class); - $this->assertServiceSetup(SpanProcessor\BatchSpanProcessor::class); $this->assertServiceSetup(SpanProcessor\NoopSpanProcessor::class); $this->assertServiceSetup(SpanProcessor\MultiSpanProcessor::class); }