Skip to content

Commit 60c2553

Browse files
authored
Fix metric name filter in MultiCollector #891 (#893)
Signed-off-by: Fabian Stäber <[email protected]>
1 parent bc2244d commit 60c2553

File tree

3 files changed

+119
-7
lines changed

3 files changed

+119
-7
lines changed

prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,8 @@ public MetricSnapshots scrape(Predicate<String> includedNames, PrometheusScrapeR
9494
MetricSnapshots.Builder result = MetricSnapshots.builder();
9595
for (Collector collector : collectors) {
9696
String prometheusName = collector.getPrometheusName();
97+
// prometheusName == null means the name is unknown, and we have to scrape to learn the name.
98+
// prometheusName != null means we can skip the scrape if the name is excluded.
9799
if (prometheusName == null || includedNames.test(prometheusName)) {
98100
MetricSnapshot snapshot = scrapeRequest == null ? collector.collect(includedNames) : collector.collect(includedNames, scrapeRequest);
99101
if (snapshot != null) {
@@ -103,17 +105,18 @@ public MetricSnapshots scrape(Predicate<String> includedNames, PrometheusScrapeR
103105
}
104106
for (MultiCollector collector : multiCollectors) {
105107
List<String> prometheusNames = collector.getPrometheusNames();
106-
boolean excluded = true; // the multi-collector is excluded unless
107-
// at least one name matches
108+
// empty prometheusNames means the names are unknown, and we have to scrape to learn the names.
109+
// non-empty prometheusNames means we can exclude the collector if all names are excluded by the filter.
110+
boolean excluded = !prometheusNames.isEmpty();
108111
for (String prometheusName : prometheusNames) {
109112
if (includedNames.test(prometheusName)) {
110113
excluded = false;
111114
break;
112115
}
113116
}
114117
if (!excluded) {
115-
MetricSnapshots snaphots = scrapeRequest == null ? collector.collect(includedNames) : collector.collect(includedNames, scrapeRequest);
116-
for (MetricSnapshot snapshot : snaphots) {
118+
MetricSnapshots snapshots = scrapeRequest == null ? collector.collect(includedNames) : collector.collect(includedNames, scrapeRequest);
119+
for (MetricSnapshot snapshot : snapshots) {
117120
if (snapshot != null) {
118121
result.metricSnapshot(snapshot);
119122
}

prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/MetricNameFilterTest.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,21 @@
11
package io.prometheus.metrics.model.registry;
22

33
import io.prometheus.metrics.model.snapshots.CounterSnapshot;
4+
import io.prometheus.metrics.model.snapshots.CounterSnapshot.CounterDataPointSnapshot;
5+
import io.prometheus.metrics.model.snapshots.GaugeSnapshot;
6+
import io.prometheus.metrics.model.snapshots.GaugeSnapshot.GaugeDataPointSnapshot;
47
import io.prometheus.metrics.model.snapshots.Labels;
58
import io.prometheus.metrics.model.snapshots.MetricSnapshots;
69
import org.junit.Assert;
710
import org.junit.Before;
811
import org.junit.Test;
912

13+
import java.util.ArrayList;
14+
import java.util.Collections;
15+
import java.util.List;
16+
import java.util.concurrent.atomic.AtomicBoolean;
17+
import java.util.function.Predicate;
18+
1019
public class MetricNameFilterTest {
1120

1221
private PrometheusRegistry registry;
@@ -21,12 +30,12 @@ public void testCounter() {
2130
registry.register(() -> CounterSnapshot.builder()
2231
.name("counter1")
2332
.help("test counter 1")
24-
.dataPoint(CounterSnapshot.CounterDataPointSnapshot.builder()
33+
.dataPoint(CounterDataPointSnapshot.builder()
2534
.labels(Labels.of("path", "/hello"))
2635
.value(1.0)
2736
.build()
2837
)
29-
.dataPoint(CounterSnapshot.CounterDataPointSnapshot.builder()
38+
.dataPoint(CounterDataPointSnapshot.builder()
3039
.labels(Labels.of("path", "/goodbye"))
3140
.value(2.0)
3241
.build()
@@ -36,7 +45,7 @@ public void testCounter() {
3645
registry.register(() -> CounterSnapshot.builder()
3746
.name("counter2")
3847
.help("test counter 2")
39-
.dataPoint(CounterSnapshot.CounterDataPointSnapshot.builder()
48+
.dataPoint(CounterDataPointSnapshot.builder()
4049
.value(1.0)
4150
.build()
4251
)
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package io.prometheus.metrics.model.registry;
2+
3+
import io.prometheus.metrics.model.snapshots.CounterSnapshot;
4+
import io.prometheus.metrics.model.snapshots.CounterSnapshot.CounterDataPointSnapshot;
5+
import io.prometheus.metrics.model.snapshots.GaugeSnapshot;
6+
import io.prometheus.metrics.model.snapshots.GaugeSnapshot.GaugeDataPointSnapshot;
7+
import io.prometheus.metrics.model.snapshots.MetricSnapshots;
8+
import org.junit.Assert;
9+
import org.junit.Before;
10+
import org.junit.Test;
11+
12+
import java.util.ArrayList;
13+
import java.util.Arrays;
14+
import java.util.Collections;
15+
import java.util.List;
16+
import java.util.function.Predicate;
17+
18+
public class MultiCollectorNameFilterTest {
19+
20+
private PrometheusRegistry registry;
21+
private boolean[] collectCalled = {false};
22+
private Predicate<String> includedNames = null;
23+
List<String> prometheusNames = new ArrayList<>();
24+
25+
@Before
26+
public void setUp() {
27+
registry = new PrometheusRegistry();
28+
collectCalled[0] = false;
29+
includedNames = null;
30+
prometheusNames = Collections.emptyList();
31+
32+
registry.register(new MultiCollector() {
33+
@Override
34+
public MetricSnapshots collect() {
35+
collectCalled[0] = true;
36+
return MetricSnapshots.builder()
37+
.metricSnapshot(CounterSnapshot.builder()
38+
.name("counter_1")
39+
.dataPoint(CounterDataPointSnapshot.builder().value(1.0).build())
40+
.build()
41+
)
42+
.metricSnapshot(GaugeSnapshot.builder()
43+
.name("gauge_2")
44+
.dataPoint(GaugeDataPointSnapshot.builder().value(1.0).build())
45+
.build()
46+
)
47+
.build();
48+
}
49+
50+
@Override
51+
public List<String> getPrometheusNames() {
52+
return prometheusNames;
53+
}
54+
});
55+
}
56+
57+
@Test
58+
public void testPartialFilter() {
59+
60+
includedNames = name -> name.equals("counter_1");
61+
62+
MetricSnapshots snapshots = registry.scrape(includedNames);
63+
Assert.assertTrue(collectCalled[0]);
64+
Assert.assertEquals(1, snapshots.size());
65+
Assert.assertEquals("counter_1", snapshots.get(0).getMetadata().getName());
66+
}
67+
68+
@Test
69+
public void testPartialFilterWithPrometheusNames() {
70+
71+
includedNames = name -> name.equals("counter_1");
72+
prometheusNames = Arrays.asList("counter_1", "gauge_2");
73+
74+
MetricSnapshots snapshots = registry.scrape(includedNames);
75+
Assert.assertTrue(collectCalled[0]);
76+
Assert.assertEquals(1, snapshots.size());
77+
Assert.assertEquals("counter_1", snapshots.get(0).getMetadata().getName());
78+
}
79+
80+
@Test
81+
public void testCompleteFilter_CollectCalled() {
82+
83+
includedNames = name -> !name.equals("counter_1") && !name.equals("gauge_2");
84+
85+
MetricSnapshots snapshots = registry.scrape(includedNames);
86+
Assert.assertTrue(collectCalled[0]);
87+
Assert.assertEquals(0, snapshots.size());
88+
}
89+
90+
@Test
91+
public void testCompleteFilter_CollectNotCalled() {
92+
93+
includedNames = name -> !name.equals("counter_1") && !name.equals("gauge_2");
94+
prometheusNames = Arrays.asList("counter_1", "gauge_2");
95+
96+
MetricSnapshots snapshots = registry.scrape(includedNames);
97+
Assert.assertFalse(collectCalled[0]);
98+
Assert.assertEquals(0, snapshots.size());
99+
}
100+
}

0 commit comments

Comments
 (0)