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

min statistic for distribution summaries and timers #457

Open
randysecrist opened this issue Feb 21, 2018 · 11 comments
Open

min statistic for distribution summaries and timers #457

randysecrist opened this issue Feb 21, 2018 · 11 comments
Labels
enhancement A general enhancement

Comments

@randysecrist
Copy link

Quick question ...

Why doesn't https://github.com/micrometer-metrics/micrometer/blob/master/micrometer-core/src/main/java/io/micrometer/core/instrument/distribution/HistogramSnapshot.java have a min function?

@jkschneider
Copy link
Contributor

jkschneider commented Feb 21, 2018

Just wanted to be clear that there was a use for it before introducing. Seems like alerting on a min is a rare thing, but there is probably a real world case somewhere. What are you thinking of doing?

@jkschneider
Copy link
Contributor

jkschneider commented Feb 21, 2018

BTW, workaround is a P0 percentile:

@Test
void minimum() {
    DistributionSummary summary = DistributionSummary.builder("my.summary")
            .publishPercentiles(0)
            .register(new SimpleMeterRegistry());

    // dummy data
    for (int i = 1; i < 10; i++) {
        summary.record(i);
    }

    assertThat(summary.percentile(0)).isEqualTo(1);
}

Micrometer's client-side percentiles are designed to decay in the same way max does. P0 can be interpolated, but it's something.

@jkschneider jkschneider added the question A user question, probably better suited for StackOverflow label Feb 21, 2018
@randysecrist
Copy link
Author

Historical monitoring of snapshots (but not necessarily alert) a distribution summary for say ... minimum values for something like say ... request size. All the other relevant stats are there; just seems to be missing this one.

@jkschneider
Copy link
Contributor

Reason I'm seeking concrete use cases is we should determine whether this is an always-on thing or an option to select. Leaning towards the latter unless there are an overwhelming number of use cases.

Request size has an interesting characteristic that it has a natural lower bound (request header size). Interestingly, it means request size essentially is never normally distributed and standard deviation is useless, but I digress...

Would you alert on min size?

@randysecrist
Copy link
Author

My thought was to track request size only for something like POST or PUT; since as you say the natural lower bound for a GET or HEAD request would typically be just the header + query string.

In the case of using HTTP for incoming writes; I would only alert if the use case was below the expected minimum; if that is even knowable for a given use case ... and that really depends on the application imo.

@jkschneider
Copy link
Contributor

Sounds like default off with an option to turn it on would be OK then?

@randysecrist
Copy link
Author

Ya, I think that would work. Thanks!

@jkschneider jkschneider changed the title min function on HistogramsSnapshot min statistic for distribution summaries and timers. Feb 26, 2018
@jkschneider jkschneider changed the title min statistic for distribution summaries and timers. min statistic for distribution summaries and timers Feb 26, 2018
@jkschneider
Copy link
Contributor

This would be useful for the StackDriver Range aggregate as well.

@jkschneider
Copy link
Contributor

When implemented, fix min statistic on Azure App Insights implementation as well. #441.

@jkschneider jkschneider modified the milestones: 1.1.0-rc.1, 1.1.0-rc.2 Oct 12, 2018
@jkschneider jkschneider modified the milestones: 1.1.0-rc.2, 1.1.0, 1.x Oct 21, 2018
@martel
Copy link

martel commented Mar 27, 2019

This would also be useful while reporting to Cloudwatch where you could use StatisticSets:
https://docs.aws.amazon.com/AmazonCloudWatch/latest/APIReference/API_StatisticSet.html

Instead of emitting each value separately in the reporter:

Stream.Builder<MetricDatum> metrics = Stream.builder();
metrics.add(metricDatum(summary.getId(), "sum", summary.totalAmount()));
long count = summary.count();
metrics.add(metricDatum(summary.getId(), "count", StandardUnit.Count, count));
if (count > 0) {
metrics.add(metricDatum(summary.getId(), "avg", summary.mean()));
metrics.add(metricDatum(summary.getId(), "max", summary.max()));
}

@thrxde
Copy link

thrxde commented Apr 28, 2020

to enable the adjustments for Cloudwatch. i added min value to micrometer. on branch 1.3.x
pullrequest

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

No branches or pull requests

5 participants