Skip to content

Conversation

@huangdijia
Copy link
Contributor

@huangdijia huangdijia commented Sep 29, 2025

Summary

This PR optimizes the Sentry event flushing strategy by moving the responsibility from general event listeners to tracing-specific components, resulting in more efficient event handling.

Changes Made

  • Removed flushEvents() method and all its calls from EventHandleListener
  • Moved event flushing logic to tracing components (CoroutineAspect and EventHandleListener in tracing namespace)
  • Cleaned up commented code in CoroutineAspect files
  • Added event flushing at transaction completion boundaries

Benefits

  • Performance improvement: Events are now flushed only when tracing transactions complete, rather than after every event
  • Better resource management: Reduces unnecessary flush operations
  • Cleaner code: Removes unused methods and commented code
  • More targeted approach: Event flushing happens at appropriate transaction boundaries

Files Modified

  • src/sentry/src/Aspect/CoroutineAspect.php: Removed commented code
  • src/sentry/src/Listener/EventHandleListener.php: Removed flushEvents() method and calls
  • src/sentry/src/Tracing/Aspect/CoroutineAspect.php: Added event flushing in defer block
  • src/sentry/src/Tracing/Listener/EventHandleListener.php: Added event flushing after transaction completion

Testing

This change maintains the same functionality while improving performance. Events will still be properly flushed, but at more appropriate times (when transactions complete).

Impact

  • No breaking changes
  • Backward compatible
  • Performance improvement for Sentry event handling

Summary by CodeRabbit

  • Bug Fixes

    • 在请求、命令、定时任务、消息与异步队列等追踪事务结束时主动刷新事件,降低事件丢失风险;异常仍会被捕获并刷新。
  • Refactor

    • 精简事件刷新流程,移除重复的全局刷新调用,减少不必要的开销。
    • 调整协程上下文的传播内容,提升上下文隔离与稳定性。

…ng components

- Remove flushEvents() method and calls from main EventHandleListener
- Move event flushing responsibility to tracing components where transactions finish
- Clean up commented code in CoroutineAspect files
- Ensure events are flushed at appropriate transaction boundaries for better performance

This change improves the efficiency of Sentry event handling by flushing events
only when tracing transactions complete, rather than after every event.
@coderabbitai
Copy link

coderabbitai bot commented Sep 29, 2025

Walkthrough

在常规与追踪两个模块中,调整协程上下文与事件处理的刷新策略:移除非追踪监听器中的批量 flush 方法与调用;在追踪相关的协程和事件 defer 收尾阶段完成事务后统一调用 Integration::flushEvents();精简协程上下文复制的键集合。

Changes

Cohort / File(s) Summary
Core Sentry 监听与协程切面
src/sentry/src/Aspect/CoroutineAspect.php, src/sentry/src/Listener/EventHandleListener.php
CoroutineAspect:从复制上下文的 $keys 中移除 SentrySdk::class。EventHandleListener:删除 flushEvents() 方法并移除其所有调用,保留异常捕获与内部 flush。
Tracing 协程切面与事件监听
src/sentry/src/Tracing/Aspect/CoroutineAspect.php, src/sentry/src/Tracing/Listener/EventHandleListener.php
引入 Integration,在协程/事件的 defer 收尾中:确保当前 scope 设置活动事务、完成事务后调用 Integration::flushEvents();在请求、命令、定时任务、AMQP、Kafka、异步队列等处理结束处新增统一 flush。

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant App as 应用代码
  participant Tracer as Tracing Listener/Aspect
  participant Scope as Sentry Scope
  participant Tx as Transaction
  participant Int as Integration

  App->>Tracer: 触发事件/进入协程
  Tracer->>Scope: 获取当前事务
  alt 存在事务
    Scope->>Tx: 使用活动事务
  else
    Note over Tracer: 无事务则仅继续执行
  end
  App-->>App: 执行业务逻辑
  rect rgba(230,245,255,0.6)
    note right of Tracer: defer 收尾(新/变更)
    Tracer->>Scope: 确保设置活动事务
    Tracer->>Tx: finish()
    Tracer->>Int: flushEvents()
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • guandeng
  • xuanyanwow

Poem

小兔敲键在云端,
协程轻拂把链串。
事务收尾一声唤,
flush 随后不迟缓。
监听精简风更暖,
日志如星落人间。 🐇✨

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 该标题准确概括了 PR 的核心变更,即将 Sentry 事件刷新逻辑从通用事件侦听器迁移到追踪组件以优化刷新策略,与实际代码修改保持一致,简洁且清晰地传达了主要意图。
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/sentry-event-flushing-optimization

📜 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 b5149cf and f526d1c.

📒 Files selected for processing (4)
  • src/sentry/src/Aspect/CoroutineAspect.php (0 hunks)
  • src/sentry/src/Listener/EventHandleListener.php (0 hunks)
  • src/sentry/src/Tracing/Aspect/CoroutineAspect.php (3 hunks)
  • src/sentry/src/Tracing/Listener/EventHandleListener.php (7 hunks)
💤 Files with no reviewable changes (2)
  • src/sentry/src/Listener/EventHandleListener.php
  • src/sentry/src/Aspect/CoroutineAspect.php
🧰 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/Tracing/Listener/EventHandleListener.php
  • src/sentry/src/Tracing/Aspect/CoroutineAspect.php
{src,tests}/**/*.php

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

Adhere to PSR-12 coding standards across PHP code

Files:

  • src/sentry/src/Tracing/Listener/EventHandleListener.php
  • src/sentry/src/Tracing/Aspect/CoroutineAspect.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/Tracing/Listener/EventHandleListener.php
  • src/sentry/src/Tracing/Aspect/CoroutineAspect.php
🧬 Code graph analysis (2)
src/sentry/src/Tracing/Listener/EventHandleListener.php (1)
src/sentry/src/Integration.php (2)
  • Integration (28-158)
  • flushEvents (106-115)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (2)
src/sentry/src/Integration.php (2)
  • Integration (28-158)
  • flushEvents (106-115)
src/sentry/class_map/SentrySdk.php (2)
  • SentrySdk (24-65)
  • getCurrentHub (51-54)
⏰ 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). (11)
  • 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 6.0.2
  • GitHub Check: Test on PHP 8.2 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
  • GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
  • 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 6.0.2
  • GitHub Check: Test on PHP 8.3 with Swoole 5.1.7

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

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 73917dc into main Sep 29, 2025
16 of 17 checks passed
@huangdijia huangdijia deleted the refactor/sentry-event-flushing-optimization branch September 29, 2025 05:49
huangdijia added a commit that referenced this pull request Sep 29, 2025
…ng components (#940)

- Remove flushEvents() method and calls from main EventHandleListener
- Move event flushing responsibility to tracing components where transactions finish
- Clean up commented code in CoroutineAspect files
- Ensure events are flushed at appropriate transaction boundaries for better performance

This change improves the efficiency of Sentry event handling by flushing events
only when tracing transactions complete, rather than after every event.

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