Skip to content

Commit 5807825

Browse files
authored
fix(flow-php/symfony-telemetry-bundle): stop calling Router::getRouteCollection() on every request (#2525)
fix(flow-php/symfony-telemetry-bundle): stop calling Router::getRouteCollection() on every request with route_naming: pat - add RouteNamePathMap: ConfigCache-backed [route name => path] map, warmed at cache:warmup (optional warmer) or lazily on first use - HttpKernelSpanSubscriber takes ?RouteNamePathMap instead of ?RouterInterface; routeValue() is an O(1) lookup - degrade to route-name naming (never touch the router at runtime) when the map cannot be built
1 parent 2399f25 commit 5807825

8 files changed

Lines changed: 406 additions & 6 deletions

File tree

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1151,7 +1151,9 @@ The request (SERVER) span follows the OpenTelemetry HTTP semantic conventions fo
11511151
controlled by `route_naming`:
11521152

11531153
- `path` (default) — the route **path template**, e.g. `GET /orders/{id}` (low cardinality, semconv value
1154-
for `http.route`); resolved from the router.
1154+
for `http.route`); resolved from a `[route name => path]` map built once per deployment by an optional
1155+
cache warmer (`cache:warmup`) and rebuilt lazily when missing, so the router is never queried on the
1156+
request path.
11551157
- `name` — the Symfony **route name**, e.g. `GET order_show`.
11561158
- Sub-requests (`render(controller(...))`) have no route, so they are named after the **controller**
11571159
(`GET App\Controller\NavigationController::top`); a request that matches no route at all uses the method

documentation/upgrading.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,16 @@ implementation is `yield $this->get($key);`.
294294

295295
Convert `DateTime`/`DateTimeImmutable` subclasses to `DateTime`/`DateTimeImmutable` before caching or serializing.
296296

297+
### 21) `flow-php/symfony-telemetry-bundle` - `HttpKernelSpanSubscriber` takes a
298+
`RouteNamePathMap` instead of the router
299+
300+
| Before | After |
301+
|-------------------------------------------------------|---------------------------------------------------------------------|
302+
| `new HttpKernelSpanSubscriber(…, router: $router, …)` | `new HttpKernelSpanSubscriber(…, routePaths: $routeNamePathMap, …)` |
303+
| `?RouterInterface $router = null` | `?RouteNamePathMap $routePaths = null` |
304+
305+
Applies only to direct construction; services wired by the bundle need no change.
306+
297307
---
298308

299309
## Upgrading from 0.40.x to 0.41.x

src/bridge/symfony/telemetry-bundle/src/Flow/Bridge/Symfony/TelemetryBundle/Instrumentation/HttpKernel/HttpKernelSpanSubscriber.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
use Symfony\Component\HttpKernel\Event\ResponseEvent;
3131
use Symfony\Component\HttpKernel\Event\TerminateEvent;
3232
use Symfony\Component\HttpKernel\KernelEvents;
33-
use Symfony\Component\Routing\RouterInterface;
3433

3534
use function array_key_exists;
3635
use function array_map;
@@ -59,7 +58,7 @@ public function __construct(
5958
private Propagator $propagator,
6059
private bool $contextPropagation = true,
6160
private bool $contextPropagationQuery = false,
62-
private ?RouterInterface $router = null,
61+
private ?RouteNamePathMap $routePaths = null,
6362
private RouteNaming $routeNaming = RouteNaming::Path,
6463
) {
6564
$this->excludePathRules = array_map(
@@ -98,11 +97,11 @@ public function onController(ControllerEvent $event): void
9897

9998
private function routeValue(string $routeName): string
10099
{
101-
if ($this->routeNaming !== RouteNaming::Path || $this->router === null) {
100+
if ($this->routeNaming !== RouteNaming::Path) {
102101
return $routeName;
103102
}
104103

105-
return $this->router->getRouteCollection()->get($routeName)?->getPath() ?? $routeName;
104+
return $this->routePaths?->pathFor($routeName) ?? $routeName;
106105
}
107106

108107
public function onException(ExceptionEvent $event): void
Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Flow\Bridge\Symfony\TelemetryBundle\Instrumentation\HttpKernel;
6+
7+
use Symfony\Component\Config\ConfigCache;
8+
use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface;
9+
use Symfony\Component\Routing\RouterInterface;
10+
use Throwable;
11+
12+
use function dirname;
13+
use function is_dir;
14+
use function is_writable;
15+
use function var_export;
16+
17+
/**
18+
* Deployment-static [route name => path template] map so request spans can carry the OTEL http.route
19+
* path template without touching the router at runtime — Router::getRouteCollection() bypasses the
20+
* compiled matcher and rebuilds the full route collection, which Symfony explicitly warns is too slow
21+
* for the request path.
22+
*/
23+
final class RouteNamePathMap implements CacheWarmerInterface
24+
{
25+
private ConfigCache $cache;
26+
27+
/** @var null|array<string, string> */
28+
private ?array $paths = null;
29+
30+
private bool $unavailable = false;
31+
32+
public function __construct(
33+
private readonly ?RouterInterface $router,
34+
string $directory,
35+
bool $debug,
36+
) {
37+
$this->cache = new ConfigCache($directory . '/flow_telemetry_route_paths.php', $debug);
38+
}
39+
40+
public function isOptional(): bool
41+
{
42+
return true;
43+
}
44+
45+
public function pathFor(string $routeName): ?string
46+
{
47+
if ($this->paths === null && !$this->load()) {
48+
return null;
49+
}
50+
51+
return $this->paths[$routeName] ?? null;
52+
}
53+
54+
public function warmUp(string $cacheDir, ?string $buildDir = null): array
55+
{
56+
if ($this->router === null) {
57+
return [];
58+
}
59+
60+
$collection = $this->router->getRouteCollection();
61+
$paths = [];
62+
63+
foreach ($collection->all() as $name => $route) {
64+
$paths[$name] = $route->getPath();
65+
}
66+
67+
$this->cache->write('<?php return ' . var_export($paths, true) . ';', $collection->getResources());
68+
$this->paths = $paths;
69+
70+
return [$this->cache->getPath()];
71+
}
72+
73+
private function load(): bool
74+
{
75+
if ($this->unavailable) {
76+
return false;
77+
}
78+
79+
try {
80+
if (!$this->cache->isFresh()) {
81+
$directory = dirname($this->cache->getPath());
82+
83+
// Guard against the unwritable branch before touching the router: under PHP-FPM the
84+
// failure memoization below lives one request only, so a throwing warmUp() would
85+
// rebuild the route collection on every request — the exact cost this map avoids.
86+
if (is_dir($directory) ? !is_writable($directory) : !is_writable(dirname($directory))) {
87+
$this->unavailable = true;
88+
89+
return false;
90+
}
91+
92+
$this->warmUp('');
93+
}
94+
} catch (Throwable) {
95+
$this->unavailable = true;
96+
97+
return false;
98+
}
99+
100+
if ($this->paths === null) {
101+
if (!$this->cache->isFresh()) {
102+
$this->unavailable = true;
103+
104+
return false;
105+
}
106+
107+
/** @var array<string, string> $paths */
108+
$paths = require $this->cache->getPath();
109+
$this->paths = $paths;
110+
}
111+
112+
return true;
113+
}
114+
}

src/bridge/symfony/telemetry-bundle/src/Flow/Bridge/Symfony/TelemetryBundle/Resources/config/instrumentation/http_kernel.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Flow\Bridge\Symfony\TelemetryBundle\Instrumentation\HttpKernel\ControllerSpanSubscriber;
66
use Flow\Bridge\Symfony\TelemetryBundle\Instrumentation\HttpKernel\HttpKernelFlushSubscriber;
77
use Flow\Bridge\Symfony\TelemetryBundle\Instrumentation\HttpKernel\HttpKernelSpanSubscriber;
8+
use Flow\Bridge\Symfony\TelemetryBundle\Instrumentation\HttpKernel\RouteNamePathMap;
89
use Flow\Telemetry\Telemetry;
910
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
1011

@@ -14,6 +15,15 @@
1415
return static function (ContainerConfigurator $container): void {
1516
$services = $container->services();
1617

18+
$services
19+
->set('flow.telemetry.http_kernel.route_name_path_map', RouteNamePathMap::class)
20+
->args([
21+
service('router')->ignoreOnInvalid(),
22+
'%kernel.build_dir%',
23+
'%kernel.debug%',
24+
])
25+
->tag('kernel.cache_warmer');
26+
1727
$services
1828
->set('flow.telemetry.http_kernel.span_subscriber', HttpKernelSpanSubscriber::class)
1929
->args([
@@ -23,7 +33,7 @@
2333
service('flow.telemetry.propagator'),
2434
'%flow.telemetry.http_kernel.context_propagation%',
2535
'%flow.telemetry.http_kernel.context_propagation_query%',
26-
service('router')->ignoreOnInvalid(),
36+
service('flow.telemetry.http_kernel.route_name_path_map'),
2737
// arg $routeNaming (RouteNaming enum) is set in FlowTelemetryBundle::registerInstrumentation.
2838
])
2939
->tag('kernel.event_subscriber');
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Flow\Bridge\Symfony\TelemetryBundle\Tests\Context;
6+
7+
use FilesystemIterator;
8+
use RecursiveDirectoryIterator;
9+
use RecursiveIteratorIterator;
10+
11+
use function bin2hex;
12+
use function chmod;
13+
use function is_dir;
14+
use function mkdir;
15+
use function random_bytes;
16+
use function rmdir;
17+
use function sys_get_temp_dir;
18+
use function unlink;
19+
20+
final class TempDirectoryContext
21+
{
22+
public static function remove(string $directory): void
23+
{
24+
if (!is_dir($directory)) {
25+
return;
26+
}
27+
28+
// The read-only degradation tests leave directories without write permission behind.
29+
chmod($directory, 0o755);
30+
31+
/** @var iterable<\SplFileInfo> $files */
32+
$files = new RecursiveIteratorIterator(
33+
new RecursiveDirectoryIterator($directory, FilesystemIterator::SKIP_DOTS),
34+
RecursiveIteratorIterator::CHILD_FIRST,
35+
);
36+
37+
foreach ($files as $file) {
38+
if ($file->isDir()) {
39+
chmod($file->getPathname(), 0o755);
40+
rmdir($file->getPathname());
41+
} else {
42+
unlink($file->getPathname());
43+
}
44+
}
45+
46+
rmdir($directory);
47+
}
48+
49+
/**
50+
* @param callable(string): void $test
51+
*/
52+
public static function with(callable $test): void
53+
{
54+
$directory = sys_get_temp_dir() . '/flow_telemetry_bundle_test_' . bin2hex(random_bytes(8));
55+
mkdir($directory, 0o777, true);
56+
57+
try {
58+
$test($directory);
59+
} finally {
60+
self::remove($directory);
61+
}
62+
}
63+
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Flow\Bridge\Symfony\TelemetryBundle\Tests\Fixtures\Routing;
6+
7+
use Symfony\Component\Routing\RequestContext;
8+
use Symfony\Component\Routing\RouteCollection;
9+
use Symfony\Component\Routing\RouterInterface;
10+
11+
final class FakeRouter implements RouterInterface
12+
{
13+
public int $getRouteCollectionCalls = 0;
14+
15+
private RequestContext $context;
16+
17+
public function __construct(
18+
private RouteCollection $routes = new RouteCollection(),
19+
) {
20+
$this->context = new RequestContext();
21+
}
22+
23+
public function generate(string $name, array $parameters = [], int $referenceType = self::ABSOLUTE_PATH): string
24+
{
25+
return '/generated';
26+
}
27+
28+
public function getContext(): RequestContext
29+
{
30+
return $this->context;
31+
}
32+
33+
public function getRouteCollection(): RouteCollection
34+
{
35+
$this->getRouteCollectionCalls++;
36+
37+
return $this->routes;
38+
}
39+
40+
public function match(string $pathinfo): array
41+
{
42+
return [];
43+
}
44+
45+
public function setContext(RequestContext $context): void
46+
{
47+
$this->context = $context;
48+
}
49+
50+
public function setRouteCollection(RouteCollection $routes): void
51+
{
52+
$this->routes = $routes;
53+
}
54+
}

0 commit comments

Comments
 (0)