Emit warning when TraceIdRatioBasedSampler is used as child sampler #7937
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7937 +/- ##
=========================================
Coverage 90.09% 90.10%
- Complexity 7432 7438 +6
=========================================
Files 834 834
Lines 22537 22546 +9
Branches 2236 2236
=========================================
+ Hits 20304 20314 +10
Misses 1532 1532
+ Partials 701 700 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Gosling-dude
force-pushed
the
warn-child-traceidratio
branch
from
6e2eef7 to
db30494
Compare
|
All required checks are passing. This implements the spec-required compatibility warning for TraceIdRatioBasedSampler when used as a child sampler. Logging is limited to ParentBasedSamplerBuilder and does not affect sampling behavior. Happy to adjust if there are any suggestions. |
| if (!warnedRemoteParentSampled && remoteParentSampled instanceof TraceIdRatioBasedSampler) { | ||
| warnedRemoteParentSampled = true; | ||
| logger.warning( | ||
| "TraceIdRatioBasedSampler is being used as a child sampler (remoteParentSampled). " | ||
| + "This configuration is discouraged per the OpenTelemetry specification " | ||
| + "and may lead to unexpected sampling behavior."); | ||
| } |
There was a problem hiding this comment.
This is generally only going to be done once for a given builder instance. I don't think we need the extra checks to make sure it only gets logged once.
There was a problem hiding this comment.
That makes sense - agreed.
I’ll remove the warned* guards and emit the warning unconditionally when a TraceIdRatioBasedSampler is set as a child sampler. That keeps the builder simpler and aligns with typical usage.
I’ll push a small follow-up update shortly.
2 participants
What does this change do?
Why is this needed?
Implementation notes:
How was this tested?
Closes #7790