From be6cafbcdfbb1d95360db4b168805117ef1b72db Mon Sep 17 00:00:00 2001 From: MilesChou Date: Fri, 6 Sep 2024 01:29:14 +0800 Subject: [PATCH 1/8] Feat: Add Redis watcher --- .../Illuminate/Foundation/Application.php | 2 + .../RedisCommand/RedisCommandWatcher.php | 125 ++++++++++++++++++ .../src/Watchers/RedisCommand/Serializer.php | 74 +++++++++++ .../Watches/RedisCommand/SerializerTest.php | 35 +++++ 4 files changed, 236 insertions(+) create mode 100644 src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php create mode 100644 src/Instrumentation/Laravel/src/Watchers/RedisCommand/Serializer.php create mode 100644 src/Instrumentation/Laravel/tests/Unit/Watches/RedisCommand/SerializerTest.php diff --git a/src/Instrumentation/Laravel/src/Hooks/Illuminate/Foundation/Application.php b/src/Instrumentation/Laravel/src/Hooks/Illuminate/Foundation/Application.php index ebae628c1..54aa70d80 100644 --- a/src/Instrumentation/Laravel/src/Hooks/Illuminate/Foundation/Application.php +++ b/src/Instrumentation/Laravel/src/Hooks/Illuminate/Foundation/Application.php @@ -13,6 +13,7 @@ use OpenTelemetry\Contrib\Instrumentation\Laravel\Watchers\ExceptionWatcher; use OpenTelemetry\Contrib\Instrumentation\Laravel\Watchers\LogWatcher; use OpenTelemetry\Contrib\Instrumentation\Laravel\Watchers\QueryWatcher; +use OpenTelemetry\Contrib\Instrumentation\Laravel\Watchers\RedisCommand\RedisCommandWatcher; use OpenTelemetry\Contrib\Instrumentation\Laravel\Watchers\Watcher; use function OpenTelemetry\Instrumentation\hook; use Throwable; @@ -32,6 +33,7 @@ public function instrument(): void $this->registerWatchers($application, new ExceptionWatcher()); $this->registerWatchers($application, new LogWatcher($this->instrumentation)); $this->registerWatchers($application, new QueryWatcher($this->instrumentation)); + $this->registerWatchers($application, new RedisCommandWatcher($this->instrumentation)); }, ); } diff --git a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php new file mode 100644 index 000000000..3543e8ee9 --- /dev/null +++ b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php @@ -0,0 +1,125 @@ +listen(CommandExecuted::class, [$this, 'recordRedisCommand']); + } + + /** + * Record a query. + */ + /** @psalm-suppress UndefinedThisPropertyFetch */ + public function recordRedisCommand(CommandExecuted $event): void + { + $nowInNs = (int) (microtime(true) * 1E9); + + $operationName = Str::upper($event->command); + + /** @psalm-suppress ArgumentTypeCoercion */ + $span = $this->instrumentation->tracer() + ->spanBuilder($operationName) + ->setSpanKind(SpanKind::KIND_CLIENT) + ->setStartTimestamp($this->calculateQueryStartTime($nowInNs, $event->time)) + ->startSpan(); + + // See https://opentelemetry.io/docs/specs/semconv/database/redis/ + $attributes = [ + TraceAttributes::DB_SYSTEM => TraceAttributeValues::DB_SYSTEM_REDIS, + TraceAttributes::DB_NAME => $this->fetchDbIndex($event->connection), + TraceAttributes::DB_OPERATION => $operationName, + TraceAttributes::DB_QUERY_TEXT => Serializer::serializeCommand($event->command, $event->parameters), + TraceAttributes::SERVER_ADDRESS => $this->fetchDbHost($event->connection), + ]; + + /** @psalm-suppress PossiblyInvalidArgument */ + $span->setAttributes($attributes); + $span->end($nowInNs); + } + + private function calculateQueryStartTime(int $nowInNs, float $queryTimeMs): int + { + return (int) ($nowInNs - ($queryTimeMs * 1E6)); + } + + private function fetchDbIndex(Connection $connection): int + { + if ($connection instanceof PhpRedisConnection) { + $index = $connection->client()->getDbNum(); + + if ($index === false) { + throw new RuntimeException('Cannot fetch database index.'); + } + + return $index; + } elseif ($connection instanceof PredisConnection) { + /** @psalm-suppress PossiblyUndefinedMethod */ + $index = $connection->client()->getConnection()->getParameters()->database; + + if (is_int($index)) { + throw new RuntimeException('Cannot fetch database index.'); + } + + return $index; + } + + throw new RangeException('Unknown Redis connection instance: ' . get_class($connection)); + + } + + private function fetchDbHost(Connection $connection): string + { + if ($connection instanceof PhpRedisConnection) { + $host = $connection->client()->getHost(); + + if ($host === false) { + throw new RuntimeException('Cannot fetch database host.'); + } + + return $host; + } elseif ($connection instanceof PredisConnection) { + /** @psalm-suppress PossiblyUndefinedMethod */ + $host = $connection->client()->getConnection()->getParameters()->host; + + if (is_int($host)) { + throw new RuntimeException('Cannot fetch database index.'); + } + + return $host; + } + + throw new RangeException('Unknown Redis connection instance: ' . get_class($connection)); + + } +} diff --git a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/Serializer.php b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/Serializer.php new file mode 100644 index 000000000..088ea8431 --- /dev/null +++ b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/Serializer.php @@ -0,0 +1,74 @@ + '/^ECHO/i', + 'args' => 0, + ], + [ + 'regex' => '/^(LPUSH|MSET|PFA|PUBLISH|RPUSH|SADD|SET|SPUBLISH|XADD|ZADD)/i', + 'args' => 1, + ], + [ + 'regex' => '/^(HSET|HMSET|LSET|LINSERT)/i', + 'args' => 2, + ], + [ + 'regex' => '/^(ACL|BIT|B[LRZ]|CLIENT|CLUSTER|CONFIG|COMMAND|DECR|DEL|EVAL|EX|FUNCTION|GEO|GET|HINCR|HMGET|HSCAN|INCR|L[TRLM]|MEMORY|P[EFISTU]|RPOP|S[CDIMORSU]|XACK|X[CDGILPRT]|Z[CDILMPRS])/i', + 'args' => -1, + ], + ]; + + /** + * Given the redis command name and arguments, return a combination of the + * command name + the allowed arguments according to `SERIALIZATION_SUBSETS`. + * + * @param string $command The redis command name + * @param array $params The redis command arguments + * @return string A combination of the command name + args according to `SERIALIZATION_SUBSETS`. + */ + public static function serializeCommand(string $command, array $params): string + { + if (count($params) === 0) { + return $command; + } + + $paramsToSerializeNum = 0; + + // Find the number of arguments to serialize for the given command + foreach (self::SERIALIZATION_SUBSETS as $subset) { + if (preg_match($subset['regex'], $command)) { + $paramsToSerializeNum = $subset['args']; + + break; + } + } + + // Serialize the allowed number of arguments + $paramsWillToSerialize = ($paramsToSerializeNum >= 0) ? array_slice($params, 0, $paramsToSerializeNum) : $params; + + // If there are more arguments than serialized, add a placeholder + if (count($params) > count($paramsWillToSerialize)) { + $paramsWillToSerialize[] = '[' . (count($params) - $paramsToSerializeNum) . ' other arguments]'; + } + + return $command . ' ' . implode(' ', $paramsWillToSerialize); + } +} diff --git a/src/Instrumentation/Laravel/tests/Unit/Watches/RedisCommand/SerializerTest.php b/src/Instrumentation/Laravel/tests/Unit/Watches/RedisCommand/SerializerTest.php new file mode 100644 index 000000000..69c831f34 --- /dev/null +++ b/src/Instrumentation/Laravel/tests/Unit/Watches/RedisCommand/SerializerTest.php @@ -0,0 +1,35 @@ +assertSame($expected, Serializer::serializeCommand($command, $params)); + } + + public function serializeCases(): iterable + { + // Only serialize command + yield ['ECHO', ['param1'], 'ECHO [1 other arguments]']; + + // Only serialize 1 params + yield ['SET', ['param1', 'param2'], 'SET param1 [1 other arguments]']; + yield ['SET', ['param1', 'param2', 'param3'], 'SET param1 [2 other arguments]']; + + // Only serialize 2 params + yield ['HSET', ['param1', 'param2', 'param3'], 'HSET param1 param2 [1 other arguments]']; + + // Serialize all params + yield ['DEL', ['param1', 'param2', 'param3', 'param4'], 'DEL param1 param2 param3 param4']; + } +} From 5227a8cadc69e7ad7f56b1b0a85a0b09ca1ef748 Mon Sep 17 00:00:00 2001 From: MilesChou Date: Wed, 18 Sep 2024 09:59:12 +0800 Subject: [PATCH 2/8] Update src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php Co-authored-by: Chris Lightfoot-Wild --- .../Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php index 3543e8ee9..6147fb50b 100644 --- a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php +++ b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php @@ -45,7 +45,7 @@ public function recordRedisCommand(CommandExecuted $event): void { $nowInNs = (int) (microtime(true) * 1E9); - $operationName = Str::upper($event->command); + $operationName = strtoupper($event->command); /** @psalm-suppress ArgumentTypeCoercion */ $span = $this->instrumentation->tracer() From 014b591eace68ae8ffce4afc0e289bc262ec2f43 Mon Sep 17 00:00:00 2001 From: MilesChou Date: Wed, 18 Sep 2024 10:05:54 +0800 Subject: [PATCH 3/8] Return null when get db index Don't cause application problem. --- .../RedisCommand/RedisCommandWatcher.php | 30 +++++++------------ 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php index 6147fb50b..8ca3e3f20 100644 --- a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php +++ b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php @@ -9,7 +9,6 @@ use Illuminate\Redis\Connections\PhpRedisConnection; use Illuminate\Redis\Connections\PredisConnection; use Illuminate\Redis\Events\CommandExecuted; -use Illuminate\Support\Str; use OpenTelemetry\API\Instrumentation\CachedInstrumentation; use OpenTelemetry\API\Trace\SpanKind; use OpenTelemetry\Contrib\Instrumentation\Laravel\Watchers\Watcher; @@ -17,6 +16,7 @@ use OpenTelemetry\SemConv\TraceAttributeValues; use RangeException; use RuntimeException; +use Throwable; /** * Watch the Redis Command event @@ -73,29 +73,19 @@ private function calculateQueryStartTime(int $nowInNs, float $queryTimeMs): int return (int) ($nowInNs - ($queryTimeMs * 1E6)); } - private function fetchDbIndex(Connection $connection): int + private function fetchDbIndex(Connection $connection): ?int { - if ($connection instanceof PhpRedisConnection) { - $index = $connection->client()->getDbNum(); - - if ($index === false) { - throw new RuntimeException('Cannot fetch database index.'); - } - - return $index; - } elseif ($connection instanceof PredisConnection) { - /** @psalm-suppress PossiblyUndefinedMethod */ - $index = $connection->client()->getConnection()->getParameters()->database; - - if (is_int($index)) { - throw new RuntimeException('Cannot fetch database index.'); + try { + if ($connection instanceof PhpRedisConnection) { + return $connection->client()->getDbNum(); + } elseif ($connection instanceof PredisConnection) { + /** @psalm-suppress PossiblyUndefinedMethod */ + return $connection->client()->getConnection()->getParameters()->database; } - - return $index; + } catch (Throwable $e) { } - throw new RangeException('Unknown Redis connection instance: ' . get_class($connection)); - + return null; } private function fetchDbHost(Connection $connection): string From 86fe930ee2b63d7d27c2f2823baa3784516dcba5 Mon Sep 17 00:00:00 2001 From: MilesChou Date: Wed, 18 Sep 2024 10:07:31 +0800 Subject: [PATCH 4/8] Renaming --- .../Laravel/src/Watchers/RedisCommand/Serializer.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/Serializer.php b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/Serializer.php index 088ea8431..9e76f7ce3 100644 --- a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/Serializer.php +++ b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/Serializer.php @@ -62,13 +62,13 @@ public static function serializeCommand(string $command, array $params): string } // Serialize the allowed number of arguments - $paramsWillToSerialize = ($paramsToSerializeNum >= 0) ? array_slice($params, 0, $paramsToSerializeNum) : $params; + $paramsToSerialize = ($paramsToSerializeNum >= 0) ? array_slice($params, 0, $paramsToSerializeNum) : $params; // If there are more arguments than serialized, add a placeholder - if (count($params) > count($paramsWillToSerialize)) { - $paramsWillToSerialize[] = '[' . (count($params) - $paramsToSerializeNum) . ' other arguments]'; + if (count($params) > count($paramsToSerialize)) { + $paramsToSerialize[] = '[' . (count($params) - $paramsToSerializeNum) . ' other arguments]'; } - return $command . ' ' . implode(' ', $paramsWillToSerialize); + return $command . ' ' . implode(' ', $paramsToSerialize); } } From 633a56e3b16fdbb1623797414402beb55ad50945 Mon Sep 17 00:00:00 2001 From: "miles.chou" Date: Thu, 19 Sep 2024 10:17:42 +0800 Subject: [PATCH 5/8] Fix return null statement --- .../src/Watchers/RedisCommand/RedisCommandWatcher.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php index 8ca3e3f20..2c2f3e869 100644 --- a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php +++ b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php @@ -82,10 +82,11 @@ private function fetchDbIndex(Connection $connection): ?int /** @psalm-suppress PossiblyUndefinedMethod */ return $connection->client()->getConnection()->getParameters()->database; } + + return null; } catch (Throwable $e) { + return null; } - - return null; } private function fetchDbHost(Connection $connection): string From 7af92f0be48406977f0eba901f41e09922546dc4 Mon Sep 17 00:00:00 2001 From: "miles.chou" Date: Thu, 19 Sep 2024 10:20:56 +0800 Subject: [PATCH 6/8] Fix return null statement in fetchDbHost() --- .../RedisCommand/RedisCommandWatcher.php | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php index 2c2f3e869..bdf370111 100644 --- a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php +++ b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php @@ -89,28 +89,19 @@ private function fetchDbIndex(Connection $connection): ?int } } - private function fetchDbHost(Connection $connection): string + private function fetchDbHost(Connection $connection): ?string { - if ($connection instanceof PhpRedisConnection) { - $host = $connection->client()->getHost(); - - if ($host === false) { - throw new RuntimeException('Cannot fetch database host.'); - } - - return $host; - } elseif ($connection instanceof PredisConnection) { - /** @psalm-suppress PossiblyUndefinedMethod */ - $host = $connection->client()->getConnection()->getParameters()->host; - - if (is_int($host)) { - throw new RuntimeException('Cannot fetch database index.'); + try { + if ($connection instanceof PhpRedisConnection) { + return $connection->client()->getHost(); + } elseif ($connection instanceof PredisConnection) { + /** @psalm-suppress PossiblyUndefinedMethod */ + return $connection->client()->getConnection()->getParameters()->host; } - return $host; + return null; + } catch (Throwable $e) { + return null; } - - throw new RangeException('Unknown Redis connection instance: ' . get_class($connection)); - } } From dc9b046277f89323cd73f14a15d58634da173547 Mon Sep 17 00:00:00 2001 From: "miles.chou" Date: Thu, 19 Sep 2024 10:39:04 +0800 Subject: [PATCH 7/8] Use DB_STATEMENT for compatibility with semconv 1.24. See https://github.com/open-telemetry/opentelemetry-specification/blob/v1.19.0/specification/trace/semantic_conventions/database.md#redis --- .../Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php index bdf370111..8c6d7531e 100644 --- a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php +++ b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php @@ -38,7 +38,7 @@ public function register(Application $app): void } /** - * Record a query. + * Record a Redis command. */ /** @psalm-suppress UndefinedThisPropertyFetch */ public function recordRedisCommand(CommandExecuted $event): void @@ -59,7 +59,7 @@ public function recordRedisCommand(CommandExecuted $event): void TraceAttributes::DB_SYSTEM => TraceAttributeValues::DB_SYSTEM_REDIS, TraceAttributes::DB_NAME => $this->fetchDbIndex($event->connection), TraceAttributes::DB_OPERATION => $operationName, - TraceAttributes::DB_QUERY_TEXT => Serializer::serializeCommand($event->command, $event->parameters), + TraceAttributes::DB_STATEMENT => Serializer::serializeCommand($event->command, $event->parameters), TraceAttributes::SERVER_ADDRESS => $this->fetchDbHost($event->connection), ]; From 5d3c2ec01f97b6e494545e694a86dbb3e5e8cd56 Mon Sep 17 00:00:00 2001 From: MilesChou Date: Sun, 22 Sep 2024 21:06:00 +0800 Subject: [PATCH 8/8] Remove unused class --- .../Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php index 8c6d7531e..6e798d4e5 100644 --- a/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php +++ b/src/Instrumentation/Laravel/src/Watchers/RedisCommand/RedisCommandWatcher.php @@ -14,8 +14,6 @@ use OpenTelemetry\Contrib\Instrumentation\Laravel\Watchers\Watcher; use OpenTelemetry\SemConv\TraceAttributes; use OpenTelemetry\SemConv\TraceAttributeValues; -use RangeException; -use RuntimeException; use Throwable; /**