Skip to content

Commit dd707a7

Browse files
devromikdevromik
authored
Merge pull request #34 from ringcentral/sort_buckets_and_quantiles_for_prometheus
#31: sort buckets and quantiles in increasing numerical order of their label values
2 parents 59c34ab + 0887e3c commit dd707a7

File tree

10 files changed

+423
-24
lines changed

10 files changed

+423
-24
lines changed

‎metrics-facade-base/src/main/java/com/ringcentral/platform/metrics/histogram/Histogram.java

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,14 @@
33
import com.ringcentral.platform.metrics.Meter;
44
import com.ringcentral.platform.metrics.dimensions.MetricDimensionValues;
55
import com.ringcentral.platform.metrics.measurables.MeasurableType;
6-
import com.ringcentral.platform.metrics.scale.*;
6+
import com.ringcentral.platform.metrics.scale.Scale;
7+
import com.ringcentral.platform.metrics.scale.ScaleBuilder;
78

89
import java.math.BigDecimal;
9-
import java.util.*;
10+
import java.util.EnumMap;
11+
import java.util.Locale;
12+
import java.util.Map;
13+
import java.util.Set;
1014
import java.util.concurrent.TimeUnit;
1115

1216
import static com.ringcentral.platform.metrics.dimensions.MetricDimensionValues.NO_DIMENSION_VALUES;
@@ -131,7 +135,7 @@ public int hashCode() {
131135

132136
StandardDeviation STANDARD_DEVIATION = new StandardDeviation();
133137

134-
class Percentile implements HistogramMeasurable {
138+
class Percentile implements HistogramMeasurable, Comparable<Percentile> {
135139

136140
private static final BigDecimal BIG_DECIMAL_100 = BigDecimal.valueOf(100.0);
137141

@@ -202,6 +206,11 @@ public boolean equals(Object other) {
202206
public int hashCode() {
203207
return hashCode;
204208
}
209+
210+
@Override
211+
public int compareTo(Percentile right) {
212+
return Double.compare(quantile, right.quantile);
213+
}
205214
}
206215

207216
Percentile PERCENTILE_1 = Percentile.of(0.01);
@@ -227,10 +236,11 @@ public int hashCode() {
227236
Percentile PERCENTILE_99 = Percentile.of(0.99);
228237
Percentile PERCENTILE_999 = Percentile.of(0.999);
229238

230-
class Bucket implements HistogramMeasurable {
239+
class Bucket implements HistogramMeasurable, Comparable<Bucket> {
231240

232241
private final double upperBoundInUnits;
233242
private final TimeUnit upperBoundUnit;
243+
private final double upperBound;
234244
private final long upperBoundAsLong;
235245
private final boolean inf;
236246
private final boolean negativeInf;
@@ -249,22 +259,22 @@ public Bucket(double upperBoundInUnits, TimeUnit upperBoundUnit) {
249259
this.upperBoundInUnits = upperBoundInUnits;
250260
this.upperBoundUnit = upperBoundUnit;
251261

252-
double upperBound =
262+
this.upperBound =
253263
Double.isInfinite(upperBoundInUnits) || upperBoundUnit == null || upperBoundUnit == NANOSECONDS ?
254264
upperBoundInUnits :
255265
upperBoundUnit.toNanos(1L) * upperBoundInUnits;
256266

257-
if (Double.isNaN(upperBound)) {
267+
if (Double.isNaN(this.upperBound)) {
258268
throw new IllegalArgumentException(
259269
upperBoundInUnits
260270
+ (upperBoundUnit != null ? " " + upperBoundUnit.name().toLowerCase(Locale.ENGLISH) : "")
261271
+ " in nanos results in an overflow");
262272
}
263273

264-
this.upperBoundAsLong = round(upperBound);
274+
this.upperBoundAsLong = round(this.upperBound);
265275
this.inf = (this.upperBoundAsLong == Long.MAX_VALUE);
266276
this.negativeInf = (this.upperBoundAsLong == Long.MIN_VALUE);
267-
this.upperBoundAsString = upperBoundAsString(upperBound);
277+
this.upperBoundAsString = upperBoundAsString(this.upperBound);
268278
TimeUnit resolvedUpperBoundUnit = upperBoundUnit != null ? upperBoundUnit : NANOSECONDS;
269279

270280
if (Double.isInfinite(upperBoundInUnits)) {
@@ -315,7 +325,7 @@ public Bucket(double upperBoundInUnits, TimeUnit upperBoundUnit) {
315325
this.upperBoundAsStringWithUnit = unitToUpperBoundAsStringWithUnit.get(resolvedUpperBoundUnit);
316326
}
317327

318-
this.upperBoundAsNumberString = upperBoundAsNumberString(upperBound);
328+
this.upperBoundAsNumberString = upperBoundAsNumberString(this.upperBound);
319329

320330
this.upperBoundSecAsNumberString = withoutTrailingZeros(String.valueOf(
321331
Double.isInfinite(upperBoundInUnits) || upperBoundUnit == SECONDS ?
@@ -434,6 +444,27 @@ public int hashCode() {
434444
return hashCode;
435445
}
436446

447+
@Override
448+
public int compareTo(Bucket right) {
449+
if (isInf()) {
450+
return right.isInf() ? 0 : 1;
451+
}
452+
453+
if (right.isInf()) {
454+
return -1;
455+
}
456+
457+
if (isNegativeInf()) {
458+
return right.isNegativeInf() ? 0 : -1;
459+
}
460+
461+
if (right.isNegativeInf()) {
462+
return 1;
463+
}
464+
465+
return Double.compare(upperBound, right.upperBound);
466+
}
467+
437468
@Override
438469
public String toString() {
439470
return "Bucket{" +

‎metrics-facade-base/src/main/java/com/ringcentral/platform/metrics/samples/AbstractInstanceSample.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
public class AbstractInstanceSample<S extends Sample> implements InstanceSample<S> {
88

9-
private final List<S> samples;
9+
protected final List<S> samples;
1010

1111
public AbstractInstanceSample() {
1212
this(new ArrayList<>());

‎metrics-facade-base/src/test/java/com/ringcentral/platform/metrics/histogram/HistogramTest.java

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,39 @@
11
package com.ringcentral.platform.metrics.histogram;
22

3-
import com.ringcentral.platform.metrics.histogram.Histogram.Bucket;
43
import org.junit.Test;
54

5+
import static com.ringcentral.platform.metrics.histogram.Histogram.*;
66
import static java.util.concurrent.TimeUnit.MILLISECONDS;
77
import static org.hamcrest.CoreMatchers.is;
88
import static org.hamcrest.MatcherAssert.assertThat;
99

10+
@SuppressWarnings("EqualsWithItself")
1011
public class HistogramTest {
1112

13+
@Test
14+
public void sortingBuckets() {
15+
assertThat(Bucket.of(1).compareTo(Bucket.of(1)), is(0));
16+
assertThat(Bucket.of(1).compareTo(Bucket.of(2)), is(-1));
17+
assertThat(Bucket.of(2).compareTo(Bucket.of(1)), is(1));
18+
assertThat(Bucket.of(1).compareTo(INF_BUCKET), is(-1));
19+
assertThat(INF_BUCKET.compareTo(Bucket.of(2)), is(1));
20+
21+
assertThat(Bucket.of(1.5).compareTo(Bucket.of(1.5)), is(0));
22+
assertThat(Bucket.of(1.5).compareTo(Bucket.of(1.7)), is(-1));
23+
assertThat(Bucket.of(1.7).compareTo(Bucket.of(1.5)), is(1));
24+
assertThat(Bucket.of(1.5).compareTo(INF_BUCKET), is(-1));
25+
assertThat(INF_BUCKET.compareTo(Bucket.of(1.7)), is(1));
26+
27+
assertThat(INF_BUCKET.compareTo(INF_BUCKET), is(0));
28+
}
29+
30+
@Test
31+
public void sortingPercentiles() {
32+
assertThat(PERCENTILE_1.compareTo(PERCENTILE_1), is(0));
33+
assertThat(PERCENTILE_1.compareTo(PERCENTILE_5), is(-1));
34+
assertThat(PERCENTILE_5.compareTo(PERCENTILE_1), is(1));
35+
}
36+
1237
@Test
1338
public void bucketStringRepresentations() {
1439
Bucket bucket = Bucket.of(25.5);

‎metrics-facade-prometheus/src/main/java/com/ringcentral/platform/metrics/samples/prometheus/PrometheusInstanceSample.java

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
package com.ringcentral.platform.metrics.samples.prometheus;
22

3+
import com.ringcentral.platform.metrics.counter.Counter.Count;
4+
import com.ringcentral.platform.metrics.histogram.Histogram.Bucket;
5+
import com.ringcentral.platform.metrics.histogram.Histogram.Percentile;
6+
import com.ringcentral.platform.metrics.histogram.Histogram.TotalSum;
7+
import com.ringcentral.platform.metrics.measurables.Measurable;
38
import com.ringcentral.platform.metrics.names.MetricName;
49
import com.ringcentral.platform.metrics.samples.AbstractInstanceSample;
510
import io.prometheus.client.Collector;
611

7-
import java.util.*;
12+
import java.util.ArrayList;
13+
import java.util.List;
814

915
import static java.util.Collections.emptyList;
1016
import static java.util.Objects.requireNonNull;
@@ -82,6 +88,76 @@ public void add(PrometheusSample sample) {
8288
}
8389
}
8490

91+
@Override
92+
public List<PrometheusSample> samples() {
93+
if (type != Collector.Type.HISTOGRAM && type != Collector.Type.SUMMARY) {
94+
return super.samples();
95+
}
96+
97+
if (samples.isEmpty()) {
98+
return samples;
99+
}
100+
101+
samples.sort((left, right) -> {
102+
if (!left.hasMeasurable()) {
103+
return right.hasMeasurable() ? 1 : 0;
104+
}
105+
106+
if (!right.hasMeasurable()) {
107+
return -1;
108+
}
109+
110+
Measurable leftM = left.measurable();
111+
Measurable rightM = right.measurable();
112+
113+
// Bucket
114+
if (leftM instanceof Bucket) {
115+
return
116+
rightM instanceof Bucket ?
117+
((Bucket)leftM).compareTo(((Bucket)rightM)) :
118+
-1;
119+
}
120+
121+
if (rightM instanceof Bucket) {
122+
return 1;
123+
}
124+
125+
// Percentile
126+
if (leftM instanceof Percentile) {
127+
return
128+
rightM instanceof Percentile ?
129+
((Percentile)leftM).compareTo(((Percentile)rightM)) :
130+
-1;
131+
}
132+
133+
if (rightM instanceof Percentile) {
134+
return 1;
135+
}
136+
137+
// Count
138+
if (leftM instanceof Count) {
139+
return rightM instanceof Count ? 0 : -1;
140+
}
141+
142+
if (rightM instanceof Count) {
143+
return 1;
144+
}
145+
146+
// TotalSum
147+
if (leftM instanceof TotalSum) {
148+
return rightM instanceof TotalSum ? 0 : -1;
149+
}
150+
151+
if (rightM instanceof TotalSum) {
152+
return 1;
153+
}
154+
155+
return 0;
156+
});
157+
158+
return samples;
159+
}
160+
85161
@Override
86162
public boolean isEmpty() {
87163
return super.isEmpty() && !hasChildren();

‎metrics-facade-prometheus/src/main/java/com/ringcentral/platform/metrics/samples/prometheus/PrometheusSample.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.ringcentral.platform.metrics.samples.prometheus;
22

3+
import com.ringcentral.platform.metrics.measurables.Measurable;
34
import com.ringcentral.platform.metrics.names.MetricName;
45
import com.ringcentral.platform.metrics.samples.Sample;
56
import io.prometheus.client.Collector;
@@ -11,6 +12,7 @@
1112

1213
public class PrometheusSample implements Sample {
1314

15+
private final Measurable measurable;
1416
private final MetricName childInstanceSampleNameSuffix;
1517
private final Collector.Type childInstanceSampleType;
1618
private final MetricName name;
@@ -20,6 +22,7 @@ public class PrometheusSample implements Sample {
2022
private final double value;
2123

2224
public PrometheusSample(
25+
Measurable measurable,
2326
MetricName childInstanceSampleNameSuffix,
2427
Collector.Type childInstanceSampleType,
2528
MetricName name,
@@ -28,6 +31,8 @@ public PrometheusSample(
2831
List<String> labelValues,
2932
double value) {
3033

34+
this.measurable = measurable;
35+
3136
if (childInstanceSampleNameSuffix != null) {
3237
checkArgument(!childInstanceSampleNameSuffix.isEmpty(), "childInstanceSampleNameSuffix is empty");
3338
checkArgument(childInstanceSampleType != null, "childInstanceSampleType is null");
@@ -43,6 +48,14 @@ public PrometheusSample(
4348
this.value = value;
4449
}
4550

51+
public boolean hasMeasurable() {
52+
return measurable != null;
53+
}
54+
55+
public Measurable measurable() {
56+
return measurable;
57+
}
58+
4659
public boolean belongsToChildInstanceSample() {
4760
return childInstanceSampleNameSuffix != null;
4861
}
@@ -57,6 +70,7 @@ public Collector.Type childInstanceSampleType() {
5770

5871
public PrometheusSample notBelongingToChildInstanceSample() {
5972
return new PrometheusSample(
73+
measurable,
6074
null,
6175
null,
6276
name,

‎metrics-facade-prometheus/src/main/java/com/ringcentral/platform/metrics/samples/prometheus/PrometheusSampleMaker.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ public PrometheusSample makeSample(PrometheusSampleSpec spec, PrometheusInstance
9191
}
9292

9393
return new PrometheusSample(
94+
m,
9495
childInstanceSampleNameSuffix,
9596
childInstanceSampleType,
9697
null,

‎metrics-facade-prometheus/src/main/java/com/ringcentral/platform/metrics/samples/prometheus/collectorRegistry/SimpleCollectorRegistryPrometheusInstanceSamplesProvider.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ private void processCollectorRegistry(CollectorRegistry collectorRegistry, Set<P
135135
MetricName sampleName = nameBuilder.apply(fsSample.name);
136136

137137
PrometheusSample sample = new PrometheusSample(
138+
null,
138139
null,
139140
null,
140141
sampleName,

0 commit comments

Comments
 (0)