Skip to content

Conversation

@huangdijia
Copy link
Contributor

@huangdijia huangdijia commented Nov 21, 2025

Summary

This PR improves database connection tracing by enhancing the accuracy of server address and port detection for read/write connections.

Changes

  • Added TRACE_DB_USE_READ_PDO constant to track read PDO usage
  • Enhanced DbConnectionAspect to detect getPdoForSelect method calls and track the useReadPdo parameter
  • Improved EventHandleListener to properly determine server port based on read/write configuration and host matching
  • Fixed port detection for read replicas in database tracing

Problem Solved

Previously, the tracing system had difficulty accurately determining the correct server port for read/write connections, especially in environments with separate read/write hosts and ports. This could lead to incorrect server address and port information being recorded in traces.

Implementation Details

The solution tracks whether a read PDO is being used and then looks up the appropriate host/port configuration from the database config based on the read/write context. This ensures that:

  1. Read connections are traced with the correct read replica host/port
  2. Write connections are traced with the correct write host/port
  3. Fallback behavior is maintained when specific configuration is not available

Test Plan

  • Verify tracing works correctly with read/write database configurations
  • Confirm server address and port are accurately recorded for both read and write operations
  • Test fallback behavior when read/write hosts are not explicitly configured
  • Validate no regression in existing tracing functionality

Related Issues

This addresses tracing accuracy issues in environments with separate read/write database configurations.

Summary by CodeRabbit

发布说明

  • 新功能

    • 支持区分读写副本的追踪标记,增强对读副本使用情况的识别
  • 改进

    • 动态计算并填充数据库服务器主机与端口信息(在未显式配置端口时会基于读/写选择逻辑推断),使追踪记录中的服务器信息更准确和一致

✏️ Tip: You can customize this high-level summary in your review settings.

- Add TRACE_DB_USE_READ_PDO constant to track read PDO usage
- Enhance DbConnectionAspect to detect getPdoForSelect method calls
- Improve EventHandleListener to properly determine server port based on
  read/write configuration and host matching
- Fix port detection for read replicas in database tracing

This ensures accurate server address and port tracing for both read
and write database connections, especially in environments with
separate read/write hosts and ports.
@coderabbitai
Copy link

coderabbitai bot commented Nov 21, 2025

Warning

Rate limit exceeded

@huangdijia has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 57 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 1aec424 and e60fe6a.

📒 Files selected for processing (1)
  • src/sentry/src/Tracing/Listener/EventHandleListener.php (1 hunks)

Walkthrough

添加了常量 TRACE_DB_USE_READ_PDO,在 DbConnectionAspect 中捕获并设置该标志(来自 getPdoForSelect 的 useReadPdo 参数),并在 EventHandleListener 中基于此标志动态推断并填充数据库服务器的 host/port 信息。

Changes

Cohort / File(s) 变更摘要
常量定义
src/sentry/src/Constants.php
新增公共常量 TRACE_DB_USE_READ_PDO ('sentry.tracing.db.use_read_pdo')
数据库连接切面
src/sentry/src/Tracing/Aspect/DbConnectionAspect.php
process 方法中,闭包现在通过 use ($proceedingJoinPoint) 捕获切入点;当方法为 getPdoForSelect 时,从参数提取 useReadPdo 并将其写入 Context(TRACE_DB_USE_READ_PDO
事件监听处理
src/sentry/src/Tracing/Listener/EventHandleListener.php
在处理 DB 查询事件时,计算并填充 server.addressserver.port:先取 TRACE_DB_SERVER_ADDRESS(默认 localhost)作为主机,若 TRACE_DB_SERVER_PORT 未设置则从数据库配置中根据 TRACE_DB_USE_READ_PDO 选择读/写 host/port(匹配当前 host 的端口或回退到 3306),否则使用显式端口值;保留原有 pool/metadata 与 sql.bindings 行为

Sequence Diagram(s)

sequenceDiagram
    participant Caller as 调用者
    participant Aspect as DbConnectionAspect
    participant Context as Context
    participant Listener as EventHandleListener
    participant Config as DB配置

    Caller->>Aspect: 调用 getPdoForSelect(..., useReadPdo)
    Note right of Aspect: 闭包捕获 $proceedingJoinPoint
    Aspect->>Context: 设置 TRACE_DB_USE_READ_PDO = useReadPdo
    Aspect-->>Caller: 返回 PDO

    Caller->>Listener: 触发 handleDbQueryExecuted 事件
    Listener->>Context: 读取 TRACE_DB_USE_READ_PDO
    alt TRACE_DB_USE_READ_PDO == true
        Listener->>Config: 获取读库 hosts/ports
    else TRACE_DB_USE_READ_PDO == false
        Listener->>Config: 获取写库 hosts/ports
    end
    Config-->>Listener: 返回 host/port(或无)
    Listener->>Listener: 计算最终 server.address 与 server.port(使用显式或推断或回退)
    Listener-->>Collector: 发送事件载荷(含 server.address/server.port)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

需要额外关注的点:

  • DbConnectionAspect.php 中闭包对 $proceedingJoinPoint 的捕获是否安全且无副作用。
  • Context 中 TRACE_DB_USE_READ_PDO 的写入/生命周期,确保不会跨请求污染。
  • EventHandleListener.php 中从 DB 配置匹配 host/port 的逻辑分支与回退值(3306)是否覆盖所有部署情形。

Possibly related PRs

Poem

兔子说:代码里藏着洞,
我轻敲常量添一行,
读写之路分明朗,🐰
主机端口随风量,
追踪更近,心更安。

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 The pull request title directly and concisely describes the main objective of the changeset: improving database read/write connection tracing. All changes across multiple files support this core purpose.

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.

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: 4

📜 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 e7a25ef and 5424af3.

📒 Files selected for processing (3)
  • src/sentry/src/Constants.php (1 hunks)
  • src/sentry/src/Tracing/Aspect/DbConnectionAspect.php (1 hunks)
  • src/sentry/src/Tracing/Listener/EventHandleListener.php (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/sentry/src/Tracing/Listener/EventHandleListener.php (2)
src/sentry/src/Util/Carrier.php (2)
  • get (118-121)
  • has (113-116)
src/sentry/src/Constants.php (1)
  • Constants (14-43)
src/sentry/src/Tracing/Aspect/DbConnectionAspect.php (2)
src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
  • process (123-168)
src/sentry/src/Constants.php (1)
  • Constants (14-43)
⏰ 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). (6)
  • GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
  • GitHub Check: Test on PHP 8.2 with Swoole 6.1.2
  • GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
  • GitHub Check: Test on PHP 8.1 with Swoole 6.1.2
  • GitHub Check: Test on PHP 8.3 with Swoole 5.1.8
  • GitHub Check: Test on PHP 8.2 with Swoole 5.1.8
🔇 Additional comments (2)
src/sentry/src/Constants.php (1)

18-19: 常量定义符合规范

新增的 TRACE_DB_USE_READ_PDO 常量定义清晰,遵循了现有的命名模式,用于标识是否使用读取 PDO 连接。

src/sentry/src/Tracing/Listener/EventHandleListener.php (1)

213-214: 服务器地址和端口的动态计算实现正确

成功地将静态的 TRACE_DB_SERVER_ADDRESSTRACE_DB_SERVER_PORT 替换为基于读写配置动态计算的 $host$port,这样可以准确反映读写副本的实际连接信息。

@huangdijia huangdijia merged commit 2253034 into main Nov 21, 2025
4 checks passed
@huangdijia huangdijia deleted the fix/db-read-write-tracing-improvements branch November 21, 2025 08:25
huangdijia added a commit that referenced this pull request Nov 21, 2025
* fix: improve database read/write connection tracing

- Add TRACE_DB_USE_READ_PDO constant to track read PDO usage
- Enhance DbConnectionAspect to detect getPdoForSelect method calls
- Improve EventHandleListener to properly determine server port based on
  read/write configuration and host matching
- Fix port detection for read replicas in database tracing

This ensures accurate server address and port tracing for both read
and write database connections, especially in environments with
separate read/write hosts and ports.

* fix: 优化数据库连接主机和端口的选择逻辑

* fix: 优化数据库配置获取逻辑以支持更灵活的连接设置

* fix: 使用 array_search 优化数据库主机查找逻辑

* fix: 修复数据库配置获取逻辑以使用正确的主机地址

---------

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