Skip to content

Conversation

@huangdijia
Copy link
Contributor

@huangdijia huangdijia commented Nov 11, 2025

Summary

This PR removes deprecated and unused features from the Sentry integration to simplify the codebase:

  • Remove enable_tracing configuration option from sentry.php (no longer needed)
  • Remove Metrics-related singleton aspects (Metrics::getInstance and MetricsUnit::getInstance)
  • Remove W3C traceparent generation from Carrier and CarrierPacker utilities

Changes

Configuration (src/sentry/publish/sentry.php)

  • Removed the enable_tracing config option as it's deprecated/unused

Singleton Aspects (src/sentry/src/Aspect/SingletonAspect.php)

  • Removed \Sentry\Metrics\Metrics::class . '::getInstance'
  • Removed \Sentry\Metrics\MetricsUnit::class . '::getInstance'

Carrier Utilities (src/sentry/src/Util/Carrier.php & CarrierPacker.php)

  • Removed Constants::TRACEPARENT (W3C traceparent) from carrier data
  • Kept Sentry-specific trace headers (SENTRY_TRACE and BAGGAGE)

Benefits

  • Cleaner codebase with fewer unused features
  • Reduced complexity in carrier handling
  • Better alignment with current Sentry SDK usage patterns

Test Plan

  • Existing tests should continue to pass
  • Verify Sentry tracing still works correctly without the removed configuration
  • Confirm carrier propagation functions as expected

Summary by CodeRabbit

版本更新说明

  • 重构
    • 移除用于启用追踪的配置项,改由采样率控制
    • 精简单例管理中不再维护的类引用
    • 删除数据载体中对 traceparent 字段的写入,精简传输结构
  • 测试
    • 更新相关单元测试以匹配删减后的载体输出字段

- Remove `enable_tracing` config option from sentry.php (deprecated/unused)
- Remove Metrics-related singleton aspects (Metrics::getInstance and MetricsUnit::getInstance)
- Remove W3C traceparent from Carrier and CarrierPacker utilities

This cleanup simplifies the Sentry integration by removing features that are no longer needed.
@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Walkthrough

移除 Sentry PHP 配置中的 enable_tracing,从单例配置中删除两个 Metrics 相关类引用,并在 Carrier/CarrierPacker 中停止输出 TRACEPARENT 字段;相应测试更新以反映该字段不再由 Span 构造的载体中产生。

Changes

内聚组 / 文件(s) 变更摘要
配置移除
src/sentry/publish/sentry.php
从返回的配置数组中删除 enable_tracing 键及其默认值,只保留 sample_ratetraces_sample_rate
单例类清理
src/sentry/src/Aspect/SingletonAspect.php
public $classes 配置中移除 Sentry\Metrics\MetricsSentry\Metrics\MetricsUnitgetInstance 引用
追踪载体调整
src/sentry/src/Util/Carrier.php, src/sentry/src/Util/CarrierPacker.php, tests/Sentry/CarrierTest.php
Carrier::fromSpanCarrierPacker::pack 不再包含 TRACEPARENT(仅保留 SENTRY_TRACEBAGGAGE);相应测试移除对 traceparent 键的期望

Sequence Diagram(s)

sequenceDiagram
    participant Span
    participant Carrier
    participant CarrierPacker
    note over Span,Carrier `#E8F6EF`: 旧流程(之前)
    Span->>Carrier: fromSpan() -> [SENTRY_TRACE, BAGGAGE, TRACEPARENT]
    Carrier->>CarrierPacker: pack([SENTRY_TRACE, BAGGAGE, TRACEPARENT])
    CarrierPacker->>CarrierPacker: json_encode([...]) -> payload

    par 新流程(当前改动)
        Span->>Carrier: fromSpan() -> [SENTRY_TRACE, BAGGAGE]
        Carrier->>CarrierPacker: pack([SENTRY_TRACE, BAGGAGE])
        CarrierPacker->>CarrierPacker: json_encode([...]) -> payload
    end
    note right of CarrierPacker `#FFF4E6`: unpack 仍可读取第三元素(兼容外部提供的 TRACEPARENT)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 分钟

  • 需重点检查:
    • CarrierPacker::unpack 读取第三元素的行为与 pack 不再写入 TRACEPARENT 的兼容性(兼容外部载体的场景)
    • 代码库或配置处是否仍有对 enable_tracing 的访问点导致运行时错误
    • 其他模块对被移除的 Metrics 单例引用的潜在调用或隐式依赖

Possibly related PRs

Suggested reviewers

  • xuanyanwow

🐰 追踪字段悄然去,
TRACEPARENT 留空处,
配置少了一行字,
单例轻了两笔书,
小兔跳跃庆简洁。

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题清晰地总结了PR的主要变化,即移除弃用的特性和未使用的配置。标题与所有文件中的具体更改(移除enable_tracing、单例引用和traceparent字段)直接相关。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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-remove-deprecated-features

📜 Recent 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 83d2f7a and 5c2cfd6.

📒 Files selected for processing (1)
  • tests/Sentry/CarrierTest.php (2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.php

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

**/*.php: Code style must follow PSR-12 and be formatted by PHP-CS-Fixer per .php-cs-fixer.php
Run PHPStan and keep the codebase passing per phpstan.neon.dist

Files:

  • tests/Sentry/CarrierTest.php
tests/**/*.php

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

All tests should reside under tests/ and use PHPUnit following Hyperf testing patterns

Files:

  • tests/Sentry/CarrierTest.php
⏰ 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). (16)
  • GitHub Check: Test on PHP 8.1 with Swoole 6.1.1
  • GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
  • GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
  • GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
  • GitHub Check: Test on PHP 8.2 with Swoole 6.1.1
  • GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
  • GitHub Check: Test on PHP 8.3 with Swoole 6.1.1
  • GitHub Check: Test on PHP 8.1 with Swoole 6.1.1
  • GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
  • GitHub Check: Test on PHP 8.2 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.1.1
  • GitHub Check: Test on PHP 8.1 with Swoole 5.1.8
  • GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
  • GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
  • GitHub Check: Test on PHP 8.3 with Swoole 6.1.1
🔇 Additional comments (2)
tests/Sentry/CarrierTest.php (2)

65-75: 测试更新正确反映了实现变更。

此测试用例现在仅期望从 Span 创建的 Carrier 包含 sentry-tracebaggage 键,不再包含 traceparent 键。这与 PR 目标一致:移除 W3C traceparent,仅保留 Sentry 特定的追踪头。

注意:Carrier 类仍然支持手动设置的 traceparent(见 lines 135-143 的测试),这为向后兼容性提供了保障。


280-296: 集成测试更新一致且完整。

集成测试正确验证了从 Span 创建 Carrier 并添加自定义数据的场景。更新后的断言(line 289)与 line 72 的更改保持一致,仅期望 Sentry 特定的追踪头。

测试还包含了 JSON 序列化往返验证,确保数据完整性得到保持。

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

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 60bbf2a into main Nov 11, 2025
22 checks passed
@huangdijia huangdijia deleted the refactor/sentry-remove-deprecated-features branch November 11, 2025 04:46
huangdijia added a commit that referenced this pull request Nov 11, 2025
…#990)

* refactor(sentry): remove deprecated features and unused configuration

- Remove `enable_tracing` config option from sentry.php (deprecated/unused)
- Remove Metrics-related singleton aspects (Metrics::getInstance and MetricsUnit::getInstance)
- Remove W3C traceparent from Carrier and CarrierPacker utilities

This cleanup simplifies the Sentry integration by removing features that are no longer needed.

* fix(tests): remove traceparent key from Carrier data expectations

---------

Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
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