make span limits configurable

worldia · Jan 14, 2025 · 2d78e99 · 2d78e99
1 parent cd8aed8
commit 2d78e99
Show file tree

Hide file tree

Showing 8 changed files with 80 additions and 36 deletions.
diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
@@ -77,6 +77,10 @@ public function getConfigTreeBuilder(): TreeBuilder
                                                 'accept-encoding',
                                             ])
                                         ->end()
+                                        ->booleanNode('user')
+                                            ->info('Whether to add the `user.id` and `user.roles` attributes')
+                                            ->defaultTrue()
+                                        ->end()
                                     ->end()
                                 ->end()
                                 ->arrayNode('blacklist')
@@ -134,14 +138,19 @@ public function getConfigTreeBuilder(): TreeBuilder
                                     ->info('Whether trace context should be propagated by default for outgoing requests')
                                     ->defaultTrue()
                                 ->end()
-                                ->arrayNode('request_headers')
-                                    ->info('Outgoing request headers to add as span attributes')
-                                    ->defaultValue([])
-                                    ->scalarPrototype()->end()
-                                    ->example([
-                                        'accept',
-                                        'accept-encoding',
-                                    ])
+                                ->arrayNode('attributes')
+                                    ->addDefaultsIfNotSet()
+                                    ->children()
+                                        ->arrayNode('request_headers')
+                                            ->info('Outgoing request headers to add as span attributes')
+                                            ->defaultValue([])
+                                            ->scalarPrototype()->end()
+                                            ->example([
+                                                'accept',
+                                                'accept-encoding',
+                                            ])
+                                        ->end()
+                                    ->end()
                                 ->end()
                             ->end()
                         ->end()
@@ -188,14 +197,6 @@ public function getConfigTreeBuilder(): TreeBuilder
                 ->end()
 
             ->end()
-            ->beforeNormalization()
-                ->ifTrue(fn ($v) => false === \array_key_exists(ResourceAttributes::SERVICE_NAME, $v))
-                ->then(function ($v) {
-                    $v['resource'][ResourceAttributes::SERVICE_NAME] = '%env(default:instrumentation.default_service_name:OTEL_SERVICE_NAME)%';
-
-                    return $v;
-                })
-            ->end()
         ->end();
 
         return $treeBuilder;

diff --git a/src/DependencyInjection/Extension.php b/src/DependencyInjection/Extension.php
@@ -13,6 +13,8 @@
 use Instrumentation\Tracing\Bridge\TraceUrlGeneratorInterface;
 use Instrumentation\Tracing\Doctrine\Instrumentation\DBAL\Middleware as InstrumentationMiddleware;
 use Instrumentation\Tracing\Doctrine\Propagation\DBAL\Middleware as PropagationMiddleware;
+use Instrumentation\Tracing\Request\EventListener\AddUserEventSubscriber;
+use OpenTelemetry\SDK\Trace\SpanLimitsBuilder;
 use Symfony\Bundle\MonologBundle\MonologBundle;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
@@ -184,6 +186,12 @@ protected function loadTracing(array $config, ContainerBuilder $container): void
         $container->setParameter('tracing.doctrine.log_queries', $config['doctrine']['log_queries']);
         $container->setParameter('tracing.doctrine.propagation', $config['doctrine']['propagation']);
         $container->setParameter('tracing.doctrine.instrumentation', $config['doctrine']['instrumentation']);
+
+        if (!$config['request']['attributes']['user']) {
+            $container->removeDefinition(AddUserEventSubscriber::class);
+        } else {
+            $container->getDefinition(SpanLimitsBuilder::class)->addMethodCall('retainGeneralIdentityAttributes');
+        }
     }
 
     /**

diff --git a/src/DependencyInjection/config/tracing/request.php b/src/DependencyInjection/config/tracing/request.php
@@ -37,6 +37,7 @@
             param('tracing.request.methods'),
         ])
         ->autoconfigure()
+
         ->set(Sampling\EventListener\RequestEventSubscriber::class)
         ->args([
             service(Sampling\TogglableSampler::class),
@@ -46,8 +47,8 @@
 
         ->set(Tracing\Request\EventListener\AddUserEventSubscriber::class)
         ->args([
-            service(MainSpanContextInterface::class),
             service(TokenStorageInterface::class)->nullOnInvalid(),
         ])
-        ->autoconfigure();
+        ->autoconfigure()
+    ;
 };
diff --git a/src/DependencyInjection/config/tracing/tracing.php b/src/DependencyInjection/config/tracing/tracing.php
@@ -15,13 +15,19 @@
 use Instrumentation\Tracing\Bridge\TraceUrlGeneratorInterface;
 use Instrumentation\Tracing\Bridge\Twig\Extension\TracingExtension;
 use OpenTelemetry\API\Trace\TracerProviderInterface;
+use OpenTelemetry\SDK\Common\Attribute\AttributesFactory;
+use OpenTelemetry\SDK\Common\Attribute\AttributesFactoryInterface;
+use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeFactory;
+use OpenTelemetry\SDK\Common\Instrumentation\InstrumentationScopeFactoryInterface;
 use OpenTelemetry\SDK\Resource\ResourceInfo;
 use OpenTelemetry\SDK\Trace\ExporterFactory;
 use OpenTelemetry\SDK\Trace\IdGeneratorInterface;
 use OpenTelemetry\SDK\Trace\RandomIdGenerator;
 use OpenTelemetry\SDK\Trace\SamplerFactory;
 use OpenTelemetry\SDK\Trace\SamplerInterface;
 use OpenTelemetry\SDK\Trace\SpanExporterInterface;
+use OpenTelemetry\SDK\Trace\SpanLimits;
+use OpenTelemetry\SDK\Trace\SpanLimitsBuilder;
 use OpenTelemetry\SDK\Trace\SpanProcessorFactory;
 use OpenTelemetry\SDK\Trace\SpanProcessorInterface;
 use OpenTelemetry\SDK\Trace\TracerProvider;
@@ -66,15 +72,30 @@
         ->factory([service(SpanProcessorFactory::class), 'create'])
         ->args([service(SpanExporterInterface::class)])
 
+        ->set(SpanLimitsBuilder::class)
+
+        ->set(SpanLimits::class)
+        ->factory([service(SpanLimitsBuilder::class), 'build'])
+
+        ->set(AttributesFactoryInterface::class)
+        ->class(AttributesFactory::class)
+
+        ->set(InstrumentationScopeFactoryInterface::class)
+        ->class(InstrumentationScopeFactory::class)
+        ->args([
+            service(AttributesFactoryInterface::class),
+        ])
+
         ->set(TracerProviderFactory::class)
 
         ->set(TracerProviderInterface::class, TracerProvider::class)
         ->args([
             [service(SpanProcessorInterface::class)],
             service(SamplerInterface::class),
             service(ResourceInfo::class),
-            null,
+            service(SpanLimits::class),
             service(IdGeneratorInterface::class),
+            service(InstrumentationScopeFactoryInterface::class),
         ])
         ->public()
 

diff --git a/src/InstrumentationBundle.php b/src/InstrumentationBundle.php
@@ -28,7 +28,16 @@ public function boot(): void
             return;
         }
 
-        $this->container->get(Logging\Logging::class);
+        if ($this->container->has(Logging\Logging::class)) {
+            $this->container->get(Logging\Logging::class);
+
+            /** @var LoggerProviderInterface $loggerProvider */
+            $loggerProvider = $this->container->get(LoggerProviderInterface::class);
+
+            if (method_exists($loggerProvider, 'shutdown')) {
+                ShutdownHandler::register([$loggerProvider, 'shutdown']);
+            }
+        }
 
         /** @var TracerProviderInterface $tracerProvider */
         $tracerProvider = $this->container->get(TracerProviderInterface::class);
@@ -38,13 +47,6 @@ public function boot(): void
             ShutdownHandler::register([$tracerProvider, 'shutdown']);
         }
 
-        /** @var LoggerProviderInterface $loggerProvider */
-        $loggerProvider = $this->container->get(LoggerProviderInterface::class);
-
-        if (method_exists($loggerProvider, 'shutdown')) {
-            ShutdownHandler::register([$loggerProvider, 'shutdown']);
-        }
-
         /** @var MeterProviderInterface $meterProvider */
         $meterProvider = $this->container->get(MeterProviderInterface::class);
 

diff --git a/src/Semantics/ResourceInfoProvider.php b/src/Semantics/ResourceInfoProvider.php
@@ -15,7 +15,7 @@
 
 class ResourceInfoProvider implements ResourceInfoProviderInterface
 {
-    private ResourceInfo|null $info = null;
+    private ?ResourceInfo $info = null;
 
     /**
      * @param array<string,string> $attributes

diff --git a/src/Tracing/HttpClient/TracedResponse.php b/src/Tracing/HttpClient/TracedResponse.php
@@ -9,6 +9,7 @@
 
 namespace Instrumentation\Tracing\HttpClient;
 
+use Instrumentation\Logging\Logging;
 use OpenTelemetry\API\Trace\SpanInterface;
 use Symfony\Component\HttpClient\Exception\ClientException;
 use Symfony\Component\HttpClient\Exception\RedirectionException;
@@ -21,7 +22,7 @@
 
 class TracedResponse implements ResponseInterface, StreamableInterface
 {
-    private string|null $content = null;
+    private ?string $content = null;
     /** @var resource|null */
     private $stream;
 
@@ -87,7 +88,7 @@ public function cancel(): void
         }
     }
 
-    public function getInfo(string|null $type = null): mixed
+    public function getInfo(?string $type = null): mixed
     {
         return $this->response->getInfo($type);
     }
@@ -116,7 +117,7 @@ public function toStream(bool $throw = true)
      *
      * @internal
      */
-    public static function stream(HttpClientInterface $client, iterable $responses, float|null $timeout): \Generator
+    public static function stream(HttpClientInterface $client, iterable $responses, ?float $timeout): \Generator
     {
         $wrappedResponses = [];
         $traceableMap = new \SplObjectStorage();
@@ -166,7 +167,8 @@ protected function endTracing(): void
 
                 \call_user_func($info['user_data']['on_response'], $this->getHeaders(false), $stream, $this->span);
             }
-        } catch (\Throwable) {
+        } catch (\Throwable $e) {
+            Logging::getLogger()->error('There was an error during the response callback.', ['exception' => $e]);
         }
 
         $this->span->end($endEpochNanos);

diff --git a/src/Tracing/Request/EventListener/AddUserEventSubscriber.php b/src/Tracing/Request/EventListener/AddUserEventSubscriber.php
@@ -9,8 +9,8 @@
 
 namespace Instrumentation\Tracing\Request\EventListener;
 
-use Instrumentation\Tracing\Bridge\MainSpanContextInterface;
 use Instrumentation\Tracing\TracerAwareTrait;
+use OpenTelemetry\API\Trace\LocalRootSpan;
 use OpenTelemetry\SemConv\TraceAttributes;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
@@ -19,6 +19,15 @@
 use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
 use Symfony\Component\Security\Core\User\UserInterface;
 
+/**
+ * Adds the `user.id` and `user.roles`attributes.
+ *
+ * @see https://opentelemetry.io/docs/specs/semconv/attributes-registry/user/
+ *
+ * Caution: Enabling this subscriber is not enough because they are dropped by default by
+ * the `open-telemetry/sdk` (causing 'Dropped span attributes, links or events' warnings).
+ * @see https://github.com/open-telemetry/opentelemetry-php/blob/83cddd9157438e7a72b7824708be36298c8e589f/src/SDK/Trace/SpanLimitsBuilder.php#L134.
+ */
 final class AddUserEventSubscriber implements EventSubscriberInterface
 {
     use TracerAwareTrait;
@@ -30,7 +39,7 @@ public static function getSubscribedEvents(): array
         ];
     }
 
-    public function __construct(private MainSpanContextInterface $mainSpanContext, private TokenStorageInterface|null $tokenStorage = null)
+    public function __construct(private readonly ?TokenStorageInterface $tokenStorage = null)
     {
     }
 
@@ -47,7 +56,7 @@ public function onRequestEvent(RequestEvent $event): void
         }
 
         if ($token && $this->isTokenAuthenticated($token)) {
-            $span = $this->mainSpanContext->getMainSpan();
+            $span = LocalRootSpan::current();
             $user = $token->getUser();
             if ($user) {
                 $span->setAttribute(TraceAttributes::USER_ID, $this->getUsername($user));
@@ -68,7 +77,7 @@ private function getRoles(UserInterface|\Stringable|string $user): array
         return [];
     }
 
-    private function getUsername(UserInterface|\Stringable|string $user): string|null
+    private function getUsername(UserInterface|\Stringable|string $user): ?string
     {
         if ($user instanceof UserInterface) {
             if (method_exists($user, 'getUserIdentifier')) {