fix(sentry): cap spans per transaction to bound memory - #1083
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)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
b43c559 to
ab8fadd
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b43c559545
ℹ️ 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".
| $hub->configureScope(static function (Scope $scope) use ($trace, &$result) { | ||
| $result = $trace($scope); |
There was a problem hiding this comment.
Isolate skipped callbacks from the active span
When max_spans is exhausted, configureScope() passes the current active scope to the callback, so callbacks that expect a newly created span instead mutate the parent transaction or span. For example, Tracing/Listener/EventHandleListener.php sets Redis result data and error status on $scope->getSpan(), while GuzzleHttpClientAspect.php adds downstream response data and status; every over-budget operation can therefore overwrite or incorrectly mark its parent. Execute skipped callbacks with a span-less isolated scope rather than the active scope.
Useful? React with 👍 / 👎.
| // A value of 0 (or negative) disables the limit. | ||
| 'max_spans' => (int) env('SENTRY_MAX_SPANS', 1000), |
There was a problem hiding this comment.
Honor the SDK semantics when disabling the custom budget
max_spans is also a Sentry SDK option: ClientBuilderFactory retains every key defined by Sentry\Options and forwards it to ClientBuilder. Consequently, setting SENTRY_MAX_SPANS=0 makes this custom budget unlimited but gives the SDK recorder a literal zero-span cap, so the resulting transactions record no child spans instead of being unlimited as advertised. Use a component-specific option or avoid forwarding the disabling value to the SDK.
Useful? React with 👍 / 👎.
| // budget is exhausted new spans are skipped (the callable still runs) to | ||
| // prevent the span tree of a long-lived coroutine from growing unboundedly. | ||
| // A value of 0 (or negative) disables the limit. | ||
| 'max_spans' => (int) env('SENTRY_MAX_SPANS', 1000), |
There was a problem hiding this comment.
Document the span budget in every Sentry documentation source
This adds a user-facing tracing configuration key and environment variable, but neither component README nor any of the four locale Sentry pages includes max_spans or SENTRY_MAX_SPANS. The repository specifically requires tracing changes to remain synchronized across all four pages and both READMEs, so users following the documented configuration cannot discover this new default or its opt-out behavior.
AGENTS.md reference: AGENTS.md:L129-L130
Useful? React with 👍 / 👎.
问题
长驻协程反复创建子协程时父事务 span 树无界增长
修改
新增 SpanBudget 按事务限制 span 数(max_spans 默认 1000,<=0 不限制)
测试
vendor/bin/pest --group=sentry 全通过