1306: Adding support for composite sampling policy to the tailsampler #4396
Conversation
|
processor/tailsamplingprocessor/internal/sampling/composite_test.go
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| // Now we hit the rate limit, so subsequent evaluations should result in 100% NotSampled | ||
| for i := 0; i < totalSPS; i++ { |
There was a problem hiding this comment.
You just need one more iteration, don't you?
There was a problem hiding this comment.
The same applies to further places in this PR.
There was a problem hiding this comment.
@jpkrohling Maybe i am missing something here. Why do we need one more iteration ? We are running above loop to assert that "Once the totalSPS capacity is exhausted [i.e. rate_limts], we deliberately mark the matching spans a Not_Sampled"
processor/tailsamplingprocessor/internal/sampling/composite_test.go
Outdated
Show resolved
Hide resolved
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
@jpkrohling thanks for the review. Sorry for the delay. Let me take care of the comments shortly and update the PR |
|
/easycla |
|
@vikrambe, once this is ready for another review, please ping me (or click "request re-review") |
|
@jpkrohling Had a issue with EasyCLA and was running into rebase issue. I have opened a new PR #4958 - Its continuation of this PR. |
Is your feature request related to a problem? Please describe. #1410 Design Doc - Added support for composite policy in tailsampling processor. This would help in grouping sampling policies and rate limiting them. https://docs.google.com/document/d/10wpIv3TtXgOik05smHm3nYeBX48Bj76TCMxPy8e1NZw/edit#heading=h.ecy5l2puwtp4 This is a split. Refer PR open-telemetry/opentelemetry-collector#1894 (comment) Due to EasyCLA issue opening a new PR: #4396 Link to tracking Issue: 1306
Is your feature request related to a problem? Please describe.
#1410
Design Doc - Added support for composite policy in tailsampling processor. This would help in grouping sampling policies and rate limiting them. https://docs.google.com/document/d/10wpIv3TtXgOik05smHm3nYeBX48Bj76TCMxPy8e1NZw/edit#heading=h.ecy5l2puwtp4
This is a split. Refer PR open-telemetry/opentelemetry-collector#1894 (comment)
Link to tracking Issue: 1306