This repository has been archived by the owner on Dec 6, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Composite Samplers #250
Composite Samplers #250
Changes from 3 commits
bfd33d9
611a140
16005a5
772ef28
395bd0c
fcb6a4b
9495d33
b37f054
7bcd40b
c72b06a
7cb67c1
ab8e962
47084f2
a6d3705
cae3b74
0d3ce18
9a7afb3
129c99c
10ba0c1
1c15c2b
a0bfb1b
e544794
635b216
06cc32d
5cc2174
d3c82a3
bfc3081
ed2a314
424012e
9c47d0f
05943e9
4c40e85
577537e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will result in poor observability as the underlying sampler can be a primitive one so the only attributes it will be able to add will be related to its algorithm, not the fact that a specific predicate was matched. I would much rather see an ability for the user to give names to individual decision points (applies to all composite samplers). In addition to attributes these samplers could be emitting metrics with corresponding names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting ideas - do you have a specific use case in mind?
Anyway, I do not quite agree with your claim of poor observability. Most Predicates are expected to test span attributes only and therefore their underlying logic can be re-evaluated at any later time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the example at the top you have English description of different categories of spans being sampled. As a user, I would appreciate a mechanism to not only keep those description (maybe in a shorter form of "names"), but also to have observability where I can see which triggers fire how often (either by recording the trigger name as span attribute or emitting metrics labeled with those names). Your current mechanism does not allow for that afaict.