refactor: remove ensureSampled, use natural OTel root spans#7
Conversation
…r ticks The Tracing middleware previously injected a fake remote sampled span context to force ParentBased samplers to always sample worker spans. Remove this hack and let NewInternalSpan create a natural root span per tick, with sampling governed by the global TracerProvider's sampler.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe tracing middleware is simplified by removing the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@middleware/tracing.go`:
- Around line 12-14: Docstring for the exported Tracing() function was changed
to mention "natural OTel root spans / global sampler"; run the documentation
task to regenerate the public README so the published docs match the updated
docstring by running the project's doc generation (make doc which invokes
gomarkdoc), verify README.md updates reflect the new wording for Tracing(), and
commit the regenerated README.md alongside your change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1b1a85f8-5062-40b1-88cd-7a0a3834ee68
📒 Files selected for processing (2)
middleware/tracing.gomiddleware/tracing_test.go
💤 Files with no reviewable changes (1)
- middleware/tracing_test.go
There was a problem hiding this comment.
Pull request overview
This PR removes the ensureSampled span-context injection so worker-cycle spans are created as “natural” spans (without a synthetic sampled remote parent), allowing sampling decisions to be governed by the configured global OpenTelemetry TracerProvider sampler.
Changes:
- Removed
ensureSampledlogic (and itscrypto/randusage) from theTracing()middleware. - Updated
Tracing()documentation to reflect sampler-governed behavior. - Removed
TestEnsureSampled*tests and related unused imports.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| middleware/tracing.go | Stops forcing sampling via injected remote span context; relies on global OTel sampler and updates comments accordingly. |
| middleware/tracing_test.go | Removes tests and imports that only validated the deleted ensureSampled helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
ensureSampledhack that injected a fake remote sampled span context to forceParentBasedsamplers to always sample worker spansTracing()middleware now letsNewInternalSpancreate a natural OTel root span per tick, with sampling governed by the global TracerProvider's samplerTestEnsureSampled*) and unused importsTest plan
make testpasses (all tests green with-race)make lintclean (golangci-lint + govulncheck)Summary by CodeRabbit