Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add GetSampleRateMulti(key, count) #50

Closed
kentquirk opened this issue Mar 7, 2023 · 1 comment
Closed

feat: Add GetSampleRateMulti(key, count) #50

kentquirk opened this issue Mar 7, 2023 · 1 comment
Assignees

Comments

@kentquirk
Copy link
Contributor

Is your feature request related to a problem? Please describe.

The samplers in this repository are designed to be called for each span individually, and increment their counters with ++. But in Refinery, the samplers are called once per trace. Each trace accounts for a number of spans, but that's not included in the calculations. Thus, throughput sampling in particular is potentially very wrong.

Describe the solution you'd like

Add a new function that takes count as a parameter, and implement the existing GetSampleRate call in terms of that function, passing 1 as the second parameter.

Describe alternatives you've considered

Calling the samplers once per span and ignoring most of the results -- but that's an expensive alternative.

Additional context

@kentquirk
Copy link
Contributor Author

Fixed by #53.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant