Allow custom SLO configuration when using a DistributionSummary.

[nielsm5]

Allow custom SLO configuration when using a DistributionSummary.

[jonatan-ivanov]

Could you please send us a test (reproducer) that shows the issue?

DistributionSummary ds = DistributionSummary.builder("ds")
    .serviceLevelObjectives(100, 200, 300)
    .register(registry);

ds.record(10);

It's strange that Prometheus disables SLO but does NOT set the percentilesHistogram to false by default.

You can imagine this as adding extra buckets to a histogram.

[github-actions]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[github-actions]

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open.

[nielsm5]

Hi,

When adding a new DistributionSummary the way you described, the PrometheusDistributionSummary 'messes' with the distributionStatisticConfig in particularly the serviceLevelObjectives(). It calls the method but does not provide any arguments.

This causes the merge method (DistributionStatisticConfig.merge(DistributionStatisticConfig)) to favor it's own (Prometheus SLO, over the parent (provided when creating the summary).

The problem; if you define your own SLO, it will ignore the provided value, and use SLO without any value creating it's own random range from 0 to inf.

[nielsm5]

jonatan-ivanov
@jonatan-ivanov

Just so you understand the problem, it's possible to add extra buckets. I understand that the intent is there, and it works for other registries, just not Prometheus. .serviceLevelObjectives(100, 200, 300) will be overwritten, and the user defined values will NOT be used.

[jonatan-ivanov]

Could you please send us a test (reproducer) that shows the issue?
As far as I remember what you are trying to do should work.