Skip to content

Commit 0f00f8b

Browse files
jaydelucajaydeluca
committed
move label validation to scrape time
Signed-off-by: Jay DeLuca <[email protected]>
1 parent 4a9f01e commit 0f00f8b

File tree

4 files changed

+78
-104
lines changed

4 files changed

+78
-104
lines changed

‎prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -207,29 +207,29 @@ public String getPrometheusName() {
207207
}
208208
});
209209

210-
// Registration should throw exception due to duplicate time series (same name + same labels)
211-
assertThatThrownBy(
212-
() ->
213-
registry.register(
214-
new Collector() {
215-
@Override
216-
public MetricSnapshot collect() {
217-
return CounterSnapshot.builder()
218-
.name("api_responses")
219-
.help("API responses")
220-
.dataPoint(
221-
CounterSnapshot.CounterDataPointSnapshot.builder()
222-
.labels(Labels.of("uri", "/hello", "outcome", "SUCCESS"))
223-
.value(50)
224-
.build())
225-
.build();
226-
}
227-
228-
@Override
229-
public String getPrometheusName() {
230-
return "api_responses_total";
231-
}
232-
}))
210+
registry.register(
211+
new Collector() {
212+
@Override
213+
public MetricSnapshot collect() {
214+
return CounterSnapshot.builder()
215+
.name("api_responses")
216+
.help("API responses")
217+
.dataPoint(
218+
CounterSnapshot.CounterDataPointSnapshot.builder()
219+
.labels(Labels.of("uri", "/hello", "outcome", "SUCCESS"))
220+
.value(50)
221+
.build())
222+
.build();
223+
}
224+
225+
@Override
226+
public String getPrometheusName() {
227+
return "api_responses_total";
228+
}
229+
});
230+
231+
// Scrape should throw exception due to duplicate time series (same name + same labels)
232+
assertThatThrownBy(() -> registry.scrape())
233233
.isInstanceOf(IllegalArgumentException.class)
234234
.hasMessageContaining("Duplicate labels detected")
235235
.hasMessageContaining("api_responses");

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

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package io.prometheus.metrics.model.registry;
22

3-
import io.prometheus.metrics.model.snapshots.DataPointSnapshot;
43
import io.prometheus.metrics.model.snapshots.MetricSnapshot;
54
import io.prometheus.metrics.model.snapshots.MetricSnapshots;
65
import java.util.List;
@@ -31,7 +30,7 @@ public void register(MultiCollector collector) {
3130

3231
/**
3332
* Validates that the new collector's type is consistent with any existing collectors that have
34-
* the same Prometheus name, and that there are no duplicate label sets.
33+
* the same Prometheus name.
3534
*/
3635
private void validateTypeConsistency(Collector newCollector) {
3736
String newName = newCollector.getPrometheusName();
@@ -56,19 +55,6 @@ private void validateTypeConsistency(Collector newCollector) {
5655
+ ". All collectors with the same Prometheus name must have the same type.");
5756
}
5857
}
59-
60-
// Validate no duplicate labels by collecting and comparing snapshots
61-
MetricSnapshot newSnapshot = newCollector.collect();
62-
if (newSnapshot != null) {
63-
for (Collector existingCollector : collectors) {
64-
if (newName.equals(existingCollector.getPrometheusName())) {
65-
MetricSnapshot existingSnapshot = existingCollector.collect();
66-
if (existingSnapshot != null) {
67-
validateNoDuplicateLabels(newSnapshot, existingSnapshot);
68-
}
69-
}
70-
}
71-
}
7258
}
7359

7460
/**
@@ -107,25 +93,6 @@ private void validateTypeConsistency(MultiCollector newCollector) {
10793
}
10894
}
10995

110-
/**
111-
* Validates that two snapshots with the same Prometheus name don't have overlapping label sets.
112-
*/
113-
private void validateNoDuplicateLabels(MetricSnapshot snapshot1, MetricSnapshot snapshot2) {
114-
String metricName = snapshot1.getMetadata().getName();
115-
116-
for (DataPointSnapshot dp1 : snapshot1.getDataPoints()) {
117-
for (DataPointSnapshot dp2 : snapshot2.getDataPoints()) {
118-
if (dp1.getLabels().equals(dp2.getLabels())) {
119-
throw new IllegalArgumentException(
120-
"Duplicate labels detected for metric '"
121-
+ metricName
122-
+ "' with labels "
123-
+ dp1.getLabels()
124-
+ ". Each time series (metric name + label set) must be unique.");
125-
}
126-
}
127-
}
128-
}
12996

13097
/**
13198
* Caches the metric identifier for lookup during future registrations.

‎prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,11 @@ public MetricSnapshots(MetricSnapshot... snapshots) {
3131
*
3232
* @param snapshots the constructor creates a sorted copy of snapshots.
3333
* @throws IllegalArgumentException if snapshots with the same Prometheus name have conflicting
34-
* types
34+
* types or have duplicate label sets
3535
*/
3636
public MetricSnapshots(Collection<MetricSnapshot> snapshots) {
3737
validateTypeConsistency(snapshots);
38+
validateNoDuplicateLabelsAcrossSnapshots(snapshots);
3839
List<MetricSnapshot> list = new ArrayList<>(snapshots);
3940
list.sort(comparing(s -> s.getMetadata().getPrometheusName()));
4041
this.snapshots = unmodifiableList(list);
@@ -60,6 +61,54 @@ private static void validateTypeConsistency(Collection<MetricSnapshot> snapshots
6061
}
6162
}
6263

64+
/**
65+
* Validates that snapshots with the same Prometheus name don't have overlapping label sets.
66+
*
67+
* <p>This validation ensures that when multiple collectors with the same metric name are
68+
* registered, each time series (metric name + label set) is unique. Validation happens at scrape
69+
* time rather than registration time for efficiency.
70+
*/
71+
private static void validateNoDuplicateLabelsAcrossSnapshots(
72+
Collection<MetricSnapshot> snapshots) {
73+
// Group snapshots by Prometheus name
74+
Map<String, List<MetricSnapshot>> snapshotsByName = new HashMap<>();
75+
for (MetricSnapshot snapshot : snapshots) {
76+
String prometheusName = snapshot.getMetadata().getPrometheusName();
77+
snapshotsByName.computeIfAbsent(prometheusName, k -> new ArrayList<>()).add(snapshot);
78+
}
79+
80+
// For each group with multiple snapshots, check for duplicate labels
81+
for (Map.Entry<String, List<MetricSnapshot>> entry : snapshotsByName.entrySet()) {
82+
List<MetricSnapshot> group = entry.getValue();
83+
if (group.size() > 1) {
84+
validateNoDuplicateLabelsInGroup(group);
85+
}
86+
}
87+
}
88+
89+
/**
90+
* Validates that a group of snapshots with the same Prometheus name don't have duplicate label
91+
* sets.
92+
*/
93+
private static void validateNoDuplicateLabelsInGroup(List<MetricSnapshot> snapshots) {
94+
String metricName = snapshots.get(0).getMetadata().getName();
95+
Set<Labels> seenLabels = new HashSet<>();
96+
97+
for (MetricSnapshot snapshot : snapshots) {
98+
for (DataPointSnapshot dataPoint : snapshot.getDataPoints()) {
99+
Labels labels = dataPoint.getLabels();
100+
if (!seenLabels.add(labels)) {
101+
throw new IllegalArgumentException(
102+
"Duplicate labels detected for metric '"
103+
+ metricName
104+
+ "' with labels "
105+
+ labels
106+
+ ". Each time series (metric name + label set) must be unique.");
107+
}
108+
}
109+
}
110+
}
111+
63112
public static MetricSnapshots of(MetricSnapshot... snapshots) {
64113
return new MetricSnapshots(snapshots);
65114
}

‎prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java‎

Lines changed: 4 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -141,49 +141,6 @@ public MetricType getMetricType() {
141141
"All collectors with the same Prometheus name must have the same type.");
142142
}
143143

144-
@Test
145-
public void register_duplicateName_sameLabelsNotAllowed() {
146-
Collector counter1 =
147-
new Collector() {
148-
@Override
149-
public MetricSnapshot collect() {
150-
return CounterSnapshot.builder()
151-
.name("my_metric")
152-
.dataPoint(CounterSnapshot.CounterDataPointSnapshot.builder().value(1).build())
153-
.build();
154-
}
155-
156-
@Override
157-
public String getPrometheusName() {
158-
return "my_metric_total";
159-
}
160-
};
161-
162-
Collector counter2 =
163-
new Collector() {
164-
@Override
165-
public MetricSnapshot collect() {
166-
return CounterSnapshot.builder()
167-
.name("my_metric")
168-
.dataPoint(CounterSnapshot.CounterDataPointSnapshot.builder().value(2).build())
169-
.build();
170-
}
171-
172-
@Override
173-
public String getPrometheusName() {
174-
return "my_metric_total";
175-
}
176-
};
177-
178-
PrometheusRegistry registry = new PrometheusRegistry();
179-
registry.register(counter1);
180-
181-
assertThatThrownBy(() -> registry.register(counter2))
182-
.isInstanceOf(IllegalArgumentException.class)
183-
.hasMessageContaining("Duplicate labels detected for metric 'my_metric'")
184-
.hasMessageContaining("Each time series (metric name + label set) must be unique.");
185-
}
186-
187144
@Test
188145
public void registerOk() {
189146
PrometheusRegistry registry = new PrometheusRegistry();
@@ -307,7 +264,7 @@ public MetricSnapshot collect() {
307264
.dataPoint(
308265
CounterSnapshot.CounterDataPointSnapshot.builder()
309266
.labels(Labels.of("uri", "/hello", "outcome", "SUCCESS"))
310-
.value(50) // Different value!
267+
.value(50)
311268
.build())
312269
.build();
313270
}
@@ -319,9 +276,10 @@ public String getPrometheusName() {
319276
};
320277

321278
registry.register(counter1);
279+
registry.register(counter2);
322280

323-
// Registration should throw exception due to duplicate time series (same name + same labels)
324-
assertThatThrownBy(() -> registry.register(counter2))
281+
// Scrape should throw exception due to duplicate time series (same name + same labels)
282+
assertThatThrownBy(registry::scrape)
325283
.isInstanceOf(IllegalArgumentException.class)
326284
.hasMessageContaining("Duplicate labels detected")
327285
.hasMessageContaining("api_responses");

0 commit comments

Comments
 (0)