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

Add min and max values to the histogram data points #266

Closed
rakyll opened this issue Feb 12, 2021 · 9 comments · Fixed by #279
Closed

Add min and max values to the histogram data points #266

rakyll opened this issue Feb 12, 2021 · 9 comments · Fixed by #279

Comments

@rakyll
Copy link

rakyll commented Feb 12, 2021

Today, OpenTelemetry doesn't report min and max values collected for a histogram data point and is breaking the capability to query absolute min and max values from a time series. We propose to add min and max values to the histogram data types. When users set boundaries explicitly, not reporting precise min and max doesn't allow them to see whether their bucket boundaries are well-chosen and precisely set.

Here are some use cases where being able to query absolute min and max values from a series is critical:

  • In latency analysis, the bucket boundaries often starts at 0 even though the smallest latency will never be 0. Being able to see the absolute minimum allows users to evaluate the minimum value. This sometimes helps users to find bugs! For example, if your min never went below 40ms for a case where theoretical latency could be 20 ms, it may help you to identify something went wrong.
  • Being able to see the max allow users to see how far it is from the 95th/99th percentile. Seldom spikes can add noise to the highest buckets, reporting maximum allows users to adjust bucket boundaries if needed.
  • Outliers are easier to detect with max. For example 1 TB of nearly full disk usage on a few machines will be invisible in the aggregate buckets but are important to identity.
  • When doing trend analysis over time, min and max (when collected) are easy to query and allows you to precisely see the boundaries of the population.
@rakyll
Copy link
Author

rakyll commented Feb 12, 2021

cc @jmacd @bogdandrutu

@bogdandrutu
Copy link
Member

@rakyll I have some questions about the behavior of min/max:

  • What is their temporality? Are they respecting the Histogram temporality? What is the advantage to know the min/max when reporting Cumulative values?
  • All the other values in a Histogram can be subtracted and compute deltas, which is what people usually look at. When you say 99% of a timeseries you probably mean "99%tile of the last X seconds", looking at "99%tile from the start of the process" is usually useless.

So I think usually what you probably need is to have always "delta" for min/max in order for the backend to be able to calculate rollups. So I think this is a bit harder to support than it may sound, and also be useful.

@rakyll
Copy link
Author

rakyll commented Feb 12, 2021

When we are reporting the histogram data points, min/max temporality should respect the histogram temporarily. I don't know any backends that allows to report min/max with different temporality, e.g. delta min/max for a cumulative histogram.

For a cumulative histograms, this is a bit less useful because min/max will mostly stay the same but will still help the user to query the global min/max for a time series with full precision.

For deltas, it enables most of the other use cases I mentioned above and the "99%tile of the last X seconds" you mentioned.

@bogdandrutu
Copy link
Member

bogdandrutu commented Feb 16, 2021

For a cumulative histograms, this is a bit less useful because min/max will mostly stay the same but will still help the user to query the global min/max for a time series with full precision.

I think this is not that useful but because I don't find this useful it does not mean it is not, I would like to hear others opinion on this.

For deltas, it enables most of the other use cases I mentioned above and the "99%tile of the last X seconds" you mentioned.

One thing that maybe I did not clearly specify here is that majority of the backends (the once that I know all of them do this) for histograms, independent of the temporality that they are received, store deltas internally. By adding min/max to the cumulative histogram we gain 0 value, because data will be dropped since they cannot be transformed into delta as all the other fields in the Histogram.

So the 1M$ question is:
Do we need min/max in the histogram (together with all the other fields that have the nice properties that are easy to be rolled up) OR we need a separate aggregation that contains these fields and maybe few others?

@rakyll
Copy link
Author

rakyll commented Feb 17, 2021

I like the idea of having separating min/max into different aggregations. Users can report min/max more frequently if it is decoupled from the histogram. The cons are:

  • We need to educate users that they need to set separate aggregations and use different time series for them.
  • If the min/max was a part of the histogram data model, it's easier to export to directly to the backends with min/max support. Otherwise, users will try to do the magic of post processing and merging two timeseries into one. This is something we don't want to implicitly encourage.

@oertl
Copy link

oertl commented Feb 17, 2021

Pros for including min/max:

  • When estimating percentiles, you expect them to be always in the range [min, max]. If min/max are not available it is not possible to return consistent estimates. When charting the maximum and the 99th percentile, for example, it would look nasty if the 99th percentile exceeds the maximum. Of course, this can also be fixed when estimating the 99th percentile. However, this would require access to two different data sources (the histogram and the min/max values).
  • If the 99th percentile falls into the very last bucket (which covers the range up to infinity), it is not possible to give any meaningful estimate for the 99th percentile. If you know the maximum, you know the effective upper bound of the last bucket and you can still return some meaningful estimate for the 99th percentile. Hence, min/max values allow a more graceful estimation behavior if the first or the last bucket are nonempty.
  • As already mentioned by @rakyll, it helps to see unexpected values. For example, assume you have implemented a timeout of 100s. Therefore all response times are supposed to not exceed 100s. So you might choose a discretization where the last boundary is 99s. Every value larger than 99s will be mapped to the last bucket no matter if it is 100s or an unexpected value like 1000s. The max value would indicate in this case that something is going wrong.
  • Furthermore, as also mentioned by @rakyll, min/max help to verify if the boundaries have been a good choice for the observed data. If the maximum is far beyond the last boundary it might be necessary to readjust the boundaries.

Cons:

  • Small space overhead for storing minimum and maximum explicitly. (This might not be relevant, if min/max are required anyway.)
  • Slightly more expensive update costs as every recorded value has to be compared against min/max. (This might not be relevant, if min/max are required anyway.)
  • As mentioned by @bogdandrutu, it is not possible to substract one histogram from another. This might be helpful when maintaining a rolling window in memory. For example, if you have a histogram for every hour and you would like to have a rolling window over the last 24 hours, it is sufficient to add the most recent one and to subtract the last one. If subtraction is not possible (because min/max are included) you have to aggregate 24 histograms every hour. (However, with the help of a tree the update could be reduced to logarithmic time.) Please also note, that if the frequency counts are floating point values instead of integer values, subtraction is a dangerous operation anyway, because errors might accumulate over time and the frequency counts might drift away.

In my opinion, the pros outweigh the cons. Therefore, the DynaHist histogram implementation includes min/max.

@jsuereth
Copy link
Contributor

I'm in favor of encouraging "bundling" min/max with a histogram to make sure they are reported and we can do interesting things with the data. Misshapped histograms buckets are pretty annoying in practice, and this could help deal with that issue.

I think this change is overall good assuming two things:

  1. min/max are not required on histogram buckets. backends can ignore them, code is careful not to assume they exist.
  2. Min/Max can be pulled off as separate timeseries (as bogdan suggests) "by convention". I.e. if any backend wants to leverage min/max but can't directly there's some easy-for-the-user semantic convention for how min + max get turned into timeseries.

@jmacd jmacd transferred this issue from open-telemetry/opentelemetry-specification Feb 24, 2021
@jmacd
Copy link
Contributor

jmacd commented Mar 4, 2021

Following the discussion on Tuesday, we still have technical details to resolve before we can add min/max to the protocol. The discussion has circled around these points:

  1. When temporality is cumulative, the implied meaning for min/max is the min and max over the entire series, which is not what's actually desired
  2. When temporality is delta, the implied meaning is 💯, we can precisely encode the min and max over short windows of time.

Looking toward Prometheus, there's at least one more concern. Data pulled from a /metrics endpoint has no (stateless) way to encode a delta, but it is equally undesirable to report the lifetime min and max of a series. Therefore, Prometheus Summary data types expose the min and max value (i.e., φ = 0 and φ = 1) over a fixed window of time.

I believe the best outcome for a Prometheus user will be to emulate the behavior of the Summary data type's min and max fields for OTLP Histograms. This statement is practically the only option when OTLP's aggregation temporality is cumulative.

For OTLP delta aggregation temporality, we have two options.

The first option is for delta to behave exactly as cumulative temporality would. This allows change-of-temporality to be considered a true no-op for histograms.

The second option is for delta to output min and max precisely, meaning to report actual min/max values over short time windows.

I have a slight preference here, but I think both behaviors should be considered valid.

@jmacd
Copy link
Contributor

jmacd commented Mar 15, 2021

Please review #279.

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

Successfully merging a pull request may close this issue.

5 participants