Skip to content

Commit 700f005

Browse files
xdavidwuxizhen wang
andauthored
laravel: improve events watcher registration (#166)
The original code hooks on ServiceProvider::{__construct,boot}. Hooking on ServiceProvider is a bad idea since it is the parent of all service providers and thus will hook on all of them. `boot` hook may be run multiple times, thus the $watcherInstalled check is needed. Even worse, only a few providers have a `boot` method, making it more unreliable. This improves it by registering the watchers after application bootstrap, via Application::booted() Laravel-native hook. The whole application will be only run once, thus a $watcherInstalled check is not needed. To get the Application object, Kernel::__construct is used. $watcherInstalled in original code is also broken by the fact that it will be reset on ServiceProvider::__construct, which may happen in the middle of service providers bootstrapping process, via Laravel "deferred" provider registration, which is, when booting a provider, it requests service from another provider that is not registered yet. This may lead to the watchers being registered multiple times as seen in open-telemetry/opentelemetry-php#1032. Fixes: open-telemetry/opentelemetry-php#1032 Co-authored-by: xizhen wang <[email protected]>
1 parent 9c0a97e commit 700f005

File tree

1 file changed

+10
-22
lines changed

1 file changed

+10
-22
lines changed

src/LaravelInstrumentation.php

Lines changed: 10 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
use Illuminate\Foundation\Application;
88
use Illuminate\Foundation\Http\Kernel;
99
use Illuminate\Http\Request;
10-
use Illuminate\Support\ServiceProvider;
1110
use OpenTelemetry\API\Common\Instrumentation\CachedInstrumentation;
1211
use OpenTelemetry\API\Common\Instrumentation\Globals;
1312
use OpenTelemetry\API\Trace\Span;
@@ -23,8 +22,6 @@
2322
class LaravelInstrumentation
2423
{
2524
public const NAME = 'laravel';
26-
private static $watchersInstalled = false;
27-
private static $application;
2825

2926
public static function registerWatchers(Application $app, Watcher $watcher)
3027
{
@@ -96,26 +93,17 @@ public static function register(): void
9693
}
9794
);
9895
hook(
99-
ServiceProvider::class,
100-
'boot',
101-
pre: static function (ServiceProvider $provider, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) {
102-
if (!self::$watchersInstalled) {
103-
self::registerWatchers(self::$application, new ClientRequestWatcher($instrumentation));
104-
self::registerWatchers(self::$application, new ExceptionWatcher());
105-
self::registerWatchers(self::$application, new CacheWatcher());
106-
self::registerWatchers(self::$application, new LogWatcher());
107-
self::registerWatchers(self::$application, new QueryWatcher($instrumentation));
108-
self::$watchersInstalled = true;
109-
}
110-
},
111-
post: null
112-
);
113-
hook(
114-
ServiceProvider::class,
96+
Kernel::class,
11597
'__construct',
116-
pre: static function (ServiceProvider $provider, array $params, string $class, string $function, ?string $filename, ?int $lineno) {
117-
self::$watchersInstalled = false;
118-
self::$application = $params[0];
98+
pre: static function (Kernel $kernel, array $params, string $class, string $function, ?string $filename, ?int $lineno) use ($instrumentation) {
99+
$app = $params[0];
100+
$app->booted(static function (Application $app) use ($instrumentation) {
101+
self::registerWatchers($app, new ClientRequestWatcher($instrumentation));
102+
self::registerWatchers($app, new ExceptionWatcher());
103+
self::registerWatchers($app, new CacheWatcher());
104+
self::registerWatchers($app, new LogWatcher());
105+
self::registerWatchers($app, new QueryWatcher($instrumentation));
106+
});
119107
},
120108
post: null
121109
);

0 commit comments

Comments
 (0)