feat(tracing): comprehensive improvements to tracing aspects and listeners #898
Conversation
…eners - Enhanced multiple tracing aspects with better data collection - Improved span creation and management across various components - Updated listeners for better tracing coverage - Optimized tracing data structure and performance
Walkthrough将大量 tracing span/transaction 的创建改为使用具名参数并在创建时设置 origin；将数据和错误信息直接写入 span/transaction，移除 finish 前的 setOrigin；引入空安全处理与若干 startSpan/startTransaction 签名更新（返回可空 span 并新增 origin 参数）。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant Aspect/Listener
participant SpanStarter
participant Hub
participant Span/Tx
Caller->>Aspect/Listener: 调用被拦截逻辑
Aspect/Listener->>SpanStarter: startSpan(op:, description:, origin: ...)
SpanStarter->>Hub: 获取当前 hub/parent
Note right of SpanStarter: 使用具名参数；可能返回 null；在创建时设置 origin
Hub-->>SpanStarter: parent (可选)
SpanStarter-->>Aspect/Listener: 返回 span/transaction (nullable)
alt span/tx 存在
Aspect/Listener->>Span/Tx: ?->setData(...) / 直接写入初始数据
Aspect/Listener->>Caller: proceed()
Caller-->>Aspect/Listener: 返回结果或抛异常
opt 成功
Aspect/Listener->>Span/Tx: setData(result/metrics)
end
opt 异常
Aspect/Listener->>Span/Tx: setStatus(...)->setTags(...)
Aspect/Listener->>Span/Tx: setData(['exception.stack_trace' => ...])
end
Aspect/Listener->>Span/Tx: finish()
else 无 span/tx
Aspect/Listener->>Caller: proceed()（不写入 trace）
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.17)Note: Using configuration file /phpstan.neon.dist. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
src/sentry/src/Tracing/Listener/TracingCommandListener.php (1)
105-116: 异常状态可能被后续基于 exit code 的 setStatus 覆盖。当存在异常时，已设置的
internalError会被下方基于退出码的状态重写，导致错误被误标为 OK。应以异常优先。建议改为先计算最终
status，再统一设置：if (method_exists($event, 'getThrowable') && $exception = $event->getThrowable()) { $transaction->setStatus(SpanStatus::internalError()); $tags = array_merge($tags, [ 'error' => true, 'exception.class' => $exception::class, 'exception.message' => $exception->getMessage(), 'exception.code' => $exception->getCode(), ]); if ($this->switcher->isTracingExtraTagEnable('exception.stack_trace')) { $data['exception.stack_trace'] = (string) $exception; } } - $transaction->setStatus($exitCode == SymfonyCommand::SUCCESS ? SpanStatus::ok() : SpanStatus::internalError()) + $status = $exitCode == SymfonyCommand::SUCCESS ? SpanStatus::ok() : SpanStatus::internalError(); + if (isset($exception) && $exception) { + $status = SpanStatus::internalError(); + } + $transaction->setStatus($status) ->setData($data) ->setTags($tags);Also applies to: 118-121
src/sentry/src/Tracing/Aspect/DbAspect.php (2)
68-72: 健壮性：避免未定义索引导致 Notice
SqlParser::parse()未必总返回table/operation。建议兜底。- $table = $sqlParse['table']; - $operation = $sqlParse['operation']; + $table = $sqlParse['table'] ?? ''; + $operation = $sqlParse['operation'] ?? ($operation ?? '');
101-103: 潜在报错：bindings可能不存在或非数组当前
foreach ($arguments['arguments']['bindings'] as ...)在bindings缺失时会报错。请做空数组兜底/类型收敛。- foreach ($arguments['arguments']['bindings'] as $key => $value) { + foreach ((array) ($arguments['arguments']['bindings'] ?? []) as $key => $value) { $data['db.parameter.' . $key] = $value; }src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php (3)
73-85: 修复潜在空指针：在使用 $span 注入追踪头之前必须先判空startSpan 可能返回 null（参见 SpanStarter 实现），当前在判空之前即调用
$span->toTraceparent()等方法，会导致致命错误。建议将判空提前，并在其后再注入头；同时移除后置的冗余判空：
// Inject trace context && Start span $span = $this->startSpan( op: 'http.client', description: $request->getMethod() . ' ' . (string) $request->getUri(), origin: 'auto.http.client', ); - $options['headers'] = array_replace($options['headers'] ?? [], [ - 'sentry-trace' => $span->toTraceparent(), - 'baggage' => $span->toBaggage(), - 'traceparent' => $span->toW3CTraceparent(), - ]); - $proceedingJoinPoint->arguments['keys']['options']['headers'] = $options['headers']; - - if (! $span) { - return $proceedingJoinPoint->process(); - } + if (! $span) { + return $proceedingJoinPoint->process(); + } + $options['headers'] = array_replace($options['headers'] ?? [], [ + 'sentry-trace' => $span->toTraceparent(), + 'baggage' => $span->toBaggage(), + 'traceparent' => $span->toW3CTraceparent(), + ]); + $proceedingJoinPoint->arguments['keys']['options']['headers'] = $options['headers'];Also applies to: 86-88
96-116: 避免敏感信息泄露：对请求头和配置/选项进行脱敏再上报
http.request.headers、http.guzzle.config、http.guzzle.options可能包含 Authorization/Cookie/密码等敏感信息，当前实现会原样上报到 Sentry。在闭包内对敏感字段做基础脱敏，并替换对应的数据字段：
$request = $stats->getRequest(); $uri = $request->getUri(); + // redact sensitive headers and options/config fields + $headersSanitized = []; + foreach ($request->getHeaders() as $name => $values) { + $lower = strtolower($name); + $headersSanitized[$name] = in_array($lower, ['authorization','cookie','set-cookie','x-api-key','x-auth-token']) + ? ['[redacted]'] + : $values; + } + $guzzleConfigSanitized = $guzzleConfig; + foreach (['auth','headers','token','password'] as $k) { + if (isset($guzzleConfigSanitized[$k])) { + $guzzleConfigSanitized[$k] = '[redacted]'; + } + } + $optionsSanitized = $options; + if (isset($optionsSanitized['headers']) && is_array($optionsSanitized['headers'])) { + foreach ($optionsSanitized['headers'] as $name => $_) { + if (in_array(strtolower($name), ['authorization','cookie','set-cookie','x-api-key','x-auth-token'])) { + $optionsSanitized['headers'][$name] = '[redacted]'; + } + } + } $data = [ // See: https://develop.sentry.dev/sdk/performance/span-data-conventions/#http 'http.query' => $uri->getQuery(), 'http.fragment' => $uri->getFragment(), 'http.request.method' => $request->getMethod(), - 'http.request.body.size' => strlen($options['body'] ?? ''), + 'http.request.body.size' => $bodySize, // set below 'http.request.full_url' => (string) $request->getUri(), 'http.request.path' => $request->getUri()->getPath(), 'http.request.scheme' => $request->getUri()->getScheme(), 'http.request.host' => $request->getUri()->getHost(), 'http.request.port' => $request->getUri()->getPort(), 'http.request.user_agent' => $request->getHeaderLine('User-Agent'), // updated key for consistency - 'http.request.headers' => $request->getHeaders(), + 'http.request.headers' => $headersSanitized, // Other 'coroutine.id' => Coroutine::id(), 'http.system' => 'guzzle', - 'http.guzzle.config' => $guzzleConfig, - 'http.guzzle.options' => $options ?? [], + 'http.guzzle.config' => $guzzleConfigSanitized, + 'http.guzzle.options' => $optionsSanitized ?? [], 'duration' => $stats->getTransferTime() * 1000, // in milliseconds ];如果需要，可再通过配置开关（如
request.headers）决定是否采集请求头。
100-103:strlen($options['body'])类型不安全，流/资源会触发报错Guzzle 的 body 支持 string|resource|StreamInterface。直接
strlen()可能抛出类型错误。在上方补充计算
$bodySize并替换数据字段（与上一条 diff 相互配合）：+ $body = $options['body'] ?? null; + $bodySize = is_string($body) + ? strlen($body) + : (($body instanceof \Psr\Http\Message\StreamInterface) + ? ($body->getSize() ?? 0) + : (is_resource($body) ? (fstat($body)['size'] ?? 0) : 0));src/sentry/src/Tracing/Aspect/GrpcAspect.php (3)
43-44:options取值未设默认，可能触发未定义索引当拦截点未传入
options时会报错。- $options = $proceedingJoinPoint->arguments['keys']['options']; + $options = $proceedingJoinPoint->arguments['keys']['options'] ?? [];
51-56: 修复潜在空指针：$parent 可能为 null 时直接调用 toTraceparent若当前 Hub 无活动 span，
$parent为 null，现有代码将致命错误。- $parent = SentrySdk::getCurrentHub()->getSpan(); - $options['headers'] = $options['headers'] ?? [] + [ - 'sentry-trace' => $parent->toTraceparent(), - 'baggage' => $parent->toBaggage(), - 'traceparent' => $parent->toW3CTraceparent(), - ]; - $proceedingJoinPoint->arguments['keys']['options'] = $options; + $parent = SentrySdk::getCurrentHub()->getSpan(); + if ($parent) { + $options['headers'] = $options['headers'] ?? [] + [ + 'sentry-trace' => $parent->toTraceparent(), + 'baggage' => $parent->toBaggage(), + 'traceparent' => $parent->toW3CTraceparent(), + ]; + $proceedingJoinPoint->arguments['keys']['options'] = $options; + }
45-49: 在上报 grpc.options 前进行脱敏，避免泄露认证信息
$options可能包含 headers/metadata 中的令牌/Cookie。建议脱敏后再写入$data。- $data = [ + $optionsSanitized = $options; + foreach (['headers','metadata'] as $hKey) { + if (isset($optionsSanitized[$hKey]) && is_array($optionsSanitized[$hKey])) { + foreach ($optionsSanitized[$hKey] as $k => $v) { + if (in_array(strtolower((string) $k), ['authorization','cookie','set-cookie','x-api-key','x-auth-token'])) { + $optionsSanitized[$hKey][$k] = '[redacted]'; + } + } + } + } + $data = [ 'grpc.method' => $method = $proceedingJoinPoint->arguments['keys']['method'], - 'grpc.options' => $options, + 'grpc.options' => $optionsSanitized, 'coroutine.id' => Coroutine::id(), ]; - $span = $this->startSpan( + $span = $this->startSpan( op: 'grpc.client', description: $method, origin: 'auto.grpc', )?->setData($data);Also applies to: 59-64
src/sentry/src/Tracing/Listener/TracingRequestListener.php (1)
115-130: 避免将全部请求头写入 Sentry 数据，存在高风险敏感信息泄露。当前会把 Authorization、Cookie、X-API-Key 等敏感头写入
data；Sentry 面板与事件转储中将可见这些内容。建议至少做黑名单脱敏，并限制单值长度。建议改造如下：
- foreach ($request->getHeaders() as $key => $value) { - $data['http.request.header.' . $key] = implode(', ', $value); - } + $redact = ['authorization','proxy-authorization','cookie','set-cookie','x-api-key','x-auth-token','x-csrf-token']; + foreach ($request->getHeaders() as $key => $values) { + $lower = strtolower($key); + $val = substr(implode(', ', (array) $values), 0, 1024); + $data['http.request.header.' . $key] = in_array($lower, $redact, true) ? '[REDACTED]' : $val; + }若需要更严格，可改为白名单只保留：host、user-agent、accept、content-type、content-length、x-request-id 等。
🧹 Nitpick comments (25)
src/sentry/src/Tracing/SpanStarter.php (1)
42-50: 对可能为 null 的 op/description/origin 做保护，避免潜在 TypeError目前无条件调用 setOp()/setDescription()/setOrigin()；若调用方传 null，将依赖 SDK 方法是否接受 null。为稳健起见建议仅在非 null 时设置，保持与 continueTrace 对 name/op/description/origin 的 is_string 检查一致。
- return tap( - $parent->startChild(new SpanContext()) - ->setOp($op) - ->setDescription($description) - ->setOrigin($origin) - ->setStatus(SpanStatus::ok()) - ->setStartTimestamp(microtime(true)), - fn (Span $span) => $asParent && SentrySdk::getCurrentHub()->setSpan($span) - ); + return tap( + $parent->startChild(new SpanContext()) + ->setStatus(SpanStatus::ok()) + ->setStartTimestamp(microtime(true)), + function (Span $span) use ($op, $description, $origin, $asParent) { + $op !== null && $span->setOp($op); + $description !== null && $span->setDescription($description); + $origin !== null && $span->setOrigin($origin); + $asParent && SentrySdk::getCurrentHub()->setSpan($span); + } + );src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php (1)
117-117: 链式 setData()->setTags() 合理；建议统一“耗时/时延”的单位当前同仓库部分代码（如 Guzzle on_stats）以毫秒记录 duration；本监听器中
messaging.message.receive.latency（定义在未改动的行 98）为秒。为便于检索与聚合，建议统一为毫秒（或统一在文档中明确单位）。src/sentry/src/Tracing/Listener/TracingCrontabListener.php (1)
101-101: 链式 setData()->setTags() 合理；可考虑与其他 Listener 对齐启动时间设置部分 Listener（如 AsyncQueue/Request）会在创建后显式 setStartTimestamp(microtime(true))。本处不强制，但如需统一时间戳来源，可在 startTransaction 时补齐。
src/sentry/src/Tracing/Listener/TracingCommandListener.php (1)
81-83: 考虑使用更贴合场景的 TransactionSource。命令行执行场景可用
TransactionSource::task()，更利于检索与来源聚合；当前custom()亦可用，但语义偏弱。- origin: 'auto.command', - source: TransactionSource::custom() + origin: 'auto.command', + source: TransactionSource::task()src/sentry/src/Tracing/Aspect/TraceAnnotationAspect.php (2)
59-66: 为注解产生的 span 显式设置 origin，保持全局一致性。其他 Aspect/Listener 已统一为
auto.*，建议此处补齐，便于仪表盘按 origin 维度聚合。$span = $this->startSpan( op: $annotation->op ?? 'method', description: $annotation->description ?? sprintf( '%s::%s()', $proceedingJoinPoint->className, $methodName ), + origin: 'auto.annotation', asParent: true );
91-91: 防御性处理：finally 中可使用 null-safe 调用。理论上
$span在此前已确保存在，但为一致性与健壮性，可使用?->避免极端情况下的空指针。- $span->setData($data)->finish(microtime(true)); + $span?->setData($data)->finish(microtime(true));src/sentry/src/Tracing/Aspect/RpcAspect.php (1)
150-151: 可选：对 RPC 参数/上下文做可配置记录或脱敏。
rpc.arguments与rpc.context可能包含敏感信息，建议与rpc.result相同，增加可开关或脱敏策略。可在
handleSend中按配置控制写入，例如：// 可配置开关示例（片段） if ($this->switcher->isTracingExtraTagEnable('rpc.arguments')) { $data['rpc.arguments'] = $proceedingJoinPoint->arguments['keys']; } if ($this->switcher->isTracingExtraTagEnable('rpc.context') && $this->container->has(Rpc\Context::class)) { $data['rpc.context'] = $this->container->get(Rpc\Context::class)->getData(); }src/sentry/src/Tracing/Aspect/DbAspect.php (2)
110-112: 控制体积：对db.result做截断或采样结果集常常很大，建议截断（如 4KB）或在配置开关下才记录，避免吞吐下降与事件超限。
- $span?->setData([ - 'db.result' => json_encode($result, JSON_UNESCAPED_UNICODE), - ]); + $json = json_encode($result, JSON_UNESCAPED_UNICODE); + $span?->setData([ + 'db.result' => strlen($json) > 4096 ? substr($json, 0, 4096) . '…(truncated)' : $json, + ]);
115-121: Sentry 标签值请使用字符串类型部分 SDK 会把 tag 视为字符串；将布尔与数值转为字符串更稳妥。
- $span?->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => true, - 'exception.class' => $exception::class, - 'exception.message' => $exception->getMessage(), - 'exception.code' => $exception->getCode(), - ]); + $span?->setStatus(SpanStatus::internalError()) + ->setTags([ + 'error' => 'true', + 'exception.class' => $exception::class, + 'exception.message' => $exception->getMessage(), + 'exception.code' => (string) $exception->getCode(), + ]);src/sentry/src/Tracing/Aspect/CoordinatorAspect.php (2)
53-59: 将标签值统一转为字符串，提升兼容性避免某些后端/索引器对布尔与整型处理不一致。
- $span?->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => true, - 'exception.class' => $exception::class, - 'exception.message' => $exception->getMessage(), - 'exception.code' => $exception->getCode(), - ]); + $span?->setStatus(SpanStatus::internalError()) + ->setTags([ + 'error' => 'true', + 'exception.class' => $exception::class, + 'exception.message' => $exception->getMessage(), + 'exception.code' => (string) $exception->getCode(), + ]);
67-67: 风格一致性：finish() 可不显式传入时间戳其它 Aspect 多为
finish()，建议统一，逻辑等价。- $span?->finish(microtime(true)); + $span?->finish();src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php (3)
92-94: 控制体积：对 elasticsearch.result 进行截断避免大响应冲击吞吐与事件体积。
- $span->setData([ - 'elasticsearch.result' => json_encode($result, JSON_UNESCAPED_UNICODE), - ]); + $json = json_encode($result, JSON_UNESCAPED_UNICODE); + $span->setData([ + 'elasticsearch.result' => strlen($json) > 4096 ? substr($json, 0, 4096) . '…(truncated)' : $json, + ]);
97-103: 将标签值统一为字符串类型与其他 Aspect 一致，避免后端差异。
- $span->setStatus(SpanStatus::internalError()) - ->setTags([ - 'error' => true, - 'exception.class' => $exception::class, - 'exception.message' => $exception->getMessage(), - 'exception.code' => $exception->getCode(), - ]); + $span->setStatus(SpanStatus::internalError()) + ->setTags([ + 'error' => 'true', + 'exception.class' => $exception::class, + 'exception.message' => $exception->getMessage(), + 'exception.code' => (string) $exception->getCode(), + ]);
80-83: 少发空值：不要发送占位空字符串键
http.request.method/url.full/server.host/server.port为空字符串会污染指标，建议未取到就不写或写null。src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php (1)
128-134: HTTP 非 2xx 状态建议映射更精确的 SpanStatus目前 4xx/5xx 一律标记为
internalError()，建议（若 SDK 支持）用SpanStatus::fromHttpStatus(...)映射，更利于告警与报表聚合。如不可用，可按 4xx/5xx 区分 client_error/server_error。
src/sentry/src/Tracing/Aspect/GrpcAspect.php (1)
72-83: 根据 gRPC Status Code 设置 SpanStatus，必要时标记错误标签当前仅在异常分支标错；非 0 的 gRPC code 也应标记为失败。
可在解析出
$code后追加：if ($code !== 0) { $span->setStatus(SpanStatus::internalError()) ->setTags(['error' => true, 'grpc.code' => $code, 'grpc.message' => $message]); }同时注意
response.body可能为二进制/巨大，可考虑截断或上限大小。src/sentry/src/Tracing/Aspect/CoroutineAspect.php (2)
53-58: 移除重复设置 origin在 startSpan 已通过命名参数传入
origin: 'auto.coroutine'，随后再次setOrigin('auto.coroutine')冗余。$parent = $this->startSpan( op: 'coroutine.create', description: $callingOnFunction, origin: 'auto.coroutine', - )?->setOrigin('auto.coroutine'); + );
78-81: 可选：defer 中恢复上一个当前 span，避免 Hub 持有已完成 span现在在
defer中把当前 span 设置为$transaction后立即finish()，可能导致 Hub 当前 span 指向已完成对象。可先缓存上一个 span 并在结束后恢复。- defer(function () use ($transaction) { - SentrySdk::getCurrentHub()->setSpan($transaction); - $transaction->finish(); - }); + $prev = SentrySdk::getCurrentHub()->getSpan(); + defer(function () use ($transaction, $prev) { + SentrySdk::getCurrentHub()->setSpan($transaction); + $transaction->finish(); + SentrySdk::getCurrentHub()->setSpan($prev); + });src/sentry/src/Tracing/Listener/TracingKafkaListener.php (1)
128-133: 小建议：避免写入空 tags，并统一度量单位
- 若
$tags为空可以跳过setTags()，减少无意义变更。messaging.message.receive.latency目前为秒（microtime(true)差值），建议与其它地方统一到毫秒，或在 key 中标注单位（例如...latency.ms）。- 另外，Sentry 的 tag 值规范为字符串，
'error' => true在不同 SDK 上可能被隐式转换，建议统一转为'true'。src/sentry/src/Tracing/Listener/TracingAmqpListener.php (1)
136-141: 小建议：空 tags 与 tag 值类型的细节
- 当没有异常时
$tags为空，建议跳过setTags()。- 建议将
'error' => true统一转为字符串，避免 SDK 差异导致的隐式类型转换。src/sentry/src/Tracing/Listener/TracingRedisListener.php (1)
100-101: 可选：注意数据体与隐私信息的体积与脱敏
db.statement与db.redis.parameters可能包含大体积或敏感值。建议提供可配置的脱敏/截断（例如最大长度、白名单键），以降低体积和敏感信息泄露风险。如需要，我可以补一个小型助手类（如
DataRedactor）并在此处接入，支持长度阈值与键名白名单。src/sentry/src/Tracing/Aspect/CacheAspect.php (1)
99-100: 可选：控制 item_size 的计算开销这里 finish 前附加的数据包含多处
json_encode与strlen，在大对象时开销可观。可考虑：
- 仅在开启额外开关时计算尺寸；或
- 对尺寸计算设置上限/采样。
src/sentry/src/Tracing/Aspect/RedisAspect.php (1)
101-101: 可选：与其它监听器对齐数据键名此处使用
'redis.result'，而TracingRedisListener使用'db.redis.result'。建议统一为后者，便于检索与分析。应用如下微调：
- $span->setData(['redis.result' => $result]); + $span->setData(['db.redis.result' => $result]);src/sentry/src/Tracing/Listener/TracingRequestListener.php (2)
101-110: 在创建事务时即设置 origin 的方向正确，但建议消除“魔法字符串”。建议将 'auto.request' 这类 origin 统一以常量/枚举集中管理，避免跨文件拼写不一致，便于全局检索与变更。
132-137: 去掉与 op 相同的 description，减少冗余。
description: 'request.received'与op相同，信息增量有限；可省略以降低事件体积。- $span = $this->startSpan( - op: 'request.received', - description: 'request.received', - origin: 'auto.request.received', - asParent: true - ); + $span = $this->startSpan( + op: 'request.received', + origin: 'auto.request.received', + asParent: true + );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (22)
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/AsyncQueueJobMessageAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/CacheAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/CoordinatorAspect.php(1 hunks)src/sentry/src/Tracing/Aspect/CoroutineAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/DbAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/GrpcAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php(3 hunks)src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php(4 hunks)src/sentry/src/Tracing/Aspect/RedisAspect.php(2 hunks)src/sentry/src/Tracing/Aspect/RpcAspect.php(3 hunks)src/sentry/src/Tracing/Aspect/TraceAnnotationAspect.php(2 hunks)src/sentry/src/Tracing/Listener/TracingAmqpListener.php(2 hunks)src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php(2 hunks)src/sentry/src/Tracing/Listener/TracingCommandListener.php(2 hunks)src/sentry/src/Tracing/Listener/TracingCrontabListener.php(2 hunks)src/sentry/src/Tracing/Listener/TracingDbQueryListener.php(1 hunks)src/sentry/src/Tracing/Listener/TracingKafkaListener.php(2 hunks)src/sentry/src/Tracing/Listener/TracingRedisListener.php(2 hunks)src/sentry/src/Tracing/Listener/TracingRequestListener.php(2 hunks)src/sentry/src/Tracing/SpanStarter.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (15)
src/sentry/src/Tracing/Aspect/TraceAnnotationAspect.php (1)
src/sentry/src/Switcher.php (1)
isTracingExtraTagEnable(53-56)
src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php (2)
src/sentry/src/Tracing/Aspect/CacheAspect.php (1)
process(44-104)src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
process(40-108)
src/sentry/src/Tracing/Aspect/DbAspect.php (4)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(32-51)src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php (1)
process(59-116)src/sentry/src/Tracing/Aspect/RedisAspect.php (1)
process(48-121)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnable(53-56)
src/sentry/src/Tracing/Listener/TracingRedisListener.php (1)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(32-51)
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php (5)
src/sentry/src/Tracing/Aspect/CacheAspect.php (1)
process(44-104)src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
process(40-108)src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php (1)
process(59-116)src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php (1)
process(46-159)src/sentry/src/Tracing/Aspect/RedisAspect.php (1)
process(48-121)
src/sentry/src/Tracing/Aspect/RpcAspect.php (3)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(32-51)src/sentry/src/Util/Carrier.php (5)
has(83-86)get(88-91)Carrier(21-111)fromSpan(52-59)toJson(103-110)src/sentry/src/Constants.php (1)
Constants(14-21)
src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php (3)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(32-51)src/sentry/src/Tracing/Aspect/DbAspect.php (1)
process(44-134)src/sentry/src/Tracing/Aspect/RedisAspect.php (1)
process(48-121)
src/sentry/src/Tracing/Aspect/CoordinatorAspect.php (6)
src/sentry/src/Tracing/Aspect/CacheAspect.php (1)
process(44-104)src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
process(40-108)src/sentry/src/Tracing/Aspect/DbAspect.php (1)
process(44-134)src/sentry/src/Tracing/Aspect/GrpcAspect.php (1)
process(37-102)src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php (1)
process(46-159)src/sentry/src/Tracing/Aspect/RedisAspect.php (1)
process(48-121)
src/sentry/src/Tracing/SpanStarter.php (7)
src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Tracing/Listener/TracingAmqpListener.php (1)
startTransaction(64-89)src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php (1)
startTransaction(64-80)src/sentry/src/Tracing/Listener/TracingCommandListener.php (1)
startTransaction(73-84)src/sentry/src/Tracing/Listener/TracingCrontabListener.php (1)
startTransaction(57-68)src/sentry/src/Tracing/Listener/TracingKafkaListener.php (1)
startTransaction(62-87)src/sentry/src/Tracing/Listener/TracingRequestListener.php (1)
startTransaction(70-146)
src/sentry/src/Tracing/Aspect/GrpcAspect.php (2)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(32-51)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnable(53-56)
src/sentry/src/Tracing/Listener/TracingDbQueryListener.php (1)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(32-51)
src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php (1)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(32-51)
src/sentry/src/Tracing/Aspect/RedisAspect.php (2)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(32-51)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnable(53-56)
src/sentry/src/Tracing/Listener/TracingRequestListener.php (1)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(32-51)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (3)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(32-51)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnable(53-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 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 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
🔇 Additional comments (29)
src/sentry/src/Tracing/Aspect/AmqpProducerAspect.php (1)
63-67: 在创建时设置 origin，方向正确且与全局改造一致使用命名参数并在 startSpan 时即设置 origin，避免 finish 前再补 origin，语义清晰也更高效。
src/sentry/src/Tracing/SpanStarter.php (3)
32-37: 扩展 startSpan 支持 origin：接口设计合理新增 origin 参数符合本 PR 的统一策略，向后兼容且提升可读性。
90-101: 使用变参接收命名参数：实现正确PHP 允许将未匹配到显式形参的命名参数收集到可变参数 …$options 中，并保留键名；因此这里对 $options['name'|'op'|'description'|'origin'] 的读取是合法且可靠的。
参见 PHP 手册与 RFC 对“命名参数 + 变参”的说明。 (php.watch, wiki.php.net)
108-111: 链式初始化 Transaction：简洁且一致在 startTransaction 后立即设置 startTimestamp 与 ok 状态，满足默认期望。
src/sentry/src/Tracing/Listener/TracingAsyncQueueListener.php (1)
71-79: 在创建时设置 async_queue 事务 origin：一致性良好与其他监听器对齐，origin 放在 continueTrace 创建时设置，避免 finish 时重复处理。
src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php (2)
61-63: 为 Kafka 发送 Span 设置 origin：OK与 AMQP/Redis 等 Aspect 统一在 startSpan 时设置 origin，可读性与一致性佳。
103-105: 批量发送时的 origin 设置也已对齐sendBatchAsync 同步加上 origin，保持一致性。
src/sentry/src/Tracing/Listener/TracingCrontabListener.php (1)
61-67: 在创建时设置 crontab 事务 origin：一致性良好保持与其他 Listener 的统一策略，便于在 Sentry 中按 origin 聚合。
src/sentry/src/Tracing/Listener/TracingDbQueryListener.php (1)
108-112: LGTM：使用命名参数与 null-safe 链式调用正确，origin 一致。
startSpan(..., origin: 'auto.db')?->setData(...)在无父 span 时安全短路，避免额外开销，符合本 PR 的统一规范。src/sentry/src/Tracing/Aspect/RpcAspect.php (1)
85-88: LGTM：在创建时设置 origin，语义与其他组件一致。
origin: 'auto.rpc'与命名参数的采用一致化了埋点元数据，便于跨组件分析。src/sentry/src/Tracing/Aspect/DbAspect.php (4)
105-105: LGTM：首批数据在创建后立即落到 span 上与 PR 目标一致，null-safe 写法也避免了父 span 缺失时的报错。
123-125: LGTM：堆栈写入受控开关与其它 Aspect 保持一致，安全可控。
130-130: LGTM：null-safe finish父 span 缺失时安全退出，符合预期。
88-91: 字段命名与规范校对
db.collection.name更常见于文档型数据库；对于 SQL 表，是否采用更贴近规范的键（如db.sql.table）需确认团队约定/可观测性平台映射。src/sentry/src/Tracing/Aspect/CoordinatorAspect.php (2)
45-48: LGTM：使用具名参数并在创建时设置 origin与整体重构一致，空安全链路也正确。
61-63: LGTM：异常堆栈受开关控制最小化噪声与敏感信息暴露。
src/sentry/src/Tracing/Aspect/ElasticsearchAspect.php (5)
65-65: LGTM：注释更正小修复，无行为变更。
67-70: LGTM：在创建时设置 origin，具名参数更清晰与全局策略一致。
87-87: LGTM：数据尽早落盘到 span便于后续异常路径也能保留上下文。
105-107: LGTM：堆栈信息受开关控制符合最小暴露原则。
112-112: LGTM：正常收尾与其他 Aspect 保持一致。
src/sentry/src/Tracing/Listener/TracingKafkaListener.php (1)
84-86: 在创建阶段设置 origin 的改动很合理在 continueTrace 时直接传入
origin: 'auto.kafka'，与本 PR 的统一策略一致，避免在 finish 阶段再突变元数据。src/sentry/src/Tracing/Listener/TracingAmqpListener.php (1)
86-88: 在创建阶段设置 origin 的改动很合理
origin: 'auto.amqp'放在事务创建处，语义更清晰，也与其它监听器保持一致。src/sentry/src/Tracing/Listener/TracingRedisListener.php (1)
73-77: LGTM：使用命名参数并在创建时设置 origin
startSpan(op:, description:, origin:)的使用方式一致且清晰，空父 span 时的早退判断在下方也已覆盖。src/sentry/src/Tracing/Aspect/CacheAspect.php (1)
75-77: LGTM：在创建时设置 origin，并将子 span 设为当前上下文
asParent: true并在 finally 中恢复父 span 的做法正确且安全。src/sentry/src/Tracing/Aspect/RedisAspect.php (1)
87-92: LGTM：命名参数与创建时设置 origin 一致
startSpan(op:, description:, origin:)的使用与全局策略对齐，初始数据直接落在 span 上也更直观。src/sentry/src/Tracing/Listener/TracingRequestListener.php (3)
132-137: startSpan 使用命名参数并在创建时设置 origin 的做法正确。与 SpanStarter::startSpan 签名对齐，且
asParent: true与下方 defer 中的恢复逻辑一致。
140-145: 移除 finish 前再设置 origin 的旧路径，直接 finish 即可。简化了收尾逻辑，符合在创建时确定 origin 的新约定。
101-114: 确认startRequestTransaction返回类型
请验证TracingRequestListener::startRequestTransaction是否可能返回null；如其始终返回Transaction实例，则无需对getSampled()使用空安全操作符。
| } | ||
|
|
||
| return tap($proceedingJoinPoint->process(), fn () => $span->setOrigin('auto.kafka')->finish()); | ||
| return tap($proceedingJoinPoint->process(), fn () => $span->finish()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
批量发送的异常收敛与 finish 建议与单发一致
同上，避免仅成功路径 finish。可复用与单发相同的 try/catch/finally 模式。
🤖 Prompt for AI Agents
src/sentry/src/Tracing/Aspect/KafkaProducerAspect.php around line 124: the
current code returns tap($proceedingJoinPoint->process(), fn () =>
$span->finish()) which only finishes the span on the success path; change to the
same try/catch/finally pattern used for single-send: call
$proceedingJoinPoint->process() inside a try, catch Throwable/Exception to
record the exception on the span (e.g., $span->setThrowable($e) or
$span->captureException/recordException) and rethrow, and always call
$span->finish() in the finally block so the span is finished regardless of
success or failure.
代码审查报告📊 整体评价这个PR对tracing系统进行了一次全面且系统性的重构，质量很高！主要改进包括：
🔍 具体发现优点
建议改进的地方1. ElasticsearchAspect.php:65 // 修复拼写错误
op: 'db.elasticsearch', // 👍 已修复 'elasticserach' 拼写2. 性能优化建议 - CacheAspect.php // 建议对大数据进行截断
'item_size' => match (true) {
$result instanceof Collection => $result->count(),
is_array($result) => count($result),
is_string($result) => strlen($result),
default => strlen(json_encode($result, JSON_UNESCAPED_UNICODE)),
}3. 异常处理一致性 - 所有Aspect // 建议统一异常处理模式
} catch (Throwable $exception) {
$span?->setStatus(SpanStatus::internalError())
->setTags([
'error' => true,
'exception.class' => $exception::class,
'exception.message' => $exception->getMessage(),
]);
throw $exception;
}
|
…eners (#898) * feat(tracing): comprehensive improvements to tracing aspects and listeners - Enhanced multiple tracing aspects with better data collection - Improved span creation and management across various components - Updated listeners for better tracing coverage - Optimized tracing data structure and performance * fix(tracing): add type hint for job variable in handlePush method * fix(tracing): add null safety checks for span status and finish methods in RedisAspect --------- Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/sentry/src/Tracing/Aspect/RedisAspect.php (1)
105-105: 修复：catch 分支可能 NPE（延续先前评论）当 startSpan() 返回 null 且业务抛异常时，这里会触发空指针。
- $span->setData(['exception.stack_trace' => (string) $exception]); + $span?->setData(['exception.stack_trace' => (string) $exception]);
🧹 Nitpick comments (2)
src/sentry/src/Tracing/Aspect/RedisAspect.php (2)
101-101: 限制 redis.result 的体积与敏感信息，避免高并发下的内存放大/PII 泄漏结果数据可能过大且包含敏感值。建议在写入前进行截断/脱敏。
- $span->setData(['redis.result' => $result]); + $span->setData(['redis.result' => $this->truncateForSentry($result)]);可添加辅助方法（类内）：
private function truncateForSentry(mixed $value, int $limit = 2048): mixed { if (is_string($value)) { return mb_strlen($value) <= $limit ? $value : (mb_substr($value, 0, $limit) . '…(truncated)'); } if (is_array($value)) { $json = json_encode($value, JSON_UNESCAPED_UNICODE); if ($json === false) { return '[unserializable]'; } return mb_strlen($json) <= $limit ? $value : ['preview' => mb_substr($json, 0, $limit) . '…(truncated)', 'length' => mb_strlen($json)]; } return $value; }
87-91: 分离 Span 赋值与 setData 调用
将$this->startSpan(...)?->setData($data)拆成两步：$span = $this->startSpan( op: $op, description: $description, origin: 'auto.cache.redis', ); if ($span) { $span->setData($data); }防止未来
setData()返回值或签名变更导致链式调用跳过后续逻辑。保持 origin 前缀一致
Redis 属于缓存场景，已有CacheAspect使用auto.cache，RedisAspect使用auto.cache.redis与之保持一致，无需改为auto.db.redis。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/sentry/src/Tracing/Aspect/RedisAspect.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/sentry/src/Tracing/Aspect/RedisAspect.php (2)
src/sentry/src/Tracing/SpanStarter.php (1)
startSpan(32-51)src/sentry/src/Switcher.php (1)
isTracingExtraTagEnable(53-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 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.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
🔇 Additional comments (2)
src/sentry/src/Tracing/Aspect/RedisAspect.php (2)
107-113: 异常状态与标签写入的链式调用 LGTM空安全前缀保证了无父 span 情况下不会抛错，标签键也与其他组件保持一致。
117-117: 使用空安全 finish() 正确在无父 span 时安全退出，符合预期。
Summary
This PR introduces comprehensive improvements to the tracing system:
Changes
Test plan
Summary by CodeRabbit
新功能
修复
重构