fix(flow-php/symfony-telemetry-bundle): resolve definition classes in compiler passes

documentation/components/bridges/symfony-telemetry-bundle.md

@@ -1494,6 +1494,12 @@ flow_telemetry:
       flush_deferred: false   # opt-in; see below
 ```
 
+Every service tagged `cache.pool` is traced, including the framework's own internal pools
+(`cache.system`, `cache.validator`, `cache.serializer`, `cache.property_info`, `cache.app`,
+`cache.doctrine.*`). Use `exclude_pools` to opt any of them out - entries are matched as an exact
+service id or, when the value is a valid regular expression, as a pattern. Before 0.43.0 those pools
+were skipped by a bug; see the [upgrade note](/documentation/upgrading.md#upgrading-from-042x-to-043x).
+
 ##### Deferred writes on a Doctrine DBAL cache pool
 
 A cache pool backed by `cache.adapter.doctrine_dbal` defers writes and flushes them from the pool's own

documentation/upgrading.md

@@ -62,12 +62,12 @@ try {
 
 On `d = 2024-01-01, 2024-01-02, 2024-01-03, 2024-01-04` and `s = 100, 200, 300, 400`:
 
-| Before                                                      | After                    |
-|-------------------------------------------------------------|--------------------------|
-| `sum(ref('s'))->over(window()->orderBy(ref('d')))` → `1000, 1000, 1000, 1000` | `100, 300, 600, 1000`    |
-| `average()`, `count()` over an ordered window - whole partition | rows up to the current row's peers |
-| `window()->partitionBy(ref('dept'))` - whole partition       | unchanged                |
-| empty frame                                                  | `sum()`/`average()` → `null`, `count()` → `0` |
+| Before                                                                        | After                                         |
+|-------------------------------------------------------------------------------|-----------------------------------------------|
+| `sum(ref('s'))->over(window()->orderBy(ref('d')))` → `1000, 1000, 1000, 1000` | `100, 300, 600, 1000`                         |
+| `average()`, `count()` over an ordered window - whole partition               | rows up to the current row's peers            |
+| `window()->partitionBy(ref('dept'))` - whole partition                        | unchanged                                     |
+| empty frame                                                                   | `sum()`/`average()` → `null`, `count()` → `0` |
 
 Restore the previous result:
 
@@ -79,17 +79,17 @@ sum(ref('s'))->over(window()->orderBy(ref('d'))->rowsBetween(unbounded_preceding
 
 On `s = 100, 100, 300` ordered by a distinct column:
 
-| Before                                                                  | After                                  |
-|-------------------------------------------------------------------------|----------------------------------------|
+| Before                                                                    | After                                           |
+|---------------------------------------------------------------------------|-------------------------------------------------|
 | `count(ref('s'))` counts rows sharing the current row's value → `2, 2, 1` | counts non-null values in the frame → `1, 2, 3` |
-| `count()` threw `Count WindowFunction function requires a reference.`     | counts every row in the frame (`COUNT(*)`) |
+| `count()` threw `Count WindowFunction function requires a reference.`     | counts every row in the frame (`COUNT(*)`)      |
 
 ### 5) `flow-php/etl` - `partitionBy()` no longer sets `orderBy()`
 
-| Before                                                                       | After   |
-|------------------------------------------------------------------------------|---------|
-| `window()->orderBy(ref('date'))->partitionBy(ref('dept'))->order()` → `['dept']` | `['date']` |
-| `window()->partitionBy(ref('dept'))->order()` → `['dept']`                     | `[]`    |
+| Before                                                                                                   | After                                             |
+|----------------------------------------------------------------------------------------------------------|---------------------------------------------------|
+| `window()->orderBy(ref('date'))->partitionBy(ref('dept'))->order()` → `['dept']`                         | `['date']`                                        |
+| `window()->partitionBy(ref('dept'))->order()` → `['dept']`                                               | `[]`                                              |
 | `rank()`/`dense_rank()`/`row_number()` over a `partitionBy()`-only window ranked by the partition column | throws `... requires to be ordered by one column` |
 
 Add the ordering explicitly:
@@ -100,15 +100,15 @@ rank()->over(window()->partitionBy(ref('dept'))->orderBy(ref('salary')->desc()))
 
 ### 6) `flow-php/etl` - `WindowFunction::apply()` receives a `WindowContext`
 
-| Before                                                    | After                                            |
-|------------------------------------------------------------|--------------------------------------------------|
-| `apply(Row $row, Rows $partition, FlowContext $context)`    | `apply(WindowContext $window)`                    |
-| `$row`                                                      | `$window->row()`                                  |
-| `$partition`                                                | `$window->partition()`                            |
-| `$context`                                                  | `$window->flowContext()`                          |
-| —                                                           | `$window->frame()` - rows within the current row's frame |
-| —                                                           | `$window->index()` - position in the ordered partition |
-| `row_number()` on duplicate rows → `1, 1, 3`                | `1, 2, 3`                                         |
+| Before                                                   | After                                                    |
+|----------------------------------------------------------|----------------------------------------------------------|
+| `apply(Row $row, Rows $partition, FlowContext $context)` | `apply(WindowContext $window)`                           |
+| `$row`                                                   | `$window->row()`                                         |
+| `$partition`                                             | `$window->partition()`                                   |
+| `$context`                                               | `$window->flowContext()`                                 |
+| —                                                        | `$window->frame()` - rows within the current row's frame |
+| —                                                        | `$window->index()` - position in the ordered partition   |
+| `row_number()` on duplicate rows → `1, 1, 3`             | `1, 2, 3`                                                |
 
 Implementations must no longer sort; `$window->partition()` and `$window->frame()` are already ordered.
 
@@ -127,6 +127,31 @@ Implementations must no longer sort; `$window->partition()` and `$window->frame(
 
 Applies to `flow-php/postgresql` users only through the bundle; `Client\Telemetry` is unchanged.
 
+### 8) `flow-php/symfony-telemetry-bundle` - cache pools and PSR-18 clients that were silently skipped are now traced
+
+| Before                                                                                                                                               | After                                                               |
+|------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------------------------------------------------------|
+| `cache.system`, `cache.validator`, `cache.serializer`, `cache.property_info`, `cache.app`, `cache.doctrine.*`, `cache.http_client.pool` - not traced | traced: `cache.*` spans and `flow.cache.hits` / `flow.cache.misses` |
+| pool or client whose class is a `%parameter%` - not traced                                                                                           | traced                                                              |
+| tag-aware pool whose class is a `%parameter%` - got the non-tag-aware decorator                                                                      | gets `TagAwareTraceableCacheAdapter`                                |
+| PSR-18 client behind an autoconfigured or abstract parent definition - container build failed with *"has a reference to an abstract definition"*     | compiles; the client is traced                                      |
+| `instrumentation.cache.exclude_pools` entries for framework pools - had no effect                                                                    | take effect                                                         |
+
+To keep the previous set of traced pools, exclude the framework's own:
+
+```yaml
+flow_telemetry:
+  instrumentation:
+    cache:
+      exclude_pools:
+        - 'cache.system'
+        - 'cache.validator'
+        - 'cache.serializer'
+        - 'cache.property_info'
+        - '/^cache\.doctrine\..*/'
+        - 'cache.http_client.pool'
+```
+
 ---
 
 ## Upgrading from 0.41.x to 0.42.x
@@ -2378,7 +2403,7 @@ After:
     ->run();
 ```
 
-### 4) ConfigBuilder::putInputIntoRows () output is now prefixed with _    (underscore)
+### 4) ConfigBuilder::putInputIntoRows () output is now prefixed with _     (underscore)
 
 In order to avoid collisions with datasets columns, additional columns created after using putInputIntoRows ()
 would now be prefixed with `_` (underscore) symbol.

src/bridge/symfony/telemetry-bundle/src/Flow/Bridge/Symfony/TelemetryBundle/DependencyInjection/Compiler/CacheTelemetryPass.php

@@ -16,7 +16,6 @@
 use Symfony\Component\DependencyInjection\Reference;
 
 use function is_a;
-use function preg_match;
 
 final class CacheTelemetryPass implements CompilerPassInterface
 {
@@ -35,12 +34,16 @@ public function process(ContainerBuilder $container): void
             ? $container->getParameter('flow.telemetry.cache.exclude_pools')
             : [];
 
+        $excluded = new ServiceIdPatterns($excludePools);
+
+        $resolver = new DefinitionClassResolver($container);
+
         $taggedServices = $container->findTaggedServiceIds('cache.pool');
 
         $innerPools = [];
 
         foreach ($taggedServices as $serviceId => $_tags) {
-            if ($this->isExcluded($serviceId, $excludePools)) {
+            if ($excluded->matches($serviceId)) {
                 continue;
             }
 
@@ -50,7 +53,7 @@ public function process(ContainerBuilder $container): void
                 continue;
             }
 
-            $serviceClass = $serviceDefinition->getClass();
+            $serviceClass = $resolver->resolve($serviceDefinition);
 
             if ($serviceClass === null) {
                 continue;
@@ -91,29 +94,4 @@ private function flushDeferredEnabled(ContainerBuilder $container): bool
             && $container->getParameter('flow.telemetry.cache.flush_deferred') === true
         );
     }
-
-    /**
-     * @param array<string> $patterns
-     */
-    private function isExcluded(string $serviceId, array $patterns): bool
-    {
-        foreach ($patterns as $pattern) {
-            if ($this->matchesPattern($serviceId, $pattern)) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
-    private function matchesPattern(string $serviceId, string $pattern): bool
-    {
-        $result = @preg_match($pattern, $serviceId);
-
-        if ($result !== false) {
-            return (bool) $result;
-        }
-
-        return $serviceId === $pattern;
-    }
 }

src/bridge/symfony/telemetry-bundle/src/Flow/Bridge/Symfony/TelemetryBundle/DependencyInjection/Compiler/DefinitionClassResolver.php

@@ -0,0 +1,52 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Flow\Bridge\Symfony\TelemetryBundle\DependencyInjection\Compiler;
+
+use Symfony\Component\DependencyInjection\ChildDefinition;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException;
+
+use function array_key_exists;
+use function is_string;
+
+final readonly class DefinitionClassResolver
+{
+    public function __construct(
+        private ContainerBuilder $container,
+    ) {}
+
+    /**
+     * @return null|class-string
+     */
+    public function resolve(Definition $definition): ?string
+    {
+        $seen = [];
+
+        while ($definition->getClass() === null && $definition instanceof ChildDefinition) {
+            $parent = $definition->getParent();
+
+            if (array_key_exists($parent, $seen) || !$this->container->has($parent)) {
+                return null;
+            }
+
+            $seen[$parent] = true;
+            $definition = $this->container->findDefinition($parent);
+        }
+
+        try {
+            /** @var mixed $class */
+            $class = $this->container->getParameterBag()->resolveValue($definition->getClass());
+        } catch (ParameterNotFoundException) {
+            return null;
+        }
+
+        if (!is_string($class)) {
+            return null;
+        }
+
+        return $this->container->getReflectionClass($class, false)?->getName();
+    }
+}

src/bridge/symfony/telemetry-bundle/src/Flow/Bridge/Symfony/TelemetryBundle/DependencyInjection/Compiler/HttpClientTelemetryPass.php

@@ -11,8 +11,6 @@
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
 
-use function preg_match;
-
 final class HttpClientTelemetryPass implements CompilerPassInterface
 {
     public function process(ContainerBuilder $container): void
@@ -30,10 +28,12 @@ public function process(ContainerBuilder $container): void
             ? $container->getParameter('flow.telemetry.http_client.exclude_clients')
             : [];
 
+        $excluded = new ServiceIdPatterns($excludeClients);
+
         $taggedServices = $container->findTaggedServiceIds('http_client.client');
 
         foreach ($taggedServices as $serviceId => $_tags) {
-            if ($this->isExcluded($serviceId, $excludeClients)) {
+            if ($excluded->matches($serviceId)) {
                 continue;
             }
 
@@ -49,29 +49,4 @@ public function process(ContainerBuilder $container): void
             $container->setDefinition($decoratorId, $definition);
         }
     }
-
-    /**
-     * @param array<string> $patterns
-     */
-    private function isExcluded(string $serviceId, array $patterns): bool
-    {
-        foreach ($patterns as $pattern) {
-            if ($this->matchesPattern($serviceId, $pattern)) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
-    private function matchesPattern(string $serviceId, string $pattern): bool
-    {
-        $result = @preg_match($pattern, $serviceId);
-
-        if ($result !== false) {
-            return (bool) $result;
-        }
-
-        return $serviceId === $pattern;
-    }
 }

src/bridge/symfony/telemetry-bundle/src/Flow/Bridge/Symfony/TelemetryBundle/DependencyInjection/Compiler/Psr18ClientTelemetryPass.php

@@ -12,9 +12,7 @@
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
 
-use function class_exists;
 use function is_a;
-use function preg_match;
 
 final class Psr18ClientTelemetryPass implements CompilerPassInterface
 {
@@ -33,12 +31,26 @@ public function process(ContainerBuilder $container): void
             ? $container->getParameter('flow.telemetry.psr18_client.exclude_clients')
             : [];
 
+        $excluded = new ServiceIdPatterns($excludeClients);
+
+        $resolver = new DefinitionClassResolver($container);
+
         foreach ($container->getDefinitions() as $serviceId => $definition) {
-            if ($this->isExcluded($serviceId, $excludeClients)) {
+            if ($excluded->matches($serviceId)) {
+                continue;
+            }
+
+            if ($definition->isAbstract()) {
+                continue;
+            }
+
+            $class = $resolver->resolve($definition);
+
+            if ($class === null || $class === PSR18TraceableClient::class) {
                 continue;
             }
 
-            if (!$this->implementsPsr18Interface($definition)) {
+            if (!is_a($class, ClientInterface::class, true)) {
                 continue;
             }
 
@@ -53,48 +65,4 @@ public function process(ContainerBuilder $container): void
             $container->setDefinition($decoratorId, $decoratorDefinition);
         }
     }
-
-    private function implementsPsr18Interface(Definition $definition): bool
-    {
-        $class = $definition->getClass();
-
-        if ($class === null) {
-            return false;
-        }
-
-        if ($class === PSR18TraceableClient::class) {
-            return false;
-        }
-
-        if (!class_exists($class)) {
-            return false;
-        }
-
-        return is_a($class, ClientInterface::class, true);
-    }
-
-    /**
-     * @param array<string> $patterns
-     */
-    private function isExcluded(string $serviceId, array $patterns): bool
-    {
-        foreach ($patterns as $pattern) {
-            if ($this->matchesPattern($serviceId, $pattern)) {
-                return true;
-            }
-        }
-
-        return false;
-    }
-
-    private function matchesPattern(string $serviceId, string $pattern): bool
-    {
-        $result = @preg_match($pattern, $serviceId);
-
-        if ($result !== false) {
-            return (bool) $result;
-        }
-
-        return $serviceId === $pattern;
-    }
 }