Skip to content

Conversation

@huangdijia
Copy link
Contributor

@huangdijia huangdijia commented Oct 12, 2025

Summary

This PR refactors the Sentry tracing implementation by centralizing hardcoded trace header strings into the Constants class for improved maintainability and consistency.

Changes

  • Added three new constants to Constants.php:

    • SENTRY_TRACE: 'sentry-trace'
    • BAGGAGE: 'baggage'
    • TRACEPARENT: 'traceparent'
  • Updated the following classes to use the new constants instead of hardcoded strings:

    • GrpcAspect: Updated header injection to use constants
    • GuzzleHttpClientAspect: Updated header injection to use constants
    • RpcAspect: Minor code formatting improvements
    • Carrier: Updated all methods to use constants for header keys
    • CarrierPacker: Updated pack/unpack methods to use constants

Benefits

  • Improved maintainability: Header names are now defined in a single location
  • Reduced risk of typos: Using constants prevents string literal typos
  • Better IDE support: Constants provide better autocomplete and refactoring support
  • Consistency: All trace header references now use the same constants throughout the codebase

Testing

  • No functional changes were made; this is purely a refactoring
  • All existing tests should continue to pass
  • The behavior remains identical to the previous implementation

Summary by CodeRabbit

  • 重构
    • 以常量统一管理 Sentry 追踪头,提升一致性与可维护性。
    • gRPC 与 HTTP 请求仅注入 sentry-trace、baggage,停止发送 traceparent,改进兼容性与稳定性。
    • 优化 RPC 上下文复用与载体生成,减少重复操作,带来轻微性能提升。

…tainability

Extract hardcoded trace header strings ('sentry-trace', 'baggage', 'traceparent') into Constants class to improve code maintainability and consistency across the Sentry tracing implementation.
@coderabbitai
Copy link

coderabbitai bot commented Oct 12, 2025

Walkthrough

在常量类中新增了 SENTRY_TRACE、BAGGAGE、TRACEPARENT 三个公共常量;各追踪切面与工具类改用上述常量替代硬编码字符串;Grpc/Guzzle 不再注入 traceparent 头;RpcAspect 复用 RPC 上下文并单次构造 Carrier。

Changes

Cohort / File(s) Summary
Constants 扩展
src/sentry/src/Constants.php
新增公共常量:SENTRY_TRACE、BAGGAGE、TRACEPARENT。
Tracing 切面(HTTP/GRPC/RPC)
src/sentry/src/Tracing/Aspect/GrpcAspect.php, src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php, src/sentry/src/Tracing/Aspect/RpcAspect.php
统一使用 Constants 替代头部键字面量;Grpc/Guzzle 不再注入 traceparent;RpcAspect 复用容器内 Rpc\Context,单次从 Span 生成 Carrier,并写入 TRACE_CARRIER。
Carrier 工具与打包
src/sentry/src/Util/Carrier.php, src/sentry/src/Util/CarrierPacker.php
sentry-tracebaggagetraceparent 访问与读写改为使用 Constants;fromRequest/pack/unpack 对应键名访问统一到常量。

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant A as Caller
  participant RA as RpcAspect
  participant S as Sentry Span
  participant C as Carrier
  participant RC as Rpc\Context

  A->>RA: 发起 RPC 调用
  RA->>S: 创建/获取当前 Span
  RA->>C: Carrier::fromSpan(Span)
  RA->>RC: 获取 Rpc\Context
  RA->>RC: set(TRACE_CARRIER, Carrier.toJson())
  Note right of RA: 头部/载体键名使用 Constants(SENTRY_TRACE/BAGGAGE/TRACEPARENT)
  RA-->>A: 继续调用链
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • guandeng
  • xuanyanwow

Poem

小兔举耳听风铃,
头名不再写死名;
一枚载体随链行,
行囊 baggage 轻又轻。
traceparent悄然隐,
sentry-trace常量鸣。
(=^•͈◡•͈=)🎐

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.69% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed 该标题准确概括了此次重构的核心内容,即将 Sentry 跟踪头常量集中管理以提高可维护性,并且采用了简洁清晰的描述方式。
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/sentry-trace-constants

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.30)

At least one path must be specified to analyse.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@huangdijia huangdijia merged commit 47f3ea4 into main Oct 12, 2025
16 of 17 checks passed
@huangdijia huangdijia deleted the refactor/sentry-trace-constants branch October 12, 2025 12:50
huangdijia added a commit that referenced this pull request Oct 12, 2025
…tainability (#946)

Extract hardcoded trace header strings ('sentry-trace', 'baggage', 'traceparent') into Constants class to improve code maintainability and consistency across the Sentry tracing implementation.

Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/sentry/src/Tracing/Aspect/RpcAspect.php (1)

104-113: 优化:复用 RPC 上下文与 Carrier 对象

此重构将 Carrier 的创建提前并复用,避免了重复构造。同时从容器中获取 RPC 上下文实例并直接使用其数据,代码更简洁高效。

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5617377 and 3560255.

📒 Files selected for processing (6)
  • src/sentry/src/Constants.php (1 hunks)
  • src/sentry/src/Tracing/Aspect/GrpcAspect.php (2 hunks)
  • src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php (2 hunks)
  • src/sentry/src/Tracing/Aspect/RpcAspect.php (1 hunks)
  • src/sentry/src/Util/Carrier.php (4 hunks)
  • src/sentry/src/Util/CarrierPacker.php (3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/*/src/**/*.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use the namespace pattern FriendsOfHyperf{ComponentName} in all component PHP source files

Files:

  • src/sentry/src/Constants.php
  • src/sentry/src/Tracing/Aspect/RpcAspect.php
  • src/sentry/src/Util/CarrierPacker.php
  • src/sentry/src/Util/Carrier.php
  • src/sentry/src/Tracing/Aspect/GrpcAspect.php
  • src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php
{src,tests}/**/*.php

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Adhere to PSR-12 coding standards across PHP code

Files:

  • src/sentry/src/Constants.php
  • src/sentry/src/Tracing/Aspect/RpcAspect.php
  • src/sentry/src/Util/CarrierPacker.php
  • src/sentry/src/Util/Carrier.php
  • src/sentry/src/Tracing/Aspect/GrpcAspect.php
  • src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php
src/*/**

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

New components must follow the standard component structure under src/{component-name}/ including .gitattributes, .github, LICENSE, README.md, composer.json, and a src/ subdirectory

Files:

  • src/sentry/src/Constants.php
  • src/sentry/src/Tracing/Aspect/RpcAspect.php
  • src/sentry/src/Util/CarrierPacker.php
  • src/sentry/src/Util/Carrier.php
  • src/sentry/src/Tracing/Aspect/GrpcAspect.php
  • src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php
🧬 Code graph analysis (5)
src/sentry/src/Tracing/Aspect/RpcAspect.php (2)
src/sentry/src/Util/Carrier.php (4)
  • get (119-122)
  • Carrier (25-142)
  • fromSpan (55-62)
  • toJson (134-141)
src/sentry/src/Constants.php (1)
  • Constants (14-27)
src/sentry/src/Util/CarrierPacker.php (1)
src/sentry/src/Constants.php (1)
  • Constants (14-27)
src/sentry/src/Util/Carrier.php (2)
src/sentry/src/Constants.php (1)
  • Constants (14-27)
src/http-client/src/Request.php (1)
  • hasHeader (76-91)
src/sentry/src/Tracing/Aspect/GrpcAspect.php (1)
src/sentry/src/Constants.php (1)
  • Constants (14-27)
src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php (1)
src/sentry/src/Constants.php (1)
  • Constants (14-27)
⏰ 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). (10)
  • 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 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.2
  • GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
🔇 Additional comments (12)
src/sentry/src/Constants.php (1)

22-26: 常量定义清晰合理

新增的三个追踪头常量命名规范,值与标准 HTTP 头保持一致。这些常量将在整个代码库中统一使用,有效提升了可维护性。

src/sentry/src/Tracing/Aspect/GrpcAspect.php (2)

14-14: 引入常量类以消除硬编码

引入 Constants 类为后续使用常量替代字符串字面量做准备,改进了代码可维护性。


56-59: 追踪头注入改用常量

sentry-tracebaggage 头的注入从硬编码字符串改为常量引用,降低了拼写错误风险,同时提升了 IDE 支持和代码一致性。

src/sentry/src/Tracing/Aspect/GuzzleHttpClientAspect.php (2)

14-14: 统一使用常量类

与其他追踪切面保持一致,引入 Constants 以支持常量化的头键名。


78-81: 头注入逻辑改用常量

追踪上下文头的注入现在使用常量引用,与 GrpcAspect 保持一致,提升了整体代码质量。

src/sentry/src/Util/CarrierPacker.php (3)

14-14: 引入常量以替换硬编码键名

尽管此类已标记为废弃(v3.1),但为保持一致性仍更新为使用常量,这是良好的维护实践。


32-34: 解包逻辑改用常量

unpack 方法现在通过常量访问键名,与新的常量体系保持一致。


43-48: 打包逻辑改用常量

pack 方法现在使用常量作为数组键,确保与代码库其他部分的一致性。

src/sentry/src/Util/Carrier.php (4)

27-29: 构造函数格式调整

微小的格式调整,无功能影响。


57-61: fromSpan 方法改用常量

从 Span 创建 Carrier 时,现在使用常量作为数据键,确保与其他组件的一致性。


67-89: fromRequest 方法全面采用常量

请求头的检查、提取和结果数组的构造均已改用常量引用,大幅提升了代码的可维护性和一致性。


99-112: Getter 方法改用常量访问

getSentryTracegetBaggagegetTraceparent 方法现在通过常量访问内部数据,与整体重构方向一致。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants