OtelTraceProvider.Builder introduce the trace rate limit property#2086
Conversation
d523380 to
46b4195
Compare
46b4195 to
83e190e
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2086 +/- ##
===========================================
+ Coverage 68.86% 69.00% +0.13%
===========================================
Files 708 708
Lines 26289 26297 +8
Branches 4424 4422 -2
===========================================
+ Hits 18103 18144 +41
+ Misses 6984 6951 -33
Partials 1202 1202
|
0xnm
left a comment
0xnm
left a comment
There was a problem hiding this comment.
lgtm, I've added some questions/comments
| * @param traceRateLimit the trace rate limit as a value between 1 and Int.MAX_VALUE (default is Int.MAX_VALUE) | ||
| */ | ||
| fun setTraceRateLimit( | ||
| @IntRange(from = 1, to = Int.MAX_VALUE.toLong()) traceRateLimit: Int |
There was a problem hiding this comment.
why min value is 1? shouldn't it be 0 instead to be consistent with sample rate? there we allow setting 0, meaning no traces will be sent.
There was a problem hiding this comment.
that's what they have in the code base
There was a problem hiding this comment.
it is a Math.max(1, traceLimit) that it's being used in that class so 0 will not be taken into account.
| } | ||
|
|
||
| @Test | ||
| @RepeatedTest(10) |
There was a problem hiding this comment.
do we have any added value from repeating this test, given that we already have quite large number of spans in the test itself?
There was a problem hiding this comment.
not quite...you are right, going to lower it.
83e190e to
275c4b3
Compare
5 participants
What does this PR do?
A brief description of the change being made with this pull request.
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)