Improved redis tracing #847
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough本次变更引入了一个新的 Changes
Sequence Diagram(s)sequenceDiagram
participant E as Redis事件
participant L as 追踪组件（Aspect/Listener）
participant RC as RedisCommand
participant T as 追踪系统
E->>L: 触发 Redis 命令事件
L->>RC: 实例化 RedisCommand(command, parameters)
RC-->>L: 返回格式化的命令字符串 (__toString())
L->>T: 记录 db.statement 字段
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
tests/Sentry/RedisCommandTest.php (1)
46-50: 考虑优化空参数输出格式当前对于无参数命令（如 PING）的处理会在命令后添加一个空格，即
"PING "。虽然这种格式在 Redis 通信中是有效的，但可能需要考虑是否有必要保留这个尾随空格，或者对无参数情况做特殊处理。- expect((string) $command)->toBe('PING '); + expect((string) $command)->toBe('PING');对应的需要修改 RedisCommand 类的 __toString 方法，检测参数是否为空并相应地调整输出格式。
src/sentry/src/Tracing/Aspect/RedisAspect.php (1)
69-69: 使用 RedisCommand 类优化命令格式化使用 RedisCommand 类替代了原来的字符串拼接方式，提高了代码的可维护性和一致性。但建议直接使用
(string)类型转换而不是显式调用__toString()，更符合 PHP 的惯用做法。- 'db.statement' => (new RedisCommand($arguments['name'], $arguments['arguments']))->__toString(), + 'db.statement' => (string)(new RedisCommand($arguments['name'], $arguments['arguments'])),src/sentry/src/Tracing/Listener/TracingRedisListener.php (2)
62-62: 使用 RedisCommand 类优化命令格式化使用 RedisCommand 类替代了原来的字符串拼接方式，提高了代码的可维护性和一致性。但建议直接使用
(string)类型转换而不是显式调用__toString()，更符合 PHP 的惯用做法。- 'db.statement' => (new RedisCommand($event->command, $event->parameters))->__toString(), + 'db.statement' => (string)(new RedisCommand($event->command, $event->parameters)),
71-76: 描述符构建方式与 RedisCommand 不一致注意
$description的构建方式仍然使用原始的implode方法，只取参数数组的前两个元素，而db.statement已改为使用 RedisCommand 类处理所有参数。建议统一使用 RedisCommand 类来构建描述符，以保持一致性。- $description = sprintf( - '%s %s', - strtoupper($event->command), - implode(' ', [$event->parameters[0] ?? '', $event->parameters[1] ?? '']) - ); + $description = (string)(new RedisCommand(strtoupper($event->command), array_slice($event->parameters, 0, 2)));src/sentry/src/Util/RedisCommand.php (2)
18-42: RedisCommand 类实现完善RedisCommand 类实现了 Stringable 接口，能够处理各种参数类型，包括简单参数、数组参数和嵌套数组参数。代码结构清晰，实现了功能需求。
但有几点可以优化：
- 在处理空参数的情况下，可能会产生尾随空格，如在无参数时输出 "PING "
- 嵌套数组的情况直接使用 json_encode，这在大多数情况下是合理的，但可能会有编码问题，尤其是在处理非 ASCII 字符时
public function __toString(): string { $parameters = collect($this->parameters)->map(function ($parameter) { if (is_array($parameter)) { return collect($parameter)->map(function ($value, $key) { if (is_array($value)) { - return sprintf('%s %s', $key, json_encode($value)); + return sprintf('%s %s', $key, json_encode($value, JSON_UNESCAPED_UNICODE)); } return is_int($key) ? $value : sprintf('%s %s', $key, $value); })->implode(' '); } return $parameter; })->implode(' '); - return sprintf('%s %s', $this->command, $parameters); + return empty($parameters) ? $this->command : sprintf('%s %s', $this->command, $parameters); }
26-39: 复杂数组处理逻辑复用考虑当前嵌套 map 函数处理数组逻辑相对复杂。考虑抽取这个逻辑到一个私有方法中，以提高代码可读性和可维护性。
+ private function formatParameter($parameter) + { + if (!is_array($parameter)) { + return $parameter; + } + + return collect($parameter)->map(function ($value, $key) { + if (is_array($value)) { + return sprintf('%s %s', $key, json_encode($value, JSON_UNESCAPED_UNICODE)); + } + + return is_int($key) ? $value : sprintf('%s %s', $key, $value); + })->implode(' '); + } + public function __toString(): string { - $parameters = collect($this->parameters)->map(function ($parameter) { - if (is_array($parameter)) { - return collect($parameter)->map(function ($value, $key) { - if (is_array($value)) { - return sprintf('%s %s', $key, json_encode($value)); - } - - return is_int($key) ? $value : sprintf('%s %s', $key, $value); - })->implode(' '); - } - - return $parameter; - })->implode(' '); + $parameters = collect($this->parameters)->map([$this, 'formatParameter'])->implode(' ');
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/sentry/src/Tracing/Aspect/RedisAspect.php(2 hunks)src/sentry/src/Tracing/Listener/TracingRedisListener.php(2 hunks)src/sentry/src/Util/RedisCommand.php(1 hunks)tests/Sentry/RedisCommandTest.php(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.1
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.1
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.1
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.1
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.1
🔇 Additional comments (5)
tests/Sentry/RedisCommandTest.php (3)
16-20: 测试用例结构清晰，覆盖了简单参数场景测试案例正确验证了 RedisCommand 类对简单参数的处理能力，代码逻辑清晰明了。
28-38: 数组参数处理测试用例全面测试用例很好地覆盖了关联数组和嵌套数组的处理逻辑，确保了 RedisCommand 类能够正确处理复杂的参数结构。特别是对嵌套数组使用 JSON 编码的方式处理得当。
58-68: Redis SET 命令选项测试完善测试用例全面覆盖了 Redis SET 命令的 NX 和 EX 选项，这些是常用的 Redis 操作场景，测试确保了 RedisCommand 能正确处理这些特殊情况。
src/sentry/src/Tracing/Aspect/RedisAspect.php (1)
16-16: 正确引入 RedisCommand 类导入语句正确添加，符合代码规范。
src/sentry/src/Tracing/Listener/TracingRedisListener.php (1)
16-16: 正确引入 RedisCommand 类导入语句正确添加，符合代码规范。
huangdijia
added a commit
that referenced
this pull request
Mar 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
RedisCommand类的测试文件，涵盖多种参数类型的 Redis 命令的字符串表示验证。