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

Latency metrics resolution (microseconds) #366

Open
zezic opened this issue Sep 28, 2021 · 4 comments
Open

Latency metrics resolution (microseconds) #366

zezic opened this issue Sep 28, 2021 · 4 comments
Labels
enhancement New feature or request

Comments

@zezic
Copy link

zezic commented Sep 28, 2021

Hi! In our company we started using Goose to run various performance tests and one of them is the latency test. We need to control the metrics after changes in our code to detect any performance degradation.
But here is the problem – the latency of some endpoints of our service is about ~2000 microseconds. Currently we use custom loadtest function to report latency as microseconds instead of milliseconds, but according to this procedure Goose rounds numbers by 1000 when they reach values of 1000:

goose/src/metrics.rs

Lines 539 to 553 in dd81446

let rounded_time = if time_elapsed < 100 {
time
}
// Round to nearest 10 for 100-500ms times.
else if time_elapsed < 500 {
((time_elapsed as f64 / 10.0).round() * 10.0) as usize
}
// Round to nearest 100 for 500-1000ms times.
else if time_elapsed < 1000 {
((time_elapsed as f64 / 100.0).round() * 100.0) as usize
}
// Round to nearest 1000 for all larger times.
else {
((time_elapsed as f64 / 1000.0).round() * 1000.0) as usize
};

It makes it impossible to detect difference between, lets say, 2200 microseconds and 2500 microseconds.

Is it possible to add options to Goose to make it support measuring latency in microseconds with more precise (configurable?) rounding?

@jeremyandrews
Copy link
Member

I'm definitely open to improvements here. Early versions of Goose did not do any of this rounding, however this lead to massive data structures and horrible performance when displaying running and final metrics. I'm happy to review a PR that works to make this more flexible.

@jeremyandrews jeremyandrews added the enhancement New feature or request label Sep 30, 2021
@jeremyandrews
Copy link
Member

jeremyandrews commented Sep 30, 2021

Goose is currently tracking the response time of each request in milliseconds:
https://github.com/tag1consulting/goose/blob/main/src/metrics.rs#L305
https://github.com/tag1consulting/goose/blob/main/src/metrics.rs#L352

It also logs when the request was made with millisecond granularity:
https://github.com/tag1consulting/goose/blob/main/src/metrics.rs#L295
https://github.com/tag1consulting/goose/blob/main/src/goose.rs#L1606

Perhaps GooseRequestMetric and GooseRequestMetricTimingData could be turned into generics with a default implementation using milliseconds, allowing you to provide your own microsecond implementations. Some thought would need to go into how this would work.

@nathanielc
Copy link

I looking into using a compressed histogram format like https://docs.rs/tdigest/latest/tdigest/index.html to track large numbers of request accurately without using much space. Would something like this be accepted?

I could look into to making a generic aggregate type to allow for multiple implementations.

@jeremyandrews
Copy link
Member

Yes, if performance isn't negatively impacted this is definitely interesting. Making it generic to allow multiple implementations isn't necessary but would certainly be appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants