Shawyeok commented on code in PR #2086:
URL: https://github.com/apache/zookeeper/pull/2086#discussion_r1402063487
##########
zookeeper-metrics-providers/zookeeper-prometheus-metrics/src/main/java/org/apache/zookeeper/metrics/prometheus/PrometheusMetricsProvider.java:
##########
@@ -483,79 +487,70 @@ private void unregister() {
}
}
- private class PrometheusSummary implements Summary {
+ // VisibleForTesting
+ class PrometheusSummary implements Summary {
- private final io.prometheus.client.Summary inner;
+ // VisibleForTesting
+ SketchesSummary inner;
private final String name;
public PrometheusSummary(String name, MetricsContext.DetailLevel
level) {
this.name = name;
if (level == MetricsContext.DetailLevel.ADVANCED) {
- this.inner = io.prometheus.client.Summary
- .build(name, name)
- .quantile(0.5, 0.05) // Add 50th percentile (= median)
with 5% tolerated error
- .quantile(0.9, 0.01) // Add 90th percentile with 1%
tolerated error
- .quantile(0.99, 0.001) // Add 99th percentile with
0.1% tolerated error
+ this.inner = SketchesSummary.build(name, name)
+ .quantile(0.5) // Add 50th percentile (= median)
+ .quantile(0.9) // Add 90th percentile
+ .quantile(0.99) // Add 99th percentile
.register(collectorRegistry);
} else {
- this.inner = io.prometheus.client.Summary
- .build(name, name)
- .quantile(0.5, 0.05) // Add 50th percentile (= median)
with 5% tolerated error
+ this.inner = SketchesSummary.build(name, name)
+ .quantile(0.5) // Add 50th percentile (= median) with
5% tolerated error
.register(collectorRegistry);
}
}
@Override
public void add(long delta) {
- reportMetrics(() -> observe(delta));
- }
-
- private void observe(final long delta) {
try {
inner.observe(delta);
- } catch (final IllegalArgumentException err) {
+ } catch (IllegalArgumentException err) {
LOG.error("invalid delta {} for metric {}", delta, name, err);
}
}
}
- private class PrometheusLabelledSummary implements SummarySet {
+ // VisibleForTesting
+ class PrometheusLabelledSummary implements SummarySet {
- private final io.prometheus.client.Summary inner;
+ // VisibleForTesting
+ final SketchesSummary inner;
private final String name;
public PrometheusLabelledSummary(String name,
MetricsContext.DetailLevel level) {
this.name = name;
if (level == MetricsContext.DetailLevel.ADVANCED) {
- this.inner = io.prometheus.client.Summary
- .build(name, name)
+ this.inner = SketchesSummary.build(name, name)
.labelNames(LABELS)
- .quantile(0.5, 0.05) // Add 50th percentile (= median)
with 5% tolerated error
- .quantile(0.9, 0.01) // Add 90th percentile with 1%
tolerated error
- .quantile(0.99, 0.001) // Add 99th percentile with
0.1% tolerated error
+ .quantile(0.5) // Add 50th percentile (= median)
Review Comment:
About `1.7%`, it's depends on _k_ param in [DoublesSketch][1] which default
is 128.
[1]:
https://github.com/apache/datasketches-java/blob/007f35b59ec8b7f56fbfdf4145af2974be3c06f0/src/main/java/org/apache/datasketches/quantiles/DoublesSketch.java#L51
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
