fix(sentry): flush global telemetry buffers periodically - #1087
fix(sentry): flush global telemetry buffers periodically#1087huangdijia wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 58 minutes Limit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f848978bbc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return [ | ||
| MetricFactoryReady::class, | ||
| ]; |
There was a problem hiding this comment.
With the published defaults (enable_logs=true, enable_metrics=false), this listener is never invoked: every production dispatch of MetricFactoryReady is gated by isMetricsEnabled() or isCommandMetricsEnabled(). Moreover, OnWorkerStart dispatches it only for worker 0, so the other worker processes receive no timer even when metrics are enabled. Consequently, the new fallback does not flush global log buffers in the default configuration or across all workers, leaving the reported log-loss/unbounded-memory scenario unresolved; start it from a lifecycle event that runs in every relevant process or arrange an unconditional per-worker dispatch.
Useful? React with 👍 / 👎.
| // Reaching the threshold triggers an automatic flush, and the telemetry | ||
| // flush listener also periodically flushes as a fallback. Memory usage | ||
| // grows linearly with the threshold, so keep it <= 5000. |
There was a problem hiding this comment.
This adds user-facing behavior and threshold guidance to the published configuration, but both component READMEs and all four localized Sentry pages still omit log_flush_threshold, the periodic fallback, and its memory recommendation. Update those six sources so users who follow the package documentation receive the same telemetry behavior and configuration guidance.
AGENTS.md reference: AGENTS.md:L129-L130
Useful? React with 👍 / 👎.
1 participant
问题
Sentry SDK 的 Logs/Metrics 聚合器归属"当前 runtime context"(见
vendor/sentry/sentry/src/Logs/Logs.php、Metrics/TraceMetrics.php)。没有 active context 的协程(引擎协程、主进程等)写入 global context 的聚合器:LogsAggregator::LOGS_BUFFER_SIZE),不会 OOM 但会静默丢弃日志。log_flush_threshold/metric_flush_threshold后,存储变为无界数组(Util/TelemetryStorage::unbounded()),仅在达到阈值时 flush —— 阈值过大时存在内存溢出风险。修改
Metrics\Listener\TelemetryFlushListener:监听MetricFactoryReady,按metrics_interval周期 tick,对 global context 的日志/指标聚合器执行 flush 兜底(用SentrySdk::endContext()结束 tick 协程自身 context 后再 flush,确保落在 global context;每个 flush 独立 try/catch,幂等无副作用)。ConfigProvider注册新 listener(默认优先级)。Feature新增isLogsEnabled(bool $default = true)读取sentry.enable_logs,与isMetricsEnabled保持一致;二者任一开启即启动周期 flush。publish/sentry.php为log_flush_threshold补充注释:达到阈值自动 flush、telemetry flush listener 周期兜底、内存占用与阈值成正比建议 <= 5000(默认值不变)。tests/Sentry/Metrics/Listener/TelemetryFlushListenerTest.php(FakeTimer 不 spawn 协程):重复 process 只 tick 一次;tick 闭包可安全调用(空聚合器 flush 返回 null);logs/metrics 均关闭时零 tick。测试
vendor/bin/pest --group=sentry:46 passed(基线 43 + 新增 3)。vendor/bin/php-cs-fixer fix --dry-run --diff:改动文件 0 处可修复。vendor/bin/phpstan analyse src/sentry:No errors。验证
git diff --check通过;commitf848978b;分支sentry-pr/r5已推送。