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: EMAThroughput sampler #58

Merged
merged 3 commits into from
Mar 21, 2023
Merged

feat: EMAThroughput sampler #58

merged 3 commits into from
Mar 21, 2023

Conversation

kentquirk
Copy link
Contributor

@kentquirk kentquirk commented Mar 19, 2023

This adds a new sampler type, an EMAThroughput sampler.

The EMASampleRate sampler requires specifying a desired sample rate. But more often, what someone would like to do is specify a given throughput. The WindowedThroughput sampler does that, but does so at the cost of maintaining a buffer of N previous sample rates, each of which is of equal weight.

The EMAThroughput sampler maintains a diminishing moving average of samples over time; older data is of less weight than current data. It also doesn't need to maintain any extra buffers.

Fixes #51.

@kentquirk kentquirk requested a review from a team as a code owner March 19, 2023 21:01
@kentquirk kentquirk added type: enhancement version: bump minor A PR that adds behavior, but is backwards-compatible. labels Mar 19, 2023
Base automatically changed from kent.common_key_calc to main March 20, 2023 14:43
emathroughput.go Outdated Show resolved Hide resolved
emathroughput.go Outdated Show resolved Hide resolved
emathroughput.go Show resolved Hide resolved
emathroughput.go Show resolved Hide resolved
var _ Sampler = (*EMAThroughput)(nil)

func (e *EMAThroughput) Start() error {
// apply defaults
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be nice to have these defaults defined as constants where possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're configuration default values; the only use of them is here. To me it seems counterproductive to invent a new set of names for the constants and move them to a block located elsewhere. I feel like that's why a Start() function exists.

emathroughput.go Show resolved Hide resolved
emathroughput.go Show resolved Hide resolved
emathroughput.go Show resolved Hide resolved
@kentquirk kentquirk merged commit 22b9a56 into main Mar 21, 2023
@kentquirk kentquirk deleted the kent.emathroughput branch March 21, 2023 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement version: bump minor A PR that adds behavior, but is backwards-compatible.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Add EMAThroughput sampler
2 participants