From a85e35aa1e5460d6a474c0e27345fecfc17daacb Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Tue, 20 Jan 2026 06:41:22 -0500 Subject: [PATCH 01/23] Type and label schema validation during registration Signed-off-by: Jay DeLuca --- .../metrics/core/metrics/Counter.java | 6 + .../core/metrics/CounterWithCallback.java | 6 + .../metrics/core/metrics/Gauge.java | 6 + .../core/metrics/GaugeWithCallback.java | 6 + .../metrics/core/metrics/Histogram.java | 6 + .../prometheus/metrics/core/metrics/Info.java | 6 + .../core/metrics/MetricWithFixedMetadata.java | 12 + .../metrics/core/metrics/StateSet.java | 6 + .../metrics/core/metrics/Summary.java | 6 + .../core/metrics/SummaryWithCallback.java | 6 + .../metrics/model/registry/Collector.java | 34 +++ .../metrics/model/registry/MetricType.java | 18 ++ .../model/registry/MultiCollector.java | 35 +++ .../model/registry/PrometheusRegistry.java | 156 +++++++++- .../registry/PrometheusRegistryTest.java | 272 +++++++++++++++++- 15 files changed, 558 insertions(+), 23 deletions(-) create mode 100644 prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MetricType.java diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Counter.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Counter.java index a2bac20d2..c5f2f1cff 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Counter.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Counter.java @@ -5,6 +5,7 @@ import io.prometheus.metrics.core.datapoints.CounterDataPoint; import io.prometheus.metrics.core.exemplars.ExemplarSampler; import io.prometheus.metrics.core.exemplars.ExemplarSamplerConfig; +import io.prometheus.metrics.model.registry.MetricType; import io.prometheus.metrics.model.snapshots.CounterSnapshot; import io.prometheus.metrics.model.snapshots.Exemplar; import io.prometheus.metrics.model.snapshots.Labels; @@ -92,6 +93,11 @@ protected CounterSnapshot collect(List labels, List metricDat return new CounterSnapshot(getMetadata(), data); } + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + @Override protected DataPoint newDataPoint() { if (exemplarSamplerConfig != null) { diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CounterWithCallback.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CounterWithCallback.java index 044644ec5..3a818c004 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CounterWithCallback.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/CounterWithCallback.java @@ -1,6 +1,7 @@ package io.prometheus.metrics.core.metrics; import io.prometheus.metrics.config.PrometheusProperties; +import io.prometheus.metrics.model.registry.MetricType; import io.prometheus.metrics.model.snapshots.CounterSnapshot; import java.util.ArrayList; import java.util.Collections; @@ -50,6 +51,11 @@ public CounterSnapshot collect() { return new CounterSnapshot(getMetadata(), dataPoints); } + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + public static Builder builder() { return new Builder(PrometheusProperties.get()); } diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Gauge.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Gauge.java index 5850a1cfe..8b1f31409 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Gauge.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Gauge.java @@ -5,6 +5,7 @@ import io.prometheus.metrics.core.datapoints.GaugeDataPoint; import io.prometheus.metrics.core.exemplars.ExemplarSampler; import io.prometheus.metrics.core.exemplars.ExemplarSamplerConfig; +import io.prometheus.metrics.model.registry.MetricType; import io.prometheus.metrics.model.snapshots.Exemplar; import io.prometheus.metrics.model.snapshots.GaugeSnapshot; import io.prometheus.metrics.model.snapshots.Labels; @@ -94,6 +95,11 @@ protected GaugeSnapshot collect(List labels, List metricData) return new GaugeSnapshot(getMetadata(), dataPointSnapshots); } + @Override + public MetricType getMetricType() { + return MetricType.GAUGE; + } + @Override protected DataPoint newDataPoint() { if (exemplarSamplerConfig != null) { diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/GaugeWithCallback.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/GaugeWithCallback.java index 82f26afe1..88aee225f 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/GaugeWithCallback.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/GaugeWithCallback.java @@ -1,6 +1,7 @@ package io.prometheus.metrics.core.metrics; import io.prometheus.metrics.config.PrometheusProperties; +import io.prometheus.metrics.model.registry.MetricType; import io.prometheus.metrics.model.snapshots.GaugeSnapshot; import java.util.ArrayList; import java.util.Collections; @@ -54,6 +55,11 @@ public GaugeSnapshot collect() { return new GaugeSnapshot(getMetadata(), dataPoints); } + @Override + public MetricType getMetricType() { + return MetricType.GAUGE; + } + public static Builder builder() { return new Builder(PrometheusProperties.get()); } diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Histogram.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Histogram.java index 85f6225d3..a2cfd79f3 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Histogram.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Histogram.java @@ -7,6 +7,7 @@ import io.prometheus.metrics.core.exemplars.ExemplarSampler; import io.prometheus.metrics.core.exemplars.ExemplarSamplerConfig; import io.prometheus.metrics.core.util.Scheduler; +import io.prometheus.metrics.model.registry.MetricType; import io.prometheus.metrics.model.snapshots.ClassicHistogramBuckets; import io.prometheus.metrics.model.snapshots.Exemplars; import io.prometheus.metrics.model.snapshots.HistogramSnapshot; @@ -649,6 +650,11 @@ protected HistogramSnapshot collect(List labels, List metricD return new HistogramSnapshot(getMetadata(), data); } + @Override + public MetricType getMetricType() { + return MetricType.HISTOGRAM; + } + @Override protected DataPoint newDataPoint() { return new DataPoint(); diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Info.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Info.java index d7aa6be70..011f0bb73 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Info.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Info.java @@ -1,6 +1,7 @@ package io.prometheus.metrics.core.metrics; import io.prometheus.metrics.config.PrometheusProperties; +import io.prometheus.metrics.model.registry.MetricType; import io.prometheus.metrics.model.snapshots.InfoSnapshot; import io.prometheus.metrics.model.snapshots.Labels; import io.prometheus.metrics.model.snapshots.Unit; @@ -105,6 +106,11 @@ public InfoSnapshot collect() { return new InfoSnapshot(getMetadata(), data); } + @Override + public MetricType getMetricType() { + return MetricType.INFO; + } + public static Builder builder() { return new Builder(PrometheusProperties.get()); } diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java index 6f6afa482..a1c1f1576 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java @@ -1,12 +1,15 @@ package io.prometheus.metrics.core.metrics; import io.prometheus.metrics.config.PrometheusProperties; +import io.prometheus.metrics.model.snapshots.Label; import io.prometheus.metrics.model.snapshots.Labels; import io.prometheus.metrics.model.snapshots.MetricMetadata; import io.prometheus.metrics.model.snapshots.PrometheusNaming; import io.prometheus.metrics.model.snapshots.Unit; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import javax.annotation.Nullable; /** @@ -48,6 +51,15 @@ public String getPrometheusName() { return metadata.getPrometheusName(); } + @Override + public Set getLabelNames() { + Set names = new HashSet<>(Arrays.asList(labelNames)); + for (Label label : constLabels) { + names.add(PrometheusNaming.prometheusName(label.getName())); + } + return names; + } + public abstract static class Builder, M extends MetricWithFixedMetadata> extends Metric.Builder { diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/StateSet.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/StateSet.java index 4dbaf8ad5..740183f31 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/StateSet.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/StateSet.java @@ -4,6 +4,7 @@ import io.prometheus.metrics.config.PrometheusProperties; import io.prometheus.metrics.core.datapoints.StateSetDataPoint; +import io.prometheus.metrics.model.registry.MetricType; import io.prometheus.metrics.model.snapshots.Labels; import io.prometheus.metrics.model.snapshots.StateSetSnapshot; import java.util.ArrayList; @@ -84,6 +85,11 @@ protected StateSetSnapshot collect(List labels, List metricDa return new StateSetSnapshot(getMetadata(), data); } + @Override + public MetricType getMetricType() { + return MetricType.STATESET; + } + @Override public void setTrue(String state) { getNoLabels().setTrue(state); diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Summary.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Summary.java index 7d964dbb6..bde60771f 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Summary.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/Summary.java @@ -7,6 +7,7 @@ import io.prometheus.metrics.core.datapoints.DistributionDataPoint; import io.prometheus.metrics.core.exemplars.ExemplarSampler; import io.prometheus.metrics.core.exemplars.ExemplarSamplerConfig; +import io.prometheus.metrics.model.registry.MetricType; import io.prometheus.metrics.model.snapshots.Exemplars; import io.prometheus.metrics.model.snapshots.Labels; import io.prometheus.metrics.model.snapshots.Quantile; @@ -118,6 +119,11 @@ protected SummarySnapshot collect(List labels, List metricDat return new SummarySnapshot(getMetadata(), data); } + @Override + public MetricType getMetricType() { + return MetricType.SUMMARY; + } + @Override protected DataPoint newDataPoint() { return new DataPoint(); diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/SummaryWithCallback.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/SummaryWithCallback.java index 3c4a910ea..fa823e68e 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/SummaryWithCallback.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/SummaryWithCallback.java @@ -1,6 +1,7 @@ package io.prometheus.metrics.core.metrics; import io.prometheus.metrics.config.PrometheusProperties; +import io.prometheus.metrics.model.registry.MetricType; import io.prometheus.metrics.model.snapshots.Exemplars; import io.prometheus.metrics.model.snapshots.Quantiles; import io.prometheus.metrics.model.snapshots.SummarySnapshot; @@ -63,6 +64,11 @@ public SummarySnapshot collect() { return new SummarySnapshot(getMetadata(), dataPoints); } + @Override + public MetricType getMetricType() { + return MetricType.SUMMARY; + } + public static Builder builder() { return new Builder(PrometheusProperties.get()); } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java index b7154ae70..b2551c81c 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java @@ -1,6 +1,7 @@ package io.prometheus.metrics.model.registry; import io.prometheus.metrics.model.snapshots.MetricSnapshot; +import java.util.Set; import java.util.function.Predicate; import javax.annotation.Nullable; @@ -78,4 +79,37 @@ default MetricSnapshot collect( default String getPrometheusName() { return null; } + + /** + * Returns the metric type for registration-time validation. + * + *

This is used to prevent different metric types (e.g., Counter and Gauge) from sharing the + * same name. Returning {@code null} means type validation is skipped for this collector. + * + * @return the metric type, or {@code null} to skip validation + */ + @Nullable + default MetricType getMetricType() { + return null; + } + + /** + * Returns the complete set of label names for this metric. + * + *

This includes both dynamic label names (specified in {@code labelNames()}) and constant + * label names (specified in {@code constLabels()}). Label names are normalized using Prometheus + * naming conventions. + * + *

This is used for registration-time validation to prevent duplicate label schemas for the + * same metric name. Two collectors with the same name and type can coexist if they have different + * label name sets. + * + *

Returning {@code null} means label schema validation is skipped for this collector. + * + * @return the set of all label names, or {@code null} to skip validation + */ + @Nullable + default Set getLabelNames() { + return null; + } } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MetricType.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MetricType.java new file mode 100644 index 000000000..5258da84e --- /dev/null +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MetricType.java @@ -0,0 +1,18 @@ +package io.prometheus.metrics.model.registry; + +/** + * Represents the type of Prometheus metric. + * + *

This enum is used for registration-time validation to ensure that metrics with the same name + * have consistent types across all registered collectors. + */ +public enum MetricType { + COUNTER, + GAUGE, + HISTOGRAM, + SUMMARY, + INFO, + STATESET, + /** Unknown metric type, used as a fallback. */ + UNKNOWN +} diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java index d1051958d..879efaba6 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java @@ -4,6 +4,7 @@ import io.prometheus.metrics.model.snapshots.MetricSnapshots; import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.function.Predicate; import javax.annotation.Nullable; @@ -70,4 +71,38 @@ default MetricSnapshots collect( default List getPrometheusNames() { return Collections.emptyList(); } + + /** + * Returns the metric type for the given Prometheus name. + * + *

This is used for per-name type validation during registration. Returning {@code null} means + * type validation is skipped for that specific metric name. + * + * @param prometheusName the Prometheus metric name + * @return the metric type for the given name, or {@code null} to skip validation + */ + @Nullable + default MetricType getMetricType(String prometheusName) { + return null; + } + + /** + * Returns the complete set of label names for the given Prometheus name. + * + *

This includes both dynamic label names and constant label names. Label names are normalized + * using Prometheus naming conventions (dots converted to underscores). + * + *

This is used for per-name label schema validation during registration. Two collectors with + * the same name and type can coexist if they have different label name sets. + * + *

Returning {@code null} means label schema validation is skipped for that specific metric + * name. + * + * @param prometheusName the Prometheus metric name + * @return the set of all label names for the given name, or {@code null} to skip validation + */ + @Nullable + default Set getLabelNames(String prometheusName) { + return null; + } } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 7db568d95..71e0f83bb 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -18,27 +18,133 @@ public class PrometheusRegistry { private final Set prometheusNames = ConcurrentHashMap.newKeySet(); private final List collectors = new CopyOnWriteArrayList<>(); private final List multiCollectors = new CopyOnWriteArrayList<>(); + private final ConcurrentHashMap registered = new ConcurrentHashMap<>(); + + /** + * Tracks registration information for each metric name to enable validation of type consistency + * and label schema uniqueness. + */ + private static class RegistrationInfo { + private final MetricType type; + private final Set> labelSets; + + RegistrationInfo(MetricType type, @Nullable Set labelNames) { + this.type = type; + this.labelSets = ConcurrentHashMap.newKeySet(); + if (labelNames != null) { + this.labelSets.add(labelNames); + } + } + + /** + * Adds a new label schema to this registration. + * + * @param labelNames the label names to add + * @return true if the label schema was added, false if it already exists + */ + boolean addLabelSet(@Nullable Set labelNames) { + if (labelNames == null) { + return true; + } + return labelSets.add(labelNames); + } + + MetricType getType() { + return type; + } + } public void register(Collector collector) { + if (collectors.contains(collector)) { + return; + } + String prometheusName = collector.getPrometheusName(); + MetricType metricType = collector.getMetricType(); + Set labelNames = collector.getLabelNames(); + + if (prometheusName != null && metricType != null) { + registered.compute( + prometheusName, + (name, existingInfo) -> { + if (existingInfo == null) { + return new RegistrationInfo(metricType, labelNames); + } else { + if (existingInfo.getType() != metricType) { + throw new IllegalArgumentException( + prometheusName + + ": Conflicting metric types. Existing: " + + existingInfo.getType() + + ", new: " + + metricType); + } + + // Check label schema uniqueness (if label names provided) + if (labelNames != null) { + if (!existingInfo.addLabelSet(labelNames)) { + throw new IllegalArgumentException( + prometheusName + + ": Duplicate label schema. A metric with the same name, type, and label" + + " names is already registered."); + } + } + + return existingInfo; + } + }); + } + if (prometheusName != null) { - if (!prometheusNames.add(prometheusName)) { - throw new IllegalStateException( - "Can't register " - + prometheusName - + " because a metric with that name is already registered."); - } + prometheusNames.add(prometheusName); } + collectors.add(collector); } public void register(MultiCollector collector) { - for (String prometheusName : collector.getPrometheusNames()) { - if (!prometheusNames.add(prometheusName)) { - throw new IllegalStateException( - "Can't register " + prometheusName + " because that name is already registered."); + if (multiCollectors.contains(collector)) { + return; + } + + List names = collector.getPrometheusNames(); + + for (String prometheusName : names) { + MetricType metricType = collector.getMetricType(prometheusName); + Set labelNames = collector.getLabelNames(prometheusName); + + if (metricType != null) { + registered.compute( + prometheusName, + (name, existingInfo) -> { + if (existingInfo == null) { + return new RegistrationInfo(metricType, labelNames); + } else { + if (existingInfo.getType() != metricType) { + throw new IllegalArgumentException( + prometheusName + + ": Conflicting metric types. Existing: " + + existingInfo.getType() + + ", new: " + + metricType); + } + + if (labelNames != null) { + if (!existingInfo.addLabelSet(labelNames)) { + throw new IllegalArgumentException( + prometheusName + + ": Duplicate label schema. A metric with the same name, type, and" + + " label names is already registered."); + } + } + + return existingInfo; + } + }); } + + prometheusNames.add(prometheusName); } + multiCollectors.add(collector); } @@ -46,14 +152,39 @@ public void unregister(Collector collector) { collectors.remove(collector); String prometheusName = collector.getPrometheusName(); if (prometheusName != null) { - prometheusNames.remove(collector.getPrometheusName()); + // Check if any other collectors are still using this name + nameInUse(prometheusName); } } public void unregister(MultiCollector collector) { multiCollectors.remove(collector); for (String prometheusName : collector.getPrometheusNames()) { - prometheusNames.remove(prometheusName(prometheusName)); + // Check if any other collectors are still using this name + nameInUse(prometheusName); + } + } + + private void nameInUse(String prometheusName) { + boolean nameStillInUse = false; + for (Collector c : collectors) { + if (prometheusName.equals(c.getPrometheusName())) { + nameStillInUse = true; + break; + } + } + if (!nameStillInUse) { + for (MultiCollector mc : multiCollectors) { + if (mc.getPrometheusNames().contains(prometheusName)) { + nameStillInUse = true; + break; + } + } + } + if (!nameStillInUse) { + prometheusNames.remove(prometheusName); + // Also remove from registered since no collectors use this name anymore + registered.remove(prometheusName); } } @@ -61,6 +192,7 @@ public void clear() { collectors.clear(); multiCollectors.clear(); prometheusNames.clear(); + registered.clear(); } public MetricSnapshots scrape() { diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index 9e87f1fc9..93e1c62ab 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -2,20 +2,20 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import io.prometheus.metrics.model.snapshots.CounterSnapshot; import io.prometheus.metrics.model.snapshots.GaugeSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.junit.jupiter.api.Test; class PrometheusRegistryTest { - Collector noName = () -> GaugeSnapshot.builder().name("no_name_gauge").build(); - Collector counterA1 = new Collector() { @Override @@ -86,8 +86,10 @@ public void registerNoName() { PrometheusRegistry registry = new PrometheusRegistry(); // If the collector does not have a name at registration time, there is no conflict during // registration. - registry.register(noName); - registry.register(noName); + Collector noName1 = () -> GaugeSnapshot.builder().name("no_name_gauge").build(); + Collector noName2 = () -> GaugeSnapshot.builder().name("no_name_gauge").build(); + registry.register(noName1); + registry.register(noName2); // However, at scrape time the collector has to provide a metric name, and then we'll get a // duplicate name error. assertThatCode(registry::scrape) @@ -96,11 +98,258 @@ public void registerNoName() { } @Test - public void registerDuplicateName() { + public void registerDuplicateName_withoutTypeInfo_allowedForBackwardCompatibility() { + PrometheusRegistry registry = new PrometheusRegistry(); + // counterA1 and counterA2 don't provide type/label info, so validation is skipped + registry.register(counterA1); + // This now succeeds for backward compatibility (validation skipped when type is null) + assertThatCode(() -> registry.register(counterA2)).doesNotThrowAnyException(); + } + + @Test + public void register_duplicateName_differentTypes_notAllowed() { PrometheusRegistry registry = new PrometheusRegistry(); + + Collector counterA1 = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("counter_a").build(); + } + + @Override + public String getPrometheusName() { + return "counter_a"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + }; + + Collector gaugeA1 = + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder().name("counter_a").build(); + } + + @Override + public String getPrometheusName() { + return "counter_a"; + } + + @Override + public MetricType getMetricType() { + return MetricType.GAUGE; + } + }; + registry.register(counterA1); - assertThatExceptionOfType(IllegalStateException.class) - .isThrownBy(() -> registry.register(counterA2)); + + assertThatThrownBy(() -> registry.register(gaugeA1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Conflicting metric types"); + } + + @Test + public void register_sameName_sameType_differentLabelSchemas_allowed() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector counterWithPathLabel = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests_total").build(); + } + + @Override + public String getPrometheusName() { + return "requests_total"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(Arrays.asList("path", "status")); + } + }; + + Collector counterWithRegionLabel = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests_total").build(); + } + + @Override + public String getPrometheusName() { + return "requests_total"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(Arrays.asList("region")); + } + }; + + // Both collectors have same name and type, but different label schemas + // This should succeed + registry.register(counterWithPathLabel); + assertThatCode(() -> registry.register(counterWithRegionLabel)).doesNotThrowAnyException(); + } + + @Test + public void register_sameName_sameType_sameLabelSchema_notAllowed() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector counter1 = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests_total").build(); + } + + @Override + public String getPrometheusName() { + return "requests_total"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(Arrays.asList("path", "status")); + } + }; + + Collector counter2 = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests_total").build(); + } + + @Override + public String getPrometheusName() { + return "requests_total"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(Arrays.asList("path", "status")); + } + }; + + registry.register(counter1); + + // Second collector has same name, type, and label schema - should fail + assertThatThrownBy(() -> registry.register(counter2)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Duplicate label schema"); + } + + @Test + public void register_backwardCompatibility_nullTypeAndLabels_skipsValidation() { + PrometheusRegistry registry = new PrometheusRegistry(); + + // Collector without getMetricType() and getLabelNames() - returns null (default) + Collector legacyCollector1 = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("legacy_metric").build(); + } + + @Override + public String getPrometheusName() { + return "legacy_metric"; + } + }; + + Collector legacyCollector2 = + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder().name("legacy_metric").build(); + } + + @Override + public String getPrometheusName() { + return "legacy_metric"; + } + }; + + // Both collectors have the same name but no type/label info + // Should succeed because validation is skipped + registry.register(legacyCollector1); + assertThatCode(() -> registry.register(legacyCollector2)).doesNotThrowAnyException(); + } + + @Test + public void register_multiCollector_withTypeValidation() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector counter = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("shared_metric").build(); + } + + @Override + public String getPrometheusName() { + return "shared_metric"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + }; + + MultiCollector multiWithGauge = + new MultiCollector() { + @Override + public MetricSnapshots collect() { + return new MetricSnapshots(GaugeSnapshot.builder().name("shared_metric").build()); + } + + @Override + public List getPrometheusNames() { + return Arrays.asList("shared_metric"); + } + + @Override + public MetricType getMetricType(String prometheusName) { + return MetricType.GAUGE; + } + }; + + registry.register(counter); + + // MultiCollector tries to register a Gauge with the same name as existing Counter + assertThatThrownBy(() -> registry.register(multiWithGauge)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Conflicting metric types"); } @Test @@ -122,11 +371,12 @@ public void registerOk() { } @Test - public void registerDuplicateMultiCollector() { + public void registerDuplicateMultiCollector_withoutTypeInfo_allowedForBackwardCompatibility() { PrometheusRegistry registry = new PrometheusRegistry(); + // multiCollector doesn't provide type/label info, so validation is skipped registry.register(multiCollector); - assertThatExceptionOfType(IllegalStateException.class) - .isThrownBy(() -> registry.register(multiCollector)); + // This now succeeds for backward compatibility (validation skipped when type is null) + assertThatCode(() -> registry.register(multiCollector)).doesNotThrowAnyException(); } @Test From 58dddc84dfbc850d203cb96a18e46accf68e75d6 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Tue, 20 Jan 2026 08:30:56 -0500 Subject: [PATCH 02/23] start adding exposition merging Signed-off-by: Jay DeLuca --- .../client/it/common/ExporterTest.java | 2 +- .../pom.xml | 59 ++++ .../DuplicateMetricsSample.java | 89 ++++++ .../it/exporter/test/DuplicateMetricsIT.java | 199 ++++++++++++ integration-tests/it-exporter/pom.xml | 1 + .../PrometheusProtobufWriterImpl.java | 7 +- .../DuplicateNamesProtobufTest.java | 300 ++++++++++++++++++ .../expositionformats/TextFormatUtil.java | 124 +++++++- .../model/registry/PrometheusRegistry.java | 105 ++++-- .../model/snapshots/MetricSnapshots.java | 13 - .../registry/PrometheusRegistryTest.java | 18 +- 11 files changed, 857 insertions(+), 60 deletions(-) create mode 100644 integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/pom.xml create mode 100644 integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java create mode 100644 integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java create mode 100644 prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java diff --git a/integration-tests/it-common/src/test/java/io/prometheus/client/it/common/ExporterTest.java b/integration-tests/it-common/src/test/java/io/prometheus/client/it/common/ExporterTest.java index 605e760f4..73972df13 100644 --- a/integration-tests/it-common/src/test/java/io/prometheus/client/it/common/ExporterTest.java +++ b/integration-tests/it-common/src/test/java/io/prometheus/client/it/common/ExporterTest.java @@ -24,7 +24,7 @@ import org.testcontainers.containers.GenericContainer; public abstract class ExporterTest { - private final GenericContainer sampleAppContainer; + protected final GenericContainer sampleAppContainer; private final Volume sampleAppVolume; protected final String sampleApp; diff --git a/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/pom.xml b/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/pom.xml new file mode 100644 index 000000000..ca982769b --- /dev/null +++ b/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/pom.xml @@ -0,0 +1,59 @@ + + + 4.0.0 + + + io.prometheus + it-exporter + 1.5.0-SNAPSHOT + + + it-exporter-duplicate-metrics-sample + + Integration Tests - Duplicate Metrics Sample + + HTTPServer Sample demonstrating duplicate metric names with different label sets + + + + + io.prometheus + prometheus-metrics-exporter-httpserver + ${project.version} + + + io.prometheus + prometheus-metrics-core + ${project.version} + + + + + exporter-duplicate-metrics-sample + + + org.apache.maven.plugins + maven-shade-plugin + + + package + + shade + + + + + + io.prometheus.metrics.it.exporter.duplicatemetrics.DuplicateMetricsSample + + + + + + + + + + \ No newline at end of file diff --git a/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java b/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java new file mode 100644 index 000000000..53d4fefdb --- /dev/null +++ b/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java @@ -0,0 +1,89 @@ +package io.prometheus.metrics.it.exporter.duplicatemetrics; + +import io.prometheus.metrics.core.metrics.Counter; +import io.prometheus.metrics.core.metrics.Gauge; +import io.prometheus.metrics.exporter.httpserver.HTTPServer; +import io.prometheus.metrics.model.snapshots.Unit; +import java.io.IOException; + +/** Integration test sample demonstrating metrics with duplicate names but different label sets. */ +public class DuplicateMetricsSample { + + public static void main(String[] args) throws IOException, InterruptedException { + if (args.length != 1) { + System.err.println("Usage: java -jar duplicate-metrics-sample.jar "); + System.exit(1); + } + + int port = parsePortOrExit(args[0]); + run(port); + } + + private static void run(int port) throws IOException, InterruptedException { + // Register multiple counters with the same Prometheus name "http_requests_total" + // but different label sets + Counter requestsSuccess = + Counter.builder() + .name("http_requests_total") + .help("Total HTTP requests by status") + .labelNames("status", "method") + .register(); + requestsSuccess.labelValues("success", "GET").inc(150); + requestsSuccess.labelValues("success", "POST").inc(45); + + Counter requestsError = + Counter.builder() + .name("http_requests_total") + .help("Total HTTP requests by status") + .labelNames("status", "endpoint") + .register(); + requestsError.labelValues("error", "/api").inc(5); + requestsError.labelValues("error", "/health").inc(2); + + // Register multiple gauges with the same Prometheus name "active_connections" + // but different label sets + Gauge connectionsByRegion = + Gauge.builder() + .name("active_connections") + .help("Active connections") + .labelNames("region", "protocol") + .register(); + connectionsByRegion.labelValues("us-east", "http").set(42); + connectionsByRegion.labelValues("us-west", "http").set(38); + connectionsByRegion.labelValues("eu-west", "https").set(55); + + Gauge connectionsByPool = + Gauge.builder() + .name("active_connections") + .help("Active connections") + .labelNames("pool", "type") + .register(); + connectionsByPool.labelValues("primary", "read").set(30); + connectionsByPool.labelValues("replica", "write").set(10); + + // Also add a regular metric without duplicates for reference + Counter uniqueMetric = + Counter.builder() + .name("unique_metric_total") + .help("A unique metric for reference") + .unit(Unit.BYTES) + .register(); + uniqueMetric.inc(1024); + + HTTPServer server = HTTPServer.builder().port(port).buildAndStart(); + + System.out.println( + "DuplicateMetricsSample listening on http://localhost:" + server.getPort() + "/metrics"); + Thread.currentThread().join(); // wait forever + } + + private static int parsePortOrExit(String port) { + try { + return Integer.parseInt(port); + } catch (NumberFormatException e) { + System.err.println("\"" + port + "\": Invalid port number."); + System.exit(1); + } + return 0; // this won't happen + } +} \ No newline at end of file diff --git a/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java b/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java new file mode 100644 index 000000000..0d1688b2d --- /dev/null +++ b/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java @@ -0,0 +1,199 @@ +package io.prometheus.metrics.it.exporter.test; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.prometheus.client.it.common.ExporterTest; +import io.prometheus.metrics.expositionformats.generated.com_google_protobuf_4_33_4.Metrics; +import java.io.IOException; +import java.net.URISyntaxException; +import java.util.List; +import org.junit.jupiter.api.Test; + +/** + * Integration test for duplicate metric names with different label sets. + * + *

This test validates that: + * + *

    + *
  • Multiple metrics with the same Prometheus name but different labels can be registered + *
  • All exposition formats (text, OpenMetrics, protobuf) correctly merge and expose them + *
  • The merged output is valid and scrapeable by Prometheus + *
+ */ +class DuplicateMetricsIT extends ExporterTest { + + public DuplicateMetricsIT() throws IOException, URISyntaxException { + super("exporter-duplicate-metrics-sample"); + } + + @Override + protected void start(String outcome) { + sampleAppContainer.withCommand("java", "-jar", "/app/" + sampleApp + ".jar", "9400").start(); + } + + @Test + void testDuplicateMetricsInPrometheusTextFormat() throws IOException { + start(); + Response response = scrape("GET", ""); + assertThat(response.status).isEqualTo(200); + assertContentType( + "text/plain; version=0.0.4; charset=utf-8", response.getHeader("Content-Type")); + + String expected = + """ + # HELP active_connections Active connections + # TYPE active_connections gauge + active_connections{pool="primary",type="read"} 30.0 + active_connections{pool="replica",type="write"} 10.0 + active_connections{protocol="http",region="us-east"} 42.0 + active_connections{protocol="http",region="us-west"} 38.0 + active_connections{protocol="https",region="eu-west"} 55.0 + # HELP http_requests_total Total HTTP requests by status + # TYPE http_requests_total counter + http_requests_total{endpoint="/api",status="error"} 5.0 + http_requests_total{endpoint="/health",status="error"} 2.0 + http_requests_total{method="GET",status="success"} 150.0 + http_requests_total{method="POST",status="success"} 45.0 + # HELP unique_metric_bytes_total A unique metric for reference + # TYPE unique_metric_bytes_total counter + unique_metric_bytes_total 1024.0 + """; + + assertThat(response.stringBody()).isEqualTo(expected); + } + + @Test + void testDuplicateMetricsInOpenMetricsTextFormat() throws IOException { + start(); + Response response = + scrape("GET", "", "Accept", "application/openmetrics-text; version=1.0.0; charset=utf-8"); + assertThat(response.status).isEqualTo(200); + assertContentType( + "application/openmetrics-text; version=1.0.0; charset=utf-8", + response.getHeader("Content-Type")); + + // OpenMetrics format should have UNIT for unique_metric_bytes (base name without _total) + String expected = + """ + # TYPE active_connections gauge + # HELP active_connections Active connections + active_connections{pool="primary",type="read"} 30.0 + active_connections{pool="replica",type="write"} 10.0 + active_connections{protocol="http",region="us-east"} 42.0 + active_connections{protocol="http",region="us-west"} 38.0 + active_connections{protocol="https",region="eu-west"} 55.0 + # TYPE http_requests counter + # HELP http_requests Total HTTP requests by status + http_requests_total{endpoint="/api",status="error"} 5.0 + http_requests_total{endpoint="/health",status="error"} 2.0 + http_requests_total{method="GET",status="success"} 150.0 + http_requests_total{method="POST",status="success"} 45.0 + # TYPE unique_metric_bytes counter + # UNIT unique_metric_bytes bytes + # HELP unique_metric_bytes A unique metric for reference + unique_metric_bytes_total 1024.0 + # EOF + """; + + assertThat(response.stringBody()).isEqualTo(expected); + } + + @Test + void testDuplicateMetricsInPrometheusProtobufFormat() throws IOException { + start(); + Response response = + scrape( + "GET", + "", + "Accept", + "application/vnd.google.protobuf; proto=io.prometheus.client.MetricFamily;" + + " encoding=delimited"); + assertThat(response.status).isEqualTo(200); + assertContentType( + "application/vnd.google.protobuf; proto=io.prometheus.client.MetricFamily;" + + " encoding=delimited", + response.getHeader("Content-Type")); + + List metrics = response.protoBody(); + + // Should have exactly 3 metric families (active_connections, http_requests_total, + // unique_metric_bytes_total) + assertThat(metrics).hasSize(3); + + // Metrics are sorted by name + assertThat(metrics.get(0).getName()).isEqualTo("active_connections"); + assertThat(metrics.get(1).getName()).isEqualTo("http_requests_total"); + assertThat(metrics.get(2).getName()).isEqualTo("unique_metric_bytes_total"); + + // Verify active_connections has all 5 data points merged + Metrics.MetricFamily activeConnections = metrics.get(0); + assertThat(activeConnections.getType()).isEqualTo(Metrics.MetricType.GAUGE); + assertThat(activeConnections.getHelp()).isEqualTo("Active connections"); + assertThat(activeConnections.getMetricList()).hasSize(5); + + // Verify http_requests_total has all 4 data points merged + Metrics.MetricFamily httpRequests = metrics.get(1); + assertThat(httpRequests.getType()).isEqualTo(Metrics.MetricType.COUNTER); + assertThat(httpRequests.getHelp()).isEqualTo("Total HTTP requests by status"); + assertThat(httpRequests.getMetricList()).hasSize(4); + + // Verify each data point has the expected labels + boolean foundSuccessGet = false; + boolean foundSuccessPost = false; + boolean foundErrorApi = false; + boolean foundErrorHealth = false; + + for (Metrics.Metric metric : httpRequests.getMetricList()) { + List labels = metric.getLabelList(); + if (hasLabel(labels, "status", "success") && hasLabel(labels, "method", "GET")) { + assertThat(metric.getCounter().getValue()).isEqualTo(150.0); + foundSuccessGet = true; + } else if (hasLabel(labels, "status", "success") && hasLabel(labels, "method", "POST")) { + assertThat(metric.getCounter().getValue()).isEqualTo(45.0); + foundSuccessPost = true; + } else if (hasLabel(labels, "status", "error") && hasLabel(labels, "endpoint", "/api")) { + assertThat(metric.getCounter().getValue()).isEqualTo(5.0); + foundErrorApi = true; + } else if (hasLabel(labels, "status", "error") && hasLabel(labels, "endpoint", "/health")) { + assertThat(metric.getCounter().getValue()).isEqualTo(2.0); + foundErrorHealth = true; + } + } + + assertThat(foundSuccessGet).isTrue(); + assertThat(foundSuccessPost).isTrue(); + assertThat(foundErrorApi).isTrue(); + assertThat(foundErrorHealth).isTrue(); + + Metrics.MetricFamily uniqueMetric = metrics.get(2); + assertThat(uniqueMetric.getType()).isEqualTo(Metrics.MetricType.COUNTER); + assertThat(uniqueMetric.getMetricList()).hasSize(1); + assertThat(uniqueMetric.getMetric(0).getCounter().getValue()).isEqualTo(1024.0); + } + + @Test + void testDuplicateMetricsWithNameFilter() throws IOException { + start(); + // Only scrape http_requests_total + Response response = scrape("GET", nameParam()); + assertThat(response.status).isEqualTo(200); + + String body = response.stringBody(); + + assertThat(body) + .contains("http_requests_total{method=\"GET\",status=\"success\"} 150.0") + .contains("http_requests_total{endpoint=\"/api\",status=\"error\"} 5.0"); + + // Should NOT contain active_connections or unique_metric_total + assertThat(body).doesNotContain("active_connections").doesNotContain("unique_metric_total"); + } + + private boolean hasLabel(List labels, String name, String value) { + return labels.stream() + .anyMatch(label -> label.getName().equals(name) && label.getValue().equals(value)); + } + + private String nameParam() { + return "name[]=" + "http_requests_total"; + } +} \ No newline at end of file diff --git a/integration-tests/it-exporter/pom.xml b/integration-tests/it-exporter/pom.xml index a442b9086..c4a29fe74 100644 --- a/integration-tests/it-exporter/pom.xml +++ b/integration-tests/it-exporter/pom.xml @@ -21,6 +21,7 @@ it-exporter-servlet-tomcat-sample it-exporter-servlet-jetty-sample it-exporter-httpserver-sample + it-exporter-duplicate-metrics-sample it-exporter-no-protobuf it-exporter-test it-no-protobuf-test diff --git a/prometheus-metrics-exposition-formats/src/main/java/io/prometheus/metrics/expositionformats/internal/PrometheusProtobufWriterImpl.java b/prometheus-metrics-exposition-formats/src/main/java/io/prometheus/metrics/expositionformats/internal/PrometheusProtobufWriterImpl.java index feaf15b22..ea8c3b8d6 100644 --- a/prometheus-metrics-exposition-formats/src/main/java/io/prometheus/metrics/expositionformats/internal/PrometheusProtobufWriterImpl.java +++ b/prometheus-metrics-exposition-formats/src/main/java/io/prometheus/metrics/expositionformats/internal/PrometheusProtobufWriterImpl.java @@ -6,6 +6,7 @@ import com.google.protobuf.TextFormat; import io.prometheus.metrics.config.EscapingScheme; import io.prometheus.metrics.expositionformats.ExpositionFormatWriter; +import io.prometheus.metrics.expositionformats.TextFormatUtil; import io.prometheus.metrics.expositionformats.generated.com_google_protobuf_4_33_4.Metrics; import io.prometheus.metrics.model.snapshots.ClassicHistogramBuckets; import io.prometheus.metrics.model.snapshots.CounterSnapshot; @@ -43,8 +44,9 @@ public String getContentType() { @Override public String toDebugString(MetricSnapshots metricSnapshots, EscapingScheme escapingScheme) { + MetricSnapshots merged = TextFormatUtil.mergeDuplicates(metricSnapshots); StringBuilder stringBuilder = new StringBuilder(); - for (MetricSnapshot s : metricSnapshots) { + for (MetricSnapshot s : merged) { MetricSnapshot snapshot = SnapshotEscaper.escapeMetricSnapshot(s, escapingScheme); if (!snapshot.getDataPoints().isEmpty()) { stringBuilder.append(TextFormat.printer().printToString(convert(snapshot, escapingScheme))); @@ -57,7 +59,8 @@ public String toDebugString(MetricSnapshots metricSnapshots, EscapingScheme esca public void write( OutputStream out, MetricSnapshots metricSnapshots, EscapingScheme escapingScheme) throws IOException { - for (MetricSnapshot s : metricSnapshots) { + MetricSnapshots merged = TextFormatUtil.mergeDuplicates(metricSnapshots); + for (MetricSnapshot s : merged) { MetricSnapshot snapshot = SnapshotEscaper.escapeMetricSnapshot(s, escapingScheme); if (!snapshot.getDataPoints().isEmpty()) { convert(snapshot, escapingScheme).writeDelimitedTo(out); diff --git a/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java b/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java new file mode 100644 index 000000000..59d1e7905 --- /dev/null +++ b/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java @@ -0,0 +1,300 @@ +package io.prometheus.metrics.expositionformats; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.prometheus.metrics.config.EscapingScheme; +import io.prometheus.metrics.expositionformats.generated.com_google_protobuf_4_33_4.Metrics; +import io.prometheus.metrics.expositionformats.internal.PrometheusProtobufWriterImpl; +import io.prometheus.metrics.model.registry.Collector; +import io.prometheus.metrics.model.registry.PrometheusRegistry; +import io.prometheus.metrics.model.snapshots.CounterSnapshot; +import io.prometheus.metrics.model.snapshots.GaugeSnapshot; +import io.prometheus.metrics.model.snapshots.Labels; +import io.prometheus.metrics.model.snapshots.MetricSnapshot; +import io.prometheus.metrics.model.snapshots.MetricSnapshots; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import org.junit.jupiter.api.Test; + +class DuplicateNamesProtobufTest { + + private static PrometheusRegistry getPrometheusRegistry() { + PrometheusRegistry registry = new PrometheusRegistry(); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels( + Labels.of("uri", "/hello", "outcome", "FAILURE", "error", "TIMEOUT")) + .value(10) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + return registry; + } + + @Test + void testDuplicateNames_differentLabels_producesSingleMetricFamily() throws IOException { + PrometheusRegistry registry = getPrometheusRegistry(); + + MetricSnapshots snapshots = registry.scrape(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + PrometheusProtobufWriterImpl writer = new PrometheusProtobufWriterImpl(); + writer.write(out, snapshots, EscapingScheme.UNDERSCORE_ESCAPING); + + List metricFamilies = parseProtobufOutput(out); + + assertThat(metricFamilies).hasSize(1); + Metrics.MetricFamily family = metricFamilies.get(0); + assertThat(family.getName()).isEqualTo("api_responses_total"); + assertThat(family.getHelp()).isEqualTo("API responses"); + assertThat(family.getType()).isEqualTo(Metrics.MetricType.COUNTER); + assertThat(family.getMetricCount()).isEqualTo(2); + + Metrics.Metric successMetric = + family.getMetricList().stream() + .filter( + m -> + m.getLabelList().stream() + .anyMatch( + l -> l.getName().equals("outcome") && l.getValue().equals("SUCCESS"))) + .findFirst() + .orElseThrow(() -> new AssertionError("SUCCESS metric not found")); + assertThat(successMetric.getCounter().getValue()).isEqualTo(100.0); + + Metrics.Metric failureMetric = + family.getMetricList().stream() + .filter( + m -> + m.getLabelList().stream() + .anyMatch( + l -> + l.getName().equals("outcome") && l.getValue().equals("FAILURE")) + && m.getLabelList().stream() + .anyMatch( + l -> l.getName().equals("error") && l.getValue().equals("TIMEOUT"))) + .findFirst() + .orElseThrow(() -> new AssertionError("FAILURE metric not found")); + assertThat(failureMetric.getCounter().getValue()).isEqualTo(10.0); + } + + @Test + void testDuplicateNames_multipleDataPoints_producesSingleMetricFamily() throws IOException { + PrometheusRegistry registry = new PrometheusRegistry(); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .build()) + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/world", "outcome", "SUCCESS")) + .value(200) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels( + Labels.of("uri", "/hello", "outcome", "FAILURE", "error", "TIMEOUT")) + .value(10) + .build()) + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels( + Labels.of("uri", "/world", "outcome", "FAILURE", "error", "NOT_FOUND")) + .value(5) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + + MetricSnapshots snapshots = registry.scrape(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + PrometheusProtobufWriterImpl writer = new PrometheusProtobufWriterImpl(); + writer.write(out, snapshots, EscapingScheme.UNDERSCORE_ESCAPING); + + List metricFamilies = parseProtobufOutput(out); + + assertThat(metricFamilies).hasSize(1); + Metrics.MetricFamily family = metricFamilies.get(0); + assertThat(family.getName()).isEqualTo("api_responses_total"); + assertThat(family.getMetricCount()).isEqualTo(4); + + long successCount = + family.getMetricList().stream() + .filter( + m -> + m.getLabelList().stream() + .anyMatch( + l -> l.getName().equals("outcome") && l.getValue().equals("SUCCESS"))) + .count(); + + long failureCount = + family.getMetricList().stream() + .filter( + m -> + m.getLabelList().stream() + .anyMatch( + l -> l.getName().equals("outcome") && l.getValue().equals("FAILURE"))) + .count(); + + assertThat(successCount).isEqualTo(2); + assertThat(failureCount).isEqualTo(2); + } + + @Test + void testDifferentMetrics_producesSeparateMetricFamilies() throws IOException { + MetricSnapshots snapshots = getMetricSnapshots(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + PrometheusProtobufWriterImpl writer = new PrometheusProtobufWriterImpl(); + writer.write(out, snapshots, EscapingScheme.UNDERSCORE_ESCAPING); + + List metricFamilies = parseProtobufOutput(out); + + assertThat(metricFamilies).hasSize(2); + + Metrics.MetricFamily counterFamily = null; + Metrics.MetricFamily gaugeFamily = null; + for (Metrics.MetricFamily family : metricFamilies) { + if (family.getName().equals("http_requests_total")) { + counterFamily = family; + } else if (family.getName().equals("active_sessions")) { + gaugeFamily = family; + } + } + + assertThat(counterFamily).isNotNull(); + assertThat(counterFamily.getType()).isEqualTo(Metrics.MetricType.COUNTER); + assertThat(counterFamily.getMetricCount()).isEqualTo(1); + assertThat(counterFamily.getMetric(0).getCounter().getValue()).isEqualTo(100.0); + + assertThat(gaugeFamily).isNotNull(); + assertThat(gaugeFamily.getType()).isEqualTo(Metrics.MetricType.GAUGE); + assertThat(gaugeFamily.getMetricCount()).isEqualTo(1); + assertThat(gaugeFamily.getMetric(0).getGauge().getValue()).isEqualTo(50.0); + } + + private static MetricSnapshots getMetricSnapshots() { + PrometheusRegistry registry = new PrometheusRegistry(); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("http_requests") + .help("HTTP Request counter") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("method", "GET")) + .value(100) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "http_requests_total"; + } + }); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder() + .name("active_sessions") + .help("Active sessions gauge") + .dataPoint( + GaugeSnapshot.GaugeDataPointSnapshot.builder() + .labels(Labels.of("region", "us-east-1")) + .value(50) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "active_sessions"; + } + }); + + return registry.scrape(); + } + + private static List parseProtobufOutput(ByteArrayOutputStream out) + throws IOException { + List metricFamilies = new ArrayList<>(); + try (ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray())) { + Metrics.MetricFamily family; + while ((family = Metrics.MetricFamily.parseDelimitedFrom(in)) != null) { + metricFamilies.add(family); + } + } + return metricFamilies; + } +} \ No newline at end of file diff --git a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java index fb9d3f313..f24cd643b 100644 --- a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java +++ b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java @@ -1,14 +1,63 @@ package io.prometheus.metrics.expositionformats; import io.prometheus.metrics.config.EscapingScheme; +import io.prometheus.metrics.model.snapshots.CounterSnapshot; +import io.prometheus.metrics.model.snapshots.DataPointSnapshot; +import io.prometheus.metrics.model.snapshots.GaugeSnapshot; +import io.prometheus.metrics.model.snapshots.HistogramSnapshot; +import io.prometheus.metrics.model.snapshots.InfoSnapshot; import io.prometheus.metrics.model.snapshots.Labels; +import io.prometheus.metrics.model.snapshots.MetricSnapshot; +import io.prometheus.metrics.model.snapshots.MetricSnapshots; import io.prometheus.metrics.model.snapshots.PrometheusNaming; import io.prometheus.metrics.model.snapshots.SnapshotEscaper; +import io.prometheus.metrics.model.snapshots.StateSetSnapshot; +import io.prometheus.metrics.model.snapshots.SummarySnapshot; +import io.prometheus.metrics.model.snapshots.UnknownSnapshot; import java.io.IOException; import java.io.Writer; +import java.util.ArrayList; +import java.util.Collection; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; import javax.annotation.Nullable; +/** + * Utility methods for writing Prometheus text exposition formats. + * + *

This class provides low-level formatting utilities used by both Prometheus text format and + * OpenMetrics format writers. It handles escaping, label formatting, timestamp conversion, and + * merging of duplicate metric names. + */ public class TextFormatUtil { + /** + * Merges snapshots with duplicate Prometheus names by combining their data points. This ensures + * only one HELP/TYPE declaration per metric family. + */ + public static MetricSnapshots mergeDuplicates(MetricSnapshots metricSnapshots) { + Map> grouped = new LinkedHashMap<>(); + + // Group snapshots by Prometheus name + for (MetricSnapshot snapshot : metricSnapshots) { + String prometheusName = snapshot.getMetadata().getPrometheusName(); + grouped.computeIfAbsent(prometheusName, k -> new ArrayList<>()).add(snapshot); + } + + // Merge groups with multiple snapshots + MetricSnapshots.Builder builder = MetricSnapshots.builder(); + for (List group : grouped.values()) { + if (group.size() == 1) { + builder.metricSnapshot(group.get(0)); + } else { + // Merge multiple snapshots with same name + MetricSnapshot merged = mergeSnapshots(group); + builder.metricSnapshot(merged); + } + } + + return builder.build(); + } static void writeLong(Writer writer, long value) throws IOException { writer.append(Long.toString(value)); @@ -26,7 +75,7 @@ static void writeDouble(Writer writer, double d) throws IOException { } static void writePrometheusTimestamp(Writer writer, long timestampMs, boolean timestampsInMs) - throws IOException { + throws IOException { if (timestampsInMs) { // correct for prometheus exposition format // https://prometheus.io/docs/instrumenting/exposition_formats/#text-format-details @@ -103,13 +152,13 @@ static void writeEscapedString(Writer writer, String s) throws IOException { } static void writeLabels( - Writer writer, - Labels labels, - @Nullable String additionalLabelName, - double additionalLabelValue, - boolean metricInsideBraces, - EscapingScheme scheme) - throws IOException { + Writer writer, + Labels labels, + @Nullable String additionalLabelName, + double additionalLabelValue, + boolean metricInsideBraces, + EscapingScheme scheme) + throws IOException { if (!metricInsideBraces) { writer.write('{'); } @@ -155,4 +204,61 @@ static void writeName(Writer writer, String name, NameType nameType) throws IOEx writeEscapedString(writer, name); writer.write('"'); } -} + + /** + * Merges multiple snapshots of the same type into a single snapshot with combined data points. + */ + @SuppressWarnings("unchecked") + private static MetricSnapshot mergeSnapshots(List snapshots) { + MetricSnapshot first = snapshots.get(0); + + // Validate all snapshots are the same type + for (MetricSnapshot snapshot : snapshots) { + if (snapshot.getClass() != first.getClass()) { + throw new IllegalArgumentException( + "Cannot merge snapshots of different types: " + + first.getClass().getName() + + " and " + + snapshot.getClass().getName()); + } + } + + List allDataPoints = new ArrayList<>(); + for (MetricSnapshot snapshot : snapshots) { + allDataPoints.addAll(snapshot.getDataPoints()); + } + + // Create merged snapshot based on type + if (first instanceof CounterSnapshot) { + return new CounterSnapshot( + first.getMetadata(), + (Collection) (Object) allDataPoints); + } else if (first instanceof GaugeSnapshot) { + return new GaugeSnapshot( + first.getMetadata(), + (Collection) (Object) allDataPoints); + } else if (first instanceof HistogramSnapshot) { + return new HistogramSnapshot( + first.getMetadata(), + (Collection) (Object) allDataPoints); + } else if (first instanceof SummarySnapshot) { + return new SummarySnapshot( + first.getMetadata(), + (Collection) (Object) allDataPoints); + } else if (first instanceof InfoSnapshot) { + return new InfoSnapshot( + first.getMetadata(), + (Collection) (Object) allDataPoints); + } else if (first instanceof StateSetSnapshot) { + return new StateSetSnapshot( + first.getMetadata(), + (Collection) (Object) allDataPoints); + } else if (first instanceof UnknownSnapshot) { + return new UnknownSnapshot( + first.getMetadata(), + (Collection) (Object) allDataPoints); + } else { + throw new IllegalArgumentException("Unknown snapshot type: " + first.getClass().getName()); + } + } +} \ No newline at end of file diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 71e0f83bb..9964c4383 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -2,9 +2,14 @@ import static io.prometheus.metrics.model.snapshots.PrometheusNaming.prometheusName; +import io.prometheus.metrics.model.snapshots.DataPointSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; @@ -55,10 +60,6 @@ MetricType getType() { } public void register(Collector collector) { - if (collectors.contains(collector)) { - return; - } - String prometheusName = collector.getPrometheusName(); MetricType metricType = collector.getMetricType(); Set labelNames = collector.getLabelNames(); @@ -102,10 +103,6 @@ public void register(Collector collector) { } public void register(MultiCollector collector) { - if (multiCollectors.contains(collector)) { - return; - } - List names = collector.getPrometheusNames(); for (String prometheusName : names) { @@ -200,29 +197,29 @@ public MetricSnapshots scrape() { } public MetricSnapshots scrape(@Nullable PrometheusScrapeRequest scrapeRequest) { - MetricSnapshots.Builder result = MetricSnapshots.builder(); + List allSnapshots = new ArrayList<>(); for (Collector collector : collectors) { MetricSnapshot snapshot = scrapeRequest == null ? collector.collect() : collector.collect(scrapeRequest); if (snapshot != null) { - if (result.containsMetricName(snapshot.getMetadata().getName())) { - throw new IllegalStateException( - snapshot.getMetadata().getPrometheusName() + ": duplicate metric name."); - } - result.metricSnapshot(snapshot); + allSnapshots.add(snapshot); } } for (MultiCollector collector : multiCollectors) { MetricSnapshots snapshots = scrapeRequest == null ? collector.collect() : collector.collect(scrapeRequest); for (MetricSnapshot snapshot : snapshots) { - if (result.containsMetricName(snapshot.getMetadata().getName())) { - throw new IllegalStateException( - snapshot.getMetadata().getPrometheusName() + ": duplicate metric name."); - } - result.metricSnapshot(snapshot); + allSnapshots.add(snapshot); } } + + // Validate no duplicate label schemas for same metric name + validateNoDuplicateLabelSchemas(allSnapshots); + + MetricSnapshots.Builder result = MetricSnapshots.builder(); + for (MetricSnapshot snapshot : allSnapshots) { + result.metricSnapshot(snapshot); + } return result.build(); } @@ -280,4 +277,74 @@ public MetricSnapshots scrape( } return result.build(); } + + /** + * Validates that snapshots with the same metric name don't have identical label schemas. This + * prevents duplicate time series which would occur if two snapshots produce data points with + * identical label sets. + */ + private void validateNoDuplicateLabelSchemas(List snapshots) { + // Group snapshots by prometheus name + Map> snapshotsByName = new HashMap<>(); + for (MetricSnapshot snapshot : snapshots) { + String name = snapshot.getMetadata().getPrometheusName(); + snapshotsByName.computeIfAbsent(name, k -> new ArrayList<>()).add(snapshot); + } + + // For each group with the same name, check for duplicate label schemas + for (Map.Entry> entry : snapshotsByName.entrySet()) { + List group = entry.getValue(); + if (group.size() <= 1) { + continue; // No duplicates possible with only one snapshot + } + + // Extract label schemas from each snapshot + List> labelSchemas = new ArrayList<>(); + for (MetricSnapshot snapshot : group) { + Set labelSchema = extractLabelSchema(snapshot); + if (labelSchema != null) { + // Check if this label schema already exists + if (labelSchemas.contains(labelSchema)) { + throw new IllegalStateException( + snapshot.getMetadata().getPrometheusName() + + ": duplicate metric name with identical label schema " + + labelSchema); + } + labelSchemas.add(labelSchema); + } + } + } + } + + /** + * Extracts the label schema (set of label names) from a snapshot's data points. Returns null if + * the snapshot has no data points or if data points have inconsistent label schemas. + */ + private Set extractLabelSchema(MetricSnapshot snapshot) { + if (snapshot.getDataPoints().isEmpty()) { + return null; + } + + // Get label names from the first data point + DataPointSnapshot firstDataPoint = snapshot.getDataPoints().get(0); + Set labelNames = new HashSet<>(); + for (int i = 0; i < firstDataPoint.getLabels().size(); i++) { + labelNames.add(firstDataPoint.getLabels().getName(i)); + } + + // Verify all data points have the same label schema + for (DataPointSnapshot dataPoint : snapshot.getDataPoints()) { + Set currentLabelNames = new HashSet<>(); + for (int i = 0; i < dataPoint.getLabels().size(); i++) { + currentLabelNames.add(dataPoint.getLabels().getName(i)); + } + if (!currentLabelNames.equals(labelNames)) { + // Data points have inconsistent label schemas - this is unusual but valid + // We can't determine a single label schema, so return null + return null; + } + } + + return labelNames; + } } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java index ecee897e4..43e51417c 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java @@ -28,23 +28,10 @@ public MetricSnapshots(MetricSnapshot... snapshots) { * #builder()}. * * @param snapshots the constructor creates a sorted copy of snapshots. - * @throws IllegalArgumentException if snapshots contains duplicate metric names. To avoid - * duplicate metric names use {@link #builder()} and check {@link - * Builder#containsMetricName(String)} before calling {@link - * Builder#metricSnapshot(MetricSnapshot)}. */ public MetricSnapshots(Collection snapshots) { List list = new ArrayList<>(snapshots); list.sort(comparing(s -> s.getMetadata().getPrometheusName())); - for (int i = 0; i < snapshots.size() - 1; i++) { - if (list.get(i) - .getMetadata() - .getPrometheusName() - .equals(list.get(i + 1).getMetadata().getPrometheusName())) { - throw new IllegalArgumentException( - list.get(i).getMetadata().getPrometheusName() + ": duplicate metric name"); - } - } this.snapshots = unmodifiableList(list); } diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index 93e1c62ab..ce28a3f15 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -16,6 +16,8 @@ class PrometheusRegistryTest { + Collector noName = () -> GaugeSnapshot.builder().name("no_name_gauge").build(); + Collector counterA1 = new Collector() { @Override @@ -81,22 +83,6 @@ public List getPrometheusNames() { } }; - @Test - public void registerNoName() { - PrometheusRegistry registry = new PrometheusRegistry(); - // If the collector does not have a name at registration time, there is no conflict during - // registration. - Collector noName1 = () -> GaugeSnapshot.builder().name("no_name_gauge").build(); - Collector noName2 = () -> GaugeSnapshot.builder().name("no_name_gauge").build(); - registry.register(noName1); - registry.register(noName2); - // However, at scrape time the collector has to provide a metric name, and then we'll get a - // duplicate name error. - assertThatCode(registry::scrape) - .hasMessageContaining("duplicate") - .hasMessageContaining("no_name_gauge"); - } - @Test public void registerDuplicateName_withoutTypeInfo_allowedForBackwardCompatibility() { PrometheusRegistry registry = new PrometheusRegistry(); From 5acc4405f32c2c0e53506fe6393412ae1c12edaf Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Wed, 21 Jan 2026 14:52:40 -0500 Subject: [PATCH 03/23] add some additional validation checks Signed-off-by: Jay DeLuca --- .../DuplicateMetricsSample.java | 54 +-- .../it/exporter/test/DuplicateMetricsIT.java | 38 +- .../DuplicateNamesProtobufTest.java | 326 +++++++++--------- .../expositionformats/TextFormatUtil.java | 54 +-- .../model/registry/PrometheusRegistry.java | 15 +- .../model/snapshots/MetricSnapshots.java | 23 ++ .../registry/PrometheusRegistryTest.java | 9 +- 7 files changed, 276 insertions(+), 243 deletions(-) diff --git a/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java b/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java index 53d4fefdb..d8df37d01 100644 --- a/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java +++ b/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java @@ -23,57 +23,57 @@ private static void run(int port) throws IOException, InterruptedException { // Register multiple counters with the same Prometheus name "http_requests_total" // but different label sets Counter requestsSuccess = - Counter.builder() - .name("http_requests_total") - .help("Total HTTP requests by status") - .labelNames("status", "method") - .register(); + Counter.builder() + .name("http_requests_total") + .help("Total HTTP requests by status") + .labelNames("status", "method") + .register(); requestsSuccess.labelValues("success", "GET").inc(150); requestsSuccess.labelValues("success", "POST").inc(45); Counter requestsError = - Counter.builder() - .name("http_requests_total") - .help("Total HTTP requests by status") - .labelNames("status", "endpoint") - .register(); + Counter.builder() + .name("http_requests_total") + .help("Total HTTP requests by status") + .labelNames("status", "endpoint") + .register(); requestsError.labelValues("error", "/api").inc(5); requestsError.labelValues("error", "/health").inc(2); // Register multiple gauges with the same Prometheus name "active_connections" // but different label sets Gauge connectionsByRegion = - Gauge.builder() - .name("active_connections") - .help("Active connections") - .labelNames("region", "protocol") - .register(); + Gauge.builder() + .name("active_connections") + .help("Active connections") + .labelNames("region", "protocol") + .register(); connectionsByRegion.labelValues("us-east", "http").set(42); connectionsByRegion.labelValues("us-west", "http").set(38); connectionsByRegion.labelValues("eu-west", "https").set(55); Gauge connectionsByPool = - Gauge.builder() - .name("active_connections") - .help("Active connections") - .labelNames("pool", "type") - .register(); + Gauge.builder() + .name("active_connections") + .help("Active connections") + .labelNames("pool", "type") + .register(); connectionsByPool.labelValues("primary", "read").set(30); connectionsByPool.labelValues("replica", "write").set(10); // Also add a regular metric without duplicates for reference Counter uniqueMetric = - Counter.builder() - .name("unique_metric_total") - .help("A unique metric for reference") - .unit(Unit.BYTES) - .register(); + Counter.builder() + .name("unique_metric_total") + .help("A unique metric for reference") + .unit(Unit.BYTES) + .register(); uniqueMetric.inc(1024); HTTPServer server = HTTPServer.builder().port(port).buildAndStart(); System.out.println( - "DuplicateMetricsSample listening on http://localhost:" + server.getPort() + "/metrics"); + "DuplicateMetricsSample listening on http://localhost:" + server.getPort() + "/metrics"); Thread.currentThread().join(); // wait forever } @@ -86,4 +86,4 @@ private static int parsePortOrExit(String port) { } return 0; // this won't happen } -} \ No newline at end of file +} diff --git a/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java b/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java index 0d1688b2d..f2047b9ca 100644 --- a/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java +++ b/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java @@ -37,10 +37,10 @@ void testDuplicateMetricsInPrometheusTextFormat() throws IOException { Response response = scrape("GET", ""); assertThat(response.status).isEqualTo(200); assertContentType( - "text/plain; version=0.0.4; charset=utf-8", response.getHeader("Content-Type")); + "text/plain; version=0.0.4; charset=utf-8", response.getHeader("Content-Type")); String expected = - """ + """ # HELP active_connections Active connections # TYPE active_connections gauge active_connections{pool="primary",type="read"} 30.0 @@ -66,15 +66,15 @@ void testDuplicateMetricsInPrometheusTextFormat() throws IOException { void testDuplicateMetricsInOpenMetricsTextFormat() throws IOException { start(); Response response = - scrape("GET", "", "Accept", "application/openmetrics-text; version=1.0.0; charset=utf-8"); + scrape("GET", "", "Accept", "application/openmetrics-text; version=1.0.0; charset=utf-8"); assertThat(response.status).isEqualTo(200); assertContentType( - "application/openmetrics-text; version=1.0.0; charset=utf-8", - response.getHeader("Content-Type")); + "application/openmetrics-text; version=1.0.0; charset=utf-8", + response.getHeader("Content-Type")); // OpenMetrics format should have UNIT for unique_metric_bytes (base name without _total) String expected = - """ + """ # TYPE active_connections gauge # HELP active_connections Active connections active_connections{pool="primary",type="read"} 30.0 @@ -102,17 +102,17 @@ void testDuplicateMetricsInOpenMetricsTextFormat() throws IOException { void testDuplicateMetricsInPrometheusProtobufFormat() throws IOException { start(); Response response = - scrape( - "GET", - "", - "Accept", - "application/vnd.google.protobuf; proto=io.prometheus.client.MetricFamily;" - + " encoding=delimited"); + scrape( + "GET", + "", + "Accept", + "application/vnd.google.protobuf; proto=io.prometheus.client.MetricFamily;" + + " encoding=delimited"); assertThat(response.status).isEqualTo(200); assertContentType( - "application/vnd.google.protobuf; proto=io.prometheus.client.MetricFamily;" - + " encoding=delimited", - response.getHeader("Content-Type")); + "application/vnd.google.protobuf; proto=io.prometheus.client.MetricFamily;" + + " encoding=delimited", + response.getHeader("Content-Type")); List metrics = response.protoBody(); @@ -181,8 +181,8 @@ void testDuplicateMetricsWithNameFilter() throws IOException { String body = response.stringBody(); assertThat(body) - .contains("http_requests_total{method=\"GET\",status=\"success\"} 150.0") - .contains("http_requests_total{endpoint=\"/api\",status=\"error\"} 5.0"); + .contains("http_requests_total{method=\"GET\",status=\"success\"} 150.0") + .contains("http_requests_total{endpoint=\"/api\",status=\"error\"} 5.0"); // Should NOT contain active_connections or unique_metric_total assertThat(body).doesNotContain("active_connections").doesNotContain("unique_metric_total"); @@ -190,10 +190,10 @@ void testDuplicateMetricsWithNameFilter() throws IOException { private boolean hasLabel(List labels, String name, String value) { return labels.stream() - .anyMatch(label -> label.getName().equals(name) && label.getValue().equals(value)); + .anyMatch(label -> label.getName().equals(name) && label.getValue().equals(value)); } private String nameParam() { return "name[]=" + "http_requests_total"; } -} \ No newline at end of file +} diff --git a/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java b/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java index 59d1e7905..00dd7c59f 100644 --- a/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java +++ b/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java @@ -25,47 +25,47 @@ private static PrometheusRegistry getPrometheusRegistry() { PrometheusRegistry registry = new PrometheusRegistry(); registry.register( - new Collector() { - @Override - public MetricSnapshot collect() { - return CounterSnapshot.builder() - .name("api_responses") - .help("API responses") - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) - .value(100) - .build()) - .build(); - } - - @Override - public String getPrometheusName() { - return "api_responses_total"; - } - }); + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); registry.register( - new Collector() { - @Override - public MetricSnapshot collect() { - return CounterSnapshot.builder() - .name("api_responses") - .help("API responses") - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels( - Labels.of("uri", "/hello", "outcome", "FAILURE", "error", "TIMEOUT")) - .value(10) - .build()) - .build(); - } - - @Override - public String getPrometheusName() { - return "api_responses_total"; - } - }); + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels( + Labels.of("uri", "/hello", "outcome", "FAILURE", "error", "TIMEOUT")) + .value(10) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); return registry; } @@ -88,29 +88,29 @@ void testDuplicateNames_differentLabels_producesSingleMetricFamily() throws IOEx assertThat(family.getMetricCount()).isEqualTo(2); Metrics.Metric successMetric = - family.getMetricList().stream() - .filter( - m -> - m.getLabelList().stream() - .anyMatch( - l -> l.getName().equals("outcome") && l.getValue().equals("SUCCESS"))) - .findFirst() - .orElseThrow(() -> new AssertionError("SUCCESS metric not found")); + family.getMetricList().stream() + .filter( + m -> + m.getLabelList().stream() + .anyMatch( + l -> l.getName().equals("outcome") && l.getValue().equals("SUCCESS"))) + .findFirst() + .orElseThrow(() -> new AssertionError("SUCCESS metric not found")); assertThat(successMetric.getCounter().getValue()).isEqualTo(100.0); Metrics.Metric failureMetric = - family.getMetricList().stream() - .filter( - m -> - m.getLabelList().stream() - .anyMatch( - l -> - l.getName().equals("outcome") && l.getValue().equals("FAILURE")) - && m.getLabelList().stream() - .anyMatch( - l -> l.getName().equals("error") && l.getValue().equals("TIMEOUT"))) - .findFirst() - .orElseThrow(() -> new AssertionError("FAILURE metric not found")); + family.getMetricList().stream() + .filter( + m -> + m.getLabelList().stream() + .anyMatch( + l -> + l.getName().equals("outcome") && l.getValue().equals("FAILURE")) + && m.getLabelList().stream() + .anyMatch( + l -> l.getName().equals("error") && l.getValue().equals("TIMEOUT"))) + .findFirst() + .orElseThrow(() -> new AssertionError("FAILURE metric not found")); assertThat(failureMetric.getCounter().getValue()).isEqualTo(10.0); } @@ -119,58 +119,58 @@ void testDuplicateNames_multipleDataPoints_producesSingleMetricFamily() throws I PrometheusRegistry registry = new PrometheusRegistry(); registry.register( - new Collector() { - @Override - public MetricSnapshot collect() { - return CounterSnapshot.builder() - .name("api_responses") - .help("API responses") - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) - .value(100) - .build()) - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("uri", "/world", "outcome", "SUCCESS")) - .value(200) - .build()) - .build(); - } - - @Override - public String getPrometheusName() { - return "api_responses_total"; - } - }); + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .build()) + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/world", "outcome", "SUCCESS")) + .value(200) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); registry.register( - new Collector() { - @Override - public MetricSnapshot collect() { - return CounterSnapshot.builder() - .name("api_responses") - .help("API responses") - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels( - Labels.of("uri", "/hello", "outcome", "FAILURE", "error", "TIMEOUT")) - .value(10) - .build()) - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels( - Labels.of("uri", "/world", "outcome", "FAILURE", "error", "NOT_FOUND")) - .value(5) - .build()) - .build(); - } - - @Override - public String getPrometheusName() { - return "api_responses_total"; - } - }); + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels( + Labels.of("uri", "/hello", "outcome", "FAILURE", "error", "TIMEOUT")) + .value(10) + .build()) + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels( + Labels.of("uri", "/world", "outcome", "FAILURE", "error", "NOT_FOUND")) + .value(5) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); MetricSnapshots snapshots = registry.scrape(); ByteArrayOutputStream out = new ByteArrayOutputStream(); @@ -185,22 +185,22 @@ public String getPrometheusName() { assertThat(family.getMetricCount()).isEqualTo(4); long successCount = - family.getMetricList().stream() - .filter( - m -> - m.getLabelList().stream() - .anyMatch( - l -> l.getName().equals("outcome") && l.getValue().equals("SUCCESS"))) - .count(); + family.getMetricList().stream() + .filter( + m -> + m.getLabelList().stream() + .anyMatch( + l -> l.getName().equals("outcome") && l.getValue().equals("SUCCESS"))) + .count(); long failureCount = - family.getMetricList().stream() - .filter( - m -> - m.getLabelList().stream() - .anyMatch( - l -> l.getName().equals("outcome") && l.getValue().equals("FAILURE"))) - .count(); + family.getMetricList().stream() + .filter( + m -> + m.getLabelList().stream() + .anyMatch( + l -> l.getName().equals("outcome") && l.getValue().equals("FAILURE"))) + .count(); assertThat(successCount).isEqualTo(2); assertThat(failureCount).isEqualTo(2); @@ -242,52 +242,52 @@ private static MetricSnapshots getMetricSnapshots() { PrometheusRegistry registry = new PrometheusRegistry(); registry.register( - new Collector() { - @Override - public MetricSnapshot collect() { - return CounterSnapshot.builder() - .name("http_requests") - .help("HTTP Request counter") - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("method", "GET")) - .value(100) - .build()) - .build(); - } - - @Override - public String getPrometheusName() { - return "http_requests_total"; - } - }); + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("http_requests") + .help("HTTP Request counter") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("method", "GET")) + .value(100) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "http_requests_total"; + } + }); registry.register( - new Collector() { - @Override - public MetricSnapshot collect() { - return GaugeSnapshot.builder() - .name("active_sessions") - .help("Active sessions gauge") - .dataPoint( - GaugeSnapshot.GaugeDataPointSnapshot.builder() - .labels(Labels.of("region", "us-east-1")) - .value(50) - .build()) - .build(); - } - - @Override - public String getPrometheusName() { - return "active_sessions"; - } - }); + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder() + .name("active_sessions") + .help("Active sessions gauge") + .dataPoint( + GaugeSnapshot.GaugeDataPointSnapshot.builder() + .labels(Labels.of("region", "us-east-1")) + .value(50) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "active_sessions"; + } + }); return registry.scrape(); } private static List parseProtobufOutput(ByteArrayOutputStream out) - throws IOException { + throws IOException { List metricFamilies = new ArrayList<>(); try (ByteArrayInputStream in = new ByteArrayInputStream(out.toByteArray())) { Metrics.MetricFamily family; @@ -297,4 +297,4 @@ private static List parseProtobufOutput(ByteArrayOutputStr } return metricFamilies; } -} \ No newline at end of file +} diff --git a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java index f24cd643b..d54de3d57 100644 --- a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java +++ b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java @@ -75,7 +75,7 @@ static void writeDouble(Writer writer, double d) throws IOException { } static void writePrometheusTimestamp(Writer writer, long timestampMs, boolean timestampsInMs) - throws IOException { + throws IOException { if (timestampsInMs) { // correct for prometheus exposition format // https://prometheus.io/docs/instrumenting/exposition_formats/#text-format-details @@ -152,13 +152,13 @@ static void writeEscapedString(Writer writer, String s) throws IOException { } static void writeLabels( - Writer writer, - Labels labels, - @Nullable String additionalLabelName, - double additionalLabelValue, - boolean metricInsideBraces, - EscapingScheme scheme) - throws IOException { + Writer writer, + Labels labels, + @Nullable String additionalLabelName, + double additionalLabelValue, + boolean metricInsideBraces, + EscapingScheme scheme) + throws IOException { if (!metricInsideBraces) { writer.write('{'); } @@ -216,10 +216,10 @@ private static MetricSnapshot mergeSnapshots(List snapshots) { for (MetricSnapshot snapshot : snapshots) { if (snapshot.getClass() != first.getClass()) { throw new IllegalArgumentException( - "Cannot merge snapshots of different types: " - + first.getClass().getName() - + " and " - + snapshot.getClass().getName()); + "Cannot merge snapshots of different types: " + + first.getClass().getName() + + " and " + + snapshot.getClass().getName()); } } @@ -231,34 +231,34 @@ private static MetricSnapshot mergeSnapshots(List snapshots) { // Create merged snapshot based on type if (first instanceof CounterSnapshot) { return new CounterSnapshot( - first.getMetadata(), - (Collection) (Object) allDataPoints); + first.getMetadata(), + (Collection) (Object) allDataPoints); } else if (first instanceof GaugeSnapshot) { return new GaugeSnapshot( - first.getMetadata(), - (Collection) (Object) allDataPoints); + first.getMetadata(), + (Collection) (Object) allDataPoints); } else if (first instanceof HistogramSnapshot) { return new HistogramSnapshot( - first.getMetadata(), - (Collection) (Object) allDataPoints); + first.getMetadata(), + (Collection) (Object) allDataPoints); } else if (first instanceof SummarySnapshot) { return new SummarySnapshot( - first.getMetadata(), - (Collection) (Object) allDataPoints); + first.getMetadata(), + (Collection) (Object) allDataPoints); } else if (first instanceof InfoSnapshot) { return new InfoSnapshot( - first.getMetadata(), - (Collection) (Object) allDataPoints); + first.getMetadata(), + (Collection) (Object) allDataPoints); } else if (first instanceof StateSetSnapshot) { return new StateSetSnapshot( - first.getMetadata(), - (Collection) (Object) allDataPoints); + first.getMetadata(), + (Collection) (Object) allDataPoints); } else if (first instanceof UnknownSnapshot) { return new UnknownSnapshot( - first.getMetadata(), - (Collection) (Object) allDataPoints); + first.getMetadata(), + (Collection) (Object) allDataPoints); } else { throw new IllegalArgumentException("Unknown snapshot type: " + first.getClass().getName()); } } -} \ No newline at end of file +} diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 9964c4383..578230cb7 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -60,6 +60,11 @@ MetricType getType() { } public void register(Collector collector) { + // Check if this exact instance is already registered + if (collectors.contains(collector)) { + throw new IllegalArgumentException("Collector instance is already registered"); + } + String prometheusName = collector.getPrometheusName(); MetricType metricType = collector.getMetricType(); Set labelNames = collector.getLabelNames(); @@ -103,6 +108,11 @@ public void register(Collector collector) { } public void register(MultiCollector collector) { + // Check if this exact instance is already registered + if (multiCollectors.contains(collector)) { + throw new IllegalArgumentException("MultiCollector instance is already registered"); + } + List names = collector.getPrometheusNames(); for (String prometheusName : names) { @@ -284,7 +294,6 @@ public MetricSnapshots scrape( * identical label sets. */ private void validateNoDuplicateLabelSchemas(List snapshots) { - // Group snapshots by prometheus name Map> snapshotsByName = new HashMap<>(); for (MetricSnapshot snapshot : snapshots) { String name = snapshot.getMetadata().getPrometheusName(); @@ -295,7 +304,7 @@ private void validateNoDuplicateLabelSchemas(List snapshots) { for (Map.Entry> entry : snapshotsByName.entrySet()) { List group = entry.getValue(); if (group.size() <= 1) { - continue; // No duplicates possible with only one snapshot + continue; } // Extract label schemas from each snapshot @@ -303,7 +312,6 @@ private void validateNoDuplicateLabelSchemas(List snapshots) { for (MetricSnapshot snapshot : group) { Set labelSchema = extractLabelSchema(snapshot); if (labelSchema != null) { - // Check if this label schema already exists if (labelSchemas.contains(labelSchema)) { throw new IllegalStateException( snapshot.getMetadata().getPrometheusName() @@ -320,6 +328,7 @@ private void validateNoDuplicateLabelSchemas(List snapshots) { * Extracts the label schema (set of label names) from a snapshot's data points. Returns null if * the snapshot has no data points or if data points have inconsistent label schemas. */ + @Nullable private Set extractLabelSchema(MetricSnapshot snapshot) { if (snapshot.getDataPoints().isEmpty()) { return null; diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java index 43e51417c..091e9cd65 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java @@ -28,10 +28,33 @@ public MetricSnapshots(MetricSnapshot... snapshots) { * #builder()}. * * @param snapshots the constructor creates a sorted copy of snapshots. + * @throws IllegalArgumentException if snapshots contain conflicting metric types (same name but + * different metric types like Counter vs Gauge). */ public MetricSnapshots(Collection snapshots) { List list = new ArrayList<>(snapshots); list.sort(comparing(s -> s.getMetadata().getPrometheusName())); + + // Validate no conflicting metric types + for (int i = 0; i < list.size() - 1; i++) { + String name1 = list.get(i).getMetadata().getPrometheusName(); + String name2 = list.get(i + 1).getMetadata().getPrometheusName(); + + if (name1.equals(name2)) { + Class type1 = list.get(i).getClass(); + Class type2 = list.get(i + 1).getClass(); + + if (!type1.equals(type2)) { + throw new IllegalArgumentException( + name1 + + ": conflicting metric types: " + + type1.getSimpleName() + + " and " + + type2.getSimpleName()); + } + } + } + this.snapshots = unmodifiableList(list); } diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index ce28a3f15..9f791e691 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -357,12 +357,13 @@ public void registerOk() { } @Test - public void registerDuplicateMultiCollector_withoutTypeInfo_allowedForBackwardCompatibility() { + public void registerDuplicateMultiCollector_notAllowed() { PrometheusRegistry registry = new PrometheusRegistry(); - // multiCollector doesn't provide type/label info, so validation is skipped registry.register(multiCollector); - // This now succeeds for backward compatibility (validation skipped when type is null) - assertThatCode(() -> registry.register(multiCollector)).doesNotThrowAnyException(); + // Registering the same instance twice should fail + assertThatThrownBy(() -> registry.register(multiCollector)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("already registered"); } @Test From c8eea69c1cc4ea9225d5dd3582a527b28176a7e1 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Wed, 21 Jan 2026 15:14:56 -0500 Subject: [PATCH 04/23] optimize merge method Signed-off-by: Jay DeLuca --- .../expositionformats/TextFormatUtil.java | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java index d54de3d57..27f830f07 100644 --- a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java +++ b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java @@ -36,21 +36,27 @@ public class TextFormatUtil { * only one HELP/TYPE declaration per metric family. */ public static MetricSnapshots mergeDuplicates(MetricSnapshots metricSnapshots) { + if (metricSnapshots.size() <= 1) { + return metricSnapshots; + } + Map> grouped = new LinkedHashMap<>(); - // Group snapshots by Prometheus name for (MetricSnapshot snapshot : metricSnapshots) { String prometheusName = snapshot.getMetadata().getPrometheusName(); - grouped.computeIfAbsent(prometheusName, k -> new ArrayList<>()).add(snapshot); + List list = grouped.get(prometheusName); + if (list == null) { + list = new ArrayList<>(); + grouped.put(prometheusName, list); + } + list.add(snapshot); } - // Merge groups with multiple snapshots MetricSnapshots.Builder builder = MetricSnapshots.builder(); for (List group : grouped.values()) { if (group.size() == 1) { builder.metricSnapshot(group.get(0)); } else { - // Merge multiple snapshots with same name MetricSnapshot merged = mergeSnapshots(group); builder.metricSnapshot(merged); } @@ -212,7 +218,8 @@ static void writeName(Writer writer, String name, NameType nameType) throws IOEx private static MetricSnapshot mergeSnapshots(List snapshots) { MetricSnapshot first = snapshots.get(0); - // Validate all snapshots are the same type + // Validate all snapshots are the same type and calculate total size + int totalDataPoints = 0; for (MetricSnapshot snapshot : snapshots) { if (snapshot.getClass() != first.getClass()) { throw new IllegalArgumentException( @@ -221,9 +228,11 @@ private static MetricSnapshot mergeSnapshots(List snapshots) { + " and " + snapshot.getClass().getName()); } + totalDataPoints += snapshot.getDataPoints().size(); } - List allDataPoints = new ArrayList<>(); + // Pre-size the list to avoid resizing + List allDataPoints = new ArrayList<>(totalDataPoints); for (MetricSnapshot snapshot : snapshots) { allDataPoints.addAll(snapshot.getDataPoints()); } From 43ab4b146506ce76c9e400ed5f0e7b4159e8ba52 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Wed, 21 Jan 2026 16:23:59 -0500 Subject: [PATCH 05/23] handle other exposition formats Signed-off-by: Jay DeLuca --- .../DuplicateMetricsSample.java | 10 +- .../it/exporter/test/DuplicateMetricsIT.java | 5 - .../generate-protobuf.sh | 34 ++- .../OpenMetricsTextFormatWriter.java | 3 +- .../PrometheusTextFormatWriter.java | 3 +- .../DuplicateNamesExpositionTest.java | 237 ++++++++++++++++++ .../expositionformats/TextFormatUtilTest.java | 93 +++++++ .../model/registry/PrometheusRegistry.java | 6 - 8 files changed, 367 insertions(+), 24 deletions(-) create mode 100644 prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java diff --git a/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java b/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java index d8df37d01..c6005674a 100644 --- a/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java +++ b/integration-tests/it-exporter/it-exporter-duplicate-metrics-sample/src/main/java/io/prometheus/metrics/it/exporter/duplicatemetrics/DuplicateMetricsSample.java @@ -10,16 +10,18 @@ public class DuplicateMetricsSample { public static void main(String[] args) throws IOException, InterruptedException { - if (args.length != 1) { - System.err.println("Usage: java -jar duplicate-metrics-sample.jar "); + if (args.length != 2) { + System.err.println("Usage: java -jar duplicate-metrics-sample.jar "); + System.err.println("Where outcome is \"success\" or \"error\"."); System.exit(1); } int port = parsePortOrExit(args[0]); - run(port); + String outcome = args[1]; + run(port, outcome); } - private static void run(int port) throws IOException, InterruptedException { + private static void run(int port, String outcome) throws IOException, InterruptedException { // Register multiple counters with the same Prometheus name "http_requests_total" // but different label sets Counter requestsSuccess = diff --git a/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java b/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java index f2047b9ca..67e07ba75 100644 --- a/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java +++ b/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java @@ -26,11 +26,6 @@ public DuplicateMetricsIT() throws IOException, URISyntaxException { super("exporter-duplicate-metrics-sample"); } - @Override - protected void start(String outcome) { - sampleAppContainer.withCommand("java", "-jar", "/app/" + sampleApp + ".jar", "9400").start(); - } - @Test void testDuplicateMetricsInPrometheusTextFormat() throws IOException { start(); diff --git a/prometheus-metrics-exposition-formats/generate-protobuf.sh b/prometheus-metrics-exposition-formats/generate-protobuf.sh index 9bd222241..7f14d703e 100755 --- a/prometheus-metrics-exposition-formats/generate-protobuf.sh +++ b/prometheus-metrics-exposition-formats/generate-protobuf.sh @@ -6,6 +6,26 @@ set -euo pipefail # I could not figure out how to use a protoc Maven plugin to use the shaded module, # so I ran this command to generate the sources manually. +# Use gsed and ggrep on macOS (requires: brew install gnu-sed grep) +if [[ "$OSTYPE" == "darwin"* ]] && command -v gsed >/dev/null 2>&1; then + SED=gsed +else + SED=sed +fi + +if [[ "$OSTYPE" == "darwin"* ]] && command -v ggrep >/dev/null 2>&1; then + GREP=ggrep +else + GREP=grep +fi + +# Use mise-provided protoc if available +if command -v mise >/dev/null 2>&1; then + PROTOC="mise exec -- protoc" +else + PROTOC=protoc +fi + TARGET_DIR=$1 PROTO_DIR=src/main/protobuf PROTOBUF_VERSION_STRING=$2 @@ -18,22 +38,22 @@ mkdir -p "$TARGET_DIR" rm -rf $PROTO_DIR || true mkdir -p $PROTO_DIR -OLD_PACKAGE=$(sed -nE 's/import (io.prometheus.metrics.expositionformats.generated.*).Metrics;/\1/p' src/main/java/io/prometheus/metrics/expositionformats/internal/PrometheusProtobufWriterImpl.java) +OLD_PACKAGE=$($SED -nE 's/import (io.prometheus.metrics.expositionformats.generated.*).Metrics;/\1/p' src/main/java/io/prometheus/metrics/expositionformats/internal/PrometheusProtobufWriterImpl.java) PACKAGE="io.prometheus.metrics.expositionformats.generated.com_google_protobuf_${PROTOBUF_VERSION_STRING}" if [[ $OLD_PACKAGE != "$PACKAGE" ]]; then echo "Replacing package $OLD_PACKAGE with $PACKAGE in all java files" - find .. -type f -name "*.java" -exec sed -i "s/$OLD_PACKAGE/$PACKAGE/g" {} + + find .. -type f -name "*.java" -exec $SED -i "s/$OLD_PACKAGE/$PACKAGE/g" {} + fi curl -sL https://raw.githubusercontent.com/prometheus/client_model/master/io/prometheus/client/metrics.proto -o $PROTO_DIR/metrics.proto -sed -i "s/java_package = \"io.prometheus.client\"/java_package = \"$PACKAGE\"/" $PROTO_DIR/metrics.proto -protoc --java_out "$TARGET_DIR" $PROTO_DIR/metrics.proto -sed -i '1 i\//CHECKSTYLE:OFF: checkstyle' "$(find src/main/generated/io -type f)" -sed -i -e $'$a\\\n//CHECKSTYLE:ON: checkstyle' "$(find src/main/generated/io -type f)" +$SED -i "s/java_package = \"io.prometheus.client\"/java_package = \"$PACKAGE\"/" $PROTO_DIR/metrics.proto +$PROTOC --java_out "$TARGET_DIR" $PROTO_DIR/metrics.proto +$SED -i '1 i\//CHECKSTYLE:OFF: checkstyle' "$(find src/main/generated/io -type f)" +$SED -i -e $'$a\\\n//CHECKSTYLE:ON: checkstyle' "$(find src/main/generated/io -type f)" -GENERATED_WITH=$(grep -oP '\/\/ Protobuf Java Version: \K.*' "$TARGET_DIR/${PACKAGE//\.//}"/Metrics.java) +GENERATED_WITH=$($GREP -oP '\/\/ Protobuf Java Version: \K.*' "$TARGET_DIR/${PACKAGE//\.//}"/Metrics.java) function help() { echo "Please use https://mise.jdx.dev/ - this will use the version specified in mise.toml" diff --git a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/OpenMetricsTextFormatWriter.java b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/OpenMetricsTextFormatWriter.java index 1ba1c627d..293fbfb8c 100644 --- a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/OpenMetricsTextFormatWriter.java +++ b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/OpenMetricsTextFormatWriter.java @@ -113,7 +113,8 @@ public String getContentType() { public void write(OutputStream out, MetricSnapshots metricSnapshots, EscapingScheme scheme) throws IOException { Writer writer = new BufferedWriter(new OutputStreamWriter(out, StandardCharsets.UTF_8)); - for (MetricSnapshot s : metricSnapshots) { + MetricSnapshots merged = TextFormatUtil.mergeDuplicates(metricSnapshots); + for (MetricSnapshot s : merged) { MetricSnapshot snapshot = SnapshotEscaper.escapeMetricSnapshot(s, scheme); if (!snapshot.getDataPoints().isEmpty()) { if (snapshot instanceof CounterSnapshot) { diff --git a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/PrometheusTextFormatWriter.java b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/PrometheusTextFormatWriter.java index 73d33504e..861ebcf75 100644 --- a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/PrometheusTextFormatWriter.java +++ b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/PrometheusTextFormatWriter.java @@ -115,7 +115,8 @@ public void write(OutputStream out, MetricSnapshots metricSnapshots, EscapingSch // "unknown", "gauge", "counter", "stateset", "info", "histogram", "gaugehistogram", and // "summary". Writer writer = new BufferedWriter(new OutputStreamWriter(out, StandardCharsets.UTF_8)); - for (MetricSnapshot s : metricSnapshots) { + MetricSnapshots merged = TextFormatUtil.mergeDuplicates(metricSnapshots); + for (MetricSnapshot s : merged) { MetricSnapshot snapshot = escapeMetricSnapshot(s, scheme); if (!snapshot.getDataPoints().isEmpty()) { if (snapshot instanceof CounterSnapshot) { diff --git a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java new file mode 100644 index 000000000..e13355079 --- /dev/null +++ b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java @@ -0,0 +1,237 @@ +package io.prometheus.metrics.expositionformats; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import io.prometheus.metrics.model.registry.Collector; +import io.prometheus.metrics.model.registry.PrometheusRegistry; +import io.prometheus.metrics.model.snapshots.CounterSnapshot; +import io.prometheus.metrics.model.snapshots.Labels; +import io.prometheus.metrics.model.snapshots.MetricSnapshot; +import io.prometheus.metrics.model.snapshots.MetricSnapshots; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import org.junit.jupiter.api.Test; + +class DuplicateNamesExpositionTest { + + private static PrometheusRegistry getPrometheusRegistry() { + PrometheusRegistry registry = new PrometheusRegistry(); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels( + Labels.of("uri", "/hello", "outcome", "FAILURE", "error", "TIMEOUT")) + .value(10) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + return registry; + } + + @Test + void testDuplicateNames_differentLabels_producesValidOutput() throws IOException { + PrometheusRegistry registry = getPrometheusRegistry(); + + MetricSnapshots snapshots = registry.scrape(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + PrometheusTextFormatWriter writer = PrometheusTextFormatWriter.create(); + writer.write(out, snapshots); + String output = out.toString(UTF_8); + + String expected = + """ + # HELP api_responses_total API responses + # TYPE api_responses_total counter + api_responses_total{error="TIMEOUT",outcome="FAILURE",uri="/hello"} 10.0 + api_responses_total{outcome="SUCCESS",uri="/hello"} 100.0 + """; + + assertThat(output).isEqualTo(expected); + } + + @Test + void testDuplicateNames_multipleDataPoints_producesValidOutput() throws IOException { + PrometheusRegistry registry = new PrometheusRegistry(); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .build()) + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/world", "outcome", "SUCCESS")) + .value(200) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels( + Labels.of("uri", "/hello", "outcome", "FAILURE", "error", "TIMEOUT")) + .value(10) + .build()) + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels( + Labels.of("uri", "/world", "outcome", "FAILURE", "error", "NOT_FOUND")) + .value(5) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + + MetricSnapshots snapshots = registry.scrape(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + PrometheusTextFormatWriter writer = PrometheusTextFormatWriter.create(); + writer.write(out, snapshots); + String output = out.toString(UTF_8); + + String expected = + """ + # HELP api_responses_total API responses + # TYPE api_responses_total counter + api_responses_total{error="NOT_FOUND",outcome="FAILURE",uri="/world"} 5.0 + api_responses_total{error="TIMEOUT",outcome="FAILURE",uri="/hello"} 10.0 + api_responses_total{outcome="SUCCESS",uri="/hello"} 100.0 + api_responses_total{outcome="SUCCESS",uri="/world"} 200.0 + """; + assertThat(output).isEqualTo(expected); + } + + @Test + void testOpenMetricsFormat_withDuplicateNames() throws IOException { + PrometheusRegistry registry = getPrometheusRegistry(); + + MetricSnapshots snapshots = registry.scrape(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + OpenMetricsTextFormatWriter writer = new OpenMetricsTextFormatWriter(false, false); + writer.write(out, snapshots); + String output = out.toString(UTF_8); + + String expected = + """ + # TYPE api_responses counter + # HELP api_responses API responses + api_responses_total{error="TIMEOUT",outcome="FAILURE",uri="/hello"} 10.0 + api_responses_total{outcome="SUCCESS",uri="/hello"} 100.0 + # EOF + """; + assertThat(output).isEqualTo(expected); + } + + @Test + void testDuplicateNames_sameLabels_throwsException() { + PrometheusRegistry registry = new PrometheusRegistry(); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(50) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + + // Scrape should throw exception due to duplicate time series (same name + same labels) + assertThatThrownBy(registry::scrape) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("duplicate metric name with identical label schema [uri, outcome]") + .hasMessageContaining("api_responses"); + } +} diff --git a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java index dbb707f51..b720a753c 100644 --- a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java +++ b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java @@ -5,6 +5,10 @@ import java.io.IOException; import java.io.StringWriter; + +import io.prometheus.metrics.model.snapshots.CounterSnapshot; +import io.prometheus.metrics.model.snapshots.Labels; +import io.prometheus.metrics.model.snapshots.MetricSnapshots; import org.junit.jupiter.api.Test; class TextFormatUtilTest { @@ -34,4 +38,93 @@ private static String writePrometheusTimestamp(boolean timestampsInMs) throws IO TextFormatUtil.writePrometheusTimestamp(writer, 1000, timestampsInMs); return writer.toString(); } + + @Test + public void testMergeDuplicates_sameName_mergesDataPoints() { + CounterSnapshot counter1 = + CounterSnapshot.builder() + .name("api_responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .build()) + .build(); + + CounterSnapshot counter2 = + CounterSnapshot.builder() + .name("api_responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "FAILURE")) + .value(10) + .build()) + .build(); + + MetricSnapshots snapshots = new MetricSnapshots(counter1, counter2); + MetricSnapshots result = TextFormatUtil.mergeDuplicates(snapshots); + + assertThat(result).hasSize(1); + assertThat(result.get(0).getMetadata().getName()).isEqualTo("api_responses"); + assertThat(result.get(0).getDataPoints()).hasSize(2); + + CounterSnapshot merged = (CounterSnapshot) result.get(0); + assertThat(merged.getDataPoints()) + .anyMatch( + dp -> + dp.getLabels().equals(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + && dp.getValue() == 100); + assertThat(merged.getDataPoints()) + .anyMatch( + dp -> + dp.getLabels().equals(Labels.of("uri", "/hello", "outcome", "FAILURE")) + && dp.getValue() == 10); + } + + @Test + public void testMergeDuplicates_multipleDataPoints_allMerged() { + CounterSnapshot counter1 = + CounterSnapshot.builder() + .name("api_responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .build()) + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/world", "outcome", "SUCCESS")) + .value(200) + .build()) + .build(); + + CounterSnapshot counter2 = + CounterSnapshot.builder() + .name("api_responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "FAILURE")) + .value(10) + .build()) + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/world", "outcome", "FAILURE")) + .value(5) + .build()) + .build(); + + MetricSnapshots snapshots = new MetricSnapshots(counter1, counter2); + MetricSnapshots result = TextFormatUtil.mergeDuplicates(snapshots); + + assertThat(result).hasSize(1); + assertThat(result.get(0).getDataPoints()).hasSize(4); + } + + @Test + public void testMergeDuplicates_emptySnapshots_returnsEmpty() { + MetricSnapshots snapshots = MetricSnapshots.builder().build(); + MetricSnapshots result = TextFormatUtil.mergeDuplicates(snapshots); + + assertThat(result).isEmpty(); + } } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 578230cb7..19db7e67b 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -1,7 +1,5 @@ package io.prometheus.metrics.model.registry; -import static io.prometheus.metrics.model.snapshots.PrometheusNaming.prometheusName; - import io.prometheus.metrics.model.snapshots.DataPointSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; @@ -60,7 +58,6 @@ MetricType getType() { } public void register(Collector collector) { - // Check if this exact instance is already registered if (collectors.contains(collector)) { throw new IllegalArgumentException("Collector instance is already registered"); } @@ -108,7 +105,6 @@ public void register(Collector collector) { } public void register(MultiCollector collector) { - // Check if this exact instance is already registered if (multiCollectors.contains(collector)) { throw new IllegalArgumentException("MultiCollector instance is already registered"); } @@ -223,7 +219,6 @@ public MetricSnapshots scrape(@Nullable PrometheusScrapeRequest scrapeRequest) { } } - // Validate no duplicate label schemas for same metric name validateNoDuplicateLabelSchemas(allSnapshots); MetricSnapshots.Builder result = MetricSnapshots.builder(); @@ -341,7 +336,6 @@ private Set extractLabelSchema(MetricSnapshot snapshot) { labelNames.add(firstDataPoint.getLabels().getName(i)); } - // Verify all data points have the same label schema for (DataPointSnapshot dataPoint : snapshot.getDataPoints()) { Set currentLabelNames = new HashSet<>(); for (int i = 0; i < dataPoint.getLabels().size(); i++) { From 9289e4acb90d06deda3825344a4effafdb45cd31 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Sat, 24 Jan 2026 10:57:29 -0500 Subject: [PATCH 06/23] cleanups, ironing out edge cases Signed-off-by: Jay DeLuca --- .../it/exporter/test/DuplicateMetricsIT.java | 13 - .../core/metrics/MetricWithFixedMetadata.java | 5 +- .../metrics/core/metrics/CounterTest.java | 14 + .../expositionformats/TextFormatUtil.java | 3 - .../expositionformats/TextFormatUtilTest.java | 105 ++++--- .../model/registry/PrometheusRegistry.java | 124 +++++--- .../registry/PrometheusRegistryTest.java | 264 +++++++++++++++++- 7 files changed, 408 insertions(+), 120 deletions(-) diff --git a/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java b/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java index 67e07ba75..3979d7330 100644 --- a/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java +++ b/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java @@ -9,17 +9,6 @@ import java.util.List; import org.junit.jupiter.api.Test; -/** - * Integration test for duplicate metric names with different label sets. - * - *

This test validates that: - * - *

    - *
  • Multiple metrics with the same Prometheus name but different labels can be registered - *
  • All exposition formats (text, OpenMetrics, protobuf) correctly merge and expose them - *
  • The merged output is valid and scrapeable by Prometheus - *
- */ class DuplicateMetricsIT extends ExporterTest { public DuplicateMetricsIT() throws IOException, URISyntaxException { @@ -111,8 +100,6 @@ void testDuplicateMetricsInPrometheusProtobufFormat() throws IOException { List metrics = response.protoBody(); - // Should have exactly 3 metric families (active_connections, http_requests_total, - // unique_metric_bytes_total) assertThat(metrics).hasSize(3); // Metrics are sorted by name diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java index a1c1f1576..a88ce642a 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java @@ -53,7 +53,10 @@ public String getPrometheusName() { @Override public Set getLabelNames() { - Set names = new HashSet<>(Arrays.asList(labelNames)); + Set names = new HashSet<>(); + for (String labelName : labelNames) { + names.add(PrometheusNaming.prometheusName(labelName)); + } for (Label label : constLabels) { names.add(PrometheusNaming.prometheusName(label.getName())); } diff --git a/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterTest.java b/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterTest.java index c39bf1ad6..d5fe1337b 100644 --- a/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterTest.java +++ b/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterTest.java @@ -12,6 +12,7 @@ import io.prometheus.metrics.expositionformats.generated.com_google_protobuf_4_33_4.Metrics; import io.prometheus.metrics.expositionformats.internal.PrometheusProtobufWriterImpl; import io.prometheus.metrics.expositionformats.internal.ProtobufUtil; +import io.prometheus.metrics.model.registry.PrometheusRegistry; import io.prometheus.metrics.model.snapshots.CounterSnapshot; import io.prometheus.metrics.model.snapshots.Exemplar; import io.prometheus.metrics.model.snapshots.Label; @@ -377,4 +378,17 @@ public void testConstLabelsSecond() { .constLabels(Labels.of("const_a", "const_b")) .build()); } + + @Test + public void testLabelNormalizationInRegistration() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Counter.builder().name("requests").labelNames("request.count").register(registry); + + // request.count and request_count normalize to the same name + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy( + () -> Counter.builder().name("requests").labelNames("request_count").register(registry)) + .withMessageContaining("duplicate metric name with identical label schema"); + } } diff --git a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java index 27f830f07..2dcaf9013 100644 --- a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java +++ b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java @@ -218,7 +218,6 @@ static void writeName(Writer writer, String name, NameType nameType) throws IOEx private static MetricSnapshot mergeSnapshots(List snapshots) { MetricSnapshot first = snapshots.get(0); - // Validate all snapshots are the same type and calculate total size int totalDataPoints = 0; for (MetricSnapshot snapshot : snapshots) { if (snapshot.getClass() != first.getClass()) { @@ -231,13 +230,11 @@ private static MetricSnapshot mergeSnapshots(List snapshots) { totalDataPoints += snapshot.getDataPoints().size(); } - // Pre-size the list to avoid resizing List allDataPoints = new ArrayList<>(totalDataPoints); for (MetricSnapshot snapshot : snapshots) { allDataPoints.addAll(snapshot.getDataPoints()); } - // Create merged snapshot based on type if (first instanceof CounterSnapshot) { return new CounterSnapshot( first.getMetadata(), diff --git a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java index b720a753c..e8571bac1 100644 --- a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java +++ b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java @@ -3,12 +3,11 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; -import java.io.IOException; -import java.io.StringWriter; - import io.prometheus.metrics.model.snapshots.CounterSnapshot; import io.prometheus.metrics.model.snapshots.Labels; import io.prometheus.metrics.model.snapshots.MetricSnapshots; +import java.io.IOException; +import java.io.StringWriter; import org.junit.jupiter.api.Test; class TextFormatUtilTest { @@ -42,24 +41,24 @@ private static String writePrometheusTimestamp(boolean timestampsInMs) throws IO @Test public void testMergeDuplicates_sameName_mergesDataPoints() { CounterSnapshot counter1 = - CounterSnapshot.builder() - .name("api_responses") - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) - .value(100) - .build()) - .build(); + CounterSnapshot.builder() + .name("api_responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .build()) + .build(); CounterSnapshot counter2 = - CounterSnapshot.builder() - .name("api_responses") - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("uri", "/hello", "outcome", "FAILURE")) - .value(10) - .build()) - .build(); + CounterSnapshot.builder() + .name("api_responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "FAILURE")) + .value(10) + .build()) + .build(); MetricSnapshots snapshots = new MetricSnapshots(counter1, counter2); MetricSnapshots result = TextFormatUtil.mergeDuplicates(snapshots); @@ -70,48 +69,48 @@ public void testMergeDuplicates_sameName_mergesDataPoints() { CounterSnapshot merged = (CounterSnapshot) result.get(0); assertThat(merged.getDataPoints()) - .anyMatch( - dp -> - dp.getLabels().equals(Labels.of("uri", "/hello", "outcome", "SUCCESS")) - && dp.getValue() == 100); + .anyMatch( + dp -> + dp.getLabels().equals(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + && dp.getValue() == 100); assertThat(merged.getDataPoints()) - .anyMatch( - dp -> - dp.getLabels().equals(Labels.of("uri", "/hello", "outcome", "FAILURE")) - && dp.getValue() == 10); + .anyMatch( + dp -> + dp.getLabels().equals(Labels.of("uri", "/hello", "outcome", "FAILURE")) + && dp.getValue() == 10); } @Test public void testMergeDuplicates_multipleDataPoints_allMerged() { CounterSnapshot counter1 = - CounterSnapshot.builder() - .name("api_responses") - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) - .value(100) - .build()) - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("uri", "/world", "outcome", "SUCCESS")) - .value(200) - .build()) - .build(); + CounterSnapshot.builder() + .name("api_responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .build()) + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/world", "outcome", "SUCCESS")) + .value(200) + .build()) + .build(); CounterSnapshot counter2 = - CounterSnapshot.builder() - .name("api_responses") - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("uri", "/hello", "outcome", "FAILURE")) - .value(10) - .build()) - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("uri", "/world", "outcome", "FAILURE")) - .value(5) - .build()) - .build(); + CounterSnapshot.builder() + .name("api_responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "FAILURE")) + .value(10) + .build()) + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/world", "outcome", "FAILURE")) + .value(5) + .build()) + .build(); MetricSnapshots snapshots = new MetricSnapshots(counter1, counter2); MetricSnapshots result = TextFormatUtil.mergeDuplicates(snapshots); diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 19db7e67b..5738babba 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -4,13 +4,13 @@ import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Predicate; import javax.annotation.Nullable; @@ -19,8 +19,8 @@ public class PrometheusRegistry { public static final PrometheusRegistry defaultRegistry = new PrometheusRegistry(); private final Set prometheusNames = ConcurrentHashMap.newKeySet(); - private final List collectors = new CopyOnWriteArrayList<>(); - private final List multiCollectors = new CopyOnWriteArrayList<>(); + private final Set collectors = ConcurrentHashMap.newKeySet(); + private final Set multiCollectors = ConcurrentHashMap.newKeySet(); private final ConcurrentHashMap registered = new ConcurrentHashMap<>(); /** @@ -31,25 +31,35 @@ private static class RegistrationInfo { private final MetricType type; private final Set> labelSets; - RegistrationInfo(MetricType type, @Nullable Set labelNames) { + private RegistrationInfo(MetricType type, Set> labelSets) { this.type = type; - this.labelSets = ConcurrentHashMap.newKeySet(); - if (labelNames != null) { - this.labelSets.add(labelNames); - } + this.labelSets = labelSets; + } + + static RegistrationInfo of(MetricType type, @Nullable Set labelNames) { + Set> labelSets = ConcurrentHashMap.newKeySet(); + Set normalized = + (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; + labelSets.add(normalized); + return new RegistrationInfo(type, labelSets); + } + + static RegistrationInfo withLabelSets(MetricType type, Set> labelSets) { + Set> newLabelSets = ConcurrentHashMap.newKeySet(); + newLabelSets.addAll(labelSets); + return new RegistrationInfo(type, newLabelSets); } /** * Adds a new label schema to this registration. * - * @param labelNames the label names to add + * @param labelNames the label names to add (null or empty sets are normalized to empty set) * @return true if the label schema was added, false if it already exists */ boolean addLabelSet(@Nullable Set labelNames) { - if (labelNames == null) { - return true; - } - return labelSets.add(labelNames); + Set normalized = + (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; + return labelSets.add(normalized); } MetricType getType() { @@ -71,7 +81,7 @@ public void register(Collector collector) { prometheusName, (name, existingInfo) -> { if (existingInfo == null) { - return new RegistrationInfo(metricType, labelNames); + return RegistrationInfo.of(metricType, labelNames); } else { if (existingInfo.getType() != metricType) { throw new IllegalArgumentException( @@ -82,14 +92,15 @@ public void register(Collector collector) { + metricType); } - // Check label schema uniqueness (if label names provided) - if (labelNames != null) { - if (!existingInfo.addLabelSet(labelNames)) { - throw new IllegalArgumentException( - prometheusName - + ": Duplicate label schema. A metric with the same name, type, and label" - + " names is already registered."); - } + if (!existingInfo.addLabelSet(labelNames)) { + Set normalized = + (labelNames == null || labelNames.isEmpty()) + ? Collections.emptySet() + : labelNames; + throw new IllegalArgumentException( + prometheusName + + ": duplicate metric name with identical label schema " + + normalized); } return existingInfo; @@ -120,7 +131,7 @@ public void register(MultiCollector collector) { prometheusName, (name, existingInfo) -> { if (existingInfo == null) { - return new RegistrationInfo(metricType, labelNames); + return RegistrationInfo.of(metricType, labelNames); } else { if (existingInfo.getType() != metricType) { throw new IllegalArgumentException( @@ -131,13 +142,15 @@ public void register(MultiCollector collector) { + metricType); } - if (labelNames != null) { - if (!existingInfo.addLabelSet(labelNames)) { - throw new IllegalArgumentException( - prometheusName - + ": Duplicate label schema. A metric with the same name, type, and" - + " label names is already registered."); - } + if (!existingInfo.addLabelSet(labelNames)) { + Set normalized = + (labelNames == null || labelNames.isEmpty()) + ? Collections.emptySet() + : labelNames; + throw new IllegalArgumentException( + prometheusName + + ": duplicate metric name with identical label schema " + + normalized); } return existingInfo; @@ -169,25 +182,47 @@ public void unregister(MultiCollector collector) { } private void nameInUse(String prometheusName) { - boolean nameStillInUse = false; + List remainingCollectors = new ArrayList<>(); for (Collector c : collectors) { if (prometheusName.equals(c.getPrometheusName())) { - nameStillInUse = true; - break; + remainingCollectors.add(c); } } - if (!nameStillInUse) { + + List remainingMultiCollectors = new ArrayList<>(); + if (remainingCollectors.isEmpty()) { for (MultiCollector mc : multiCollectors) { if (mc.getPrometheusNames().contains(prometheusName)) { - nameStillInUse = true; - break; + remainingMultiCollectors.add(mc); } } } - if (!nameStillInUse) { + + if (remainingCollectors.isEmpty() && remainingMultiCollectors.isEmpty()) { prometheusNames.remove(prometheusName); // Also remove from registered since no collectors use this name anymore registered.remove(prometheusName); + } else { + // Rebuild labelSets from remaining collectors + RegistrationInfo info = registered.get(prometheusName); + if (info != null) { + Set> newLabelSets = new HashSet<>(); + for (Collector c : remainingCollectors) { + Set labelNames = c.getLabelNames(); + if (labelNames != null) { + newLabelSets.add(labelNames); + } + } + for (MultiCollector mc : remainingMultiCollectors) { + Set labelNames = mc.getLabelNames(prometheusName); + if (labelNames != null) { + newLabelSets.add(labelNames); + } + } + // Replace the RegistrationInfo with updated labelSets + registered.put( + prometheusName, RegistrationInfo.withLabelSets(info.getType(), newLabelSets)); + } } } @@ -240,7 +275,7 @@ public MetricSnapshots scrape( if (includedNames == null) { return scrape(scrapeRequest); } - MetricSnapshots.Builder result = MetricSnapshots.builder(); + List allSnapshots = new ArrayList<>(); for (Collector collector : collectors) { String prometheusName = collector.getPrometheusName(); // prometheusName == null means the name is unknown, and we have to scrape to learn the name. @@ -251,7 +286,7 @@ public MetricSnapshots scrape( ? collector.collect(includedNames) : collector.collect(includedNames, scrapeRequest); if (snapshot != null) { - result.metricSnapshot(snapshot); + allSnapshots.add(snapshot); } } } @@ -275,11 +310,18 @@ public MetricSnapshots scrape( : collector.collect(includedNames, scrapeRequest); for (MetricSnapshot snapshot : snapshots) { if (snapshot != null) { - result.metricSnapshot(snapshot); + allSnapshots.add(snapshot); } } } } + + validateNoDuplicateLabelSchemas(allSnapshots); + + MetricSnapshots.Builder result = MetricSnapshots.builder(); + for (MetricSnapshot snapshot : allSnapshots) { + result.metricSnapshot(snapshot); + } return result.build(); } @@ -302,7 +344,6 @@ private void validateNoDuplicateLabelSchemas(List snapshots) { continue; } - // Extract label schemas from each snapshot List> labelSchemas = new ArrayList<>(); for (MetricSnapshot snapshot : group) { Set labelSchema = extractLabelSchema(snapshot); @@ -329,7 +370,6 @@ private Set extractLabelSchema(MetricSnapshot snapshot) { return null; } - // Get label names from the first data point DataPointSnapshot firstDataPoint = snapshot.getDataPoints().get(0); Set labelNames = new HashSet<>(); for (int i = 0; i < firstDataPoint.getLabels().size(); i++) { diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index 9f791e691..f1af8872f 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -1,5 +1,6 @@ package io.prometheus.metrics.model.registry; +import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -8,7 +9,6 @@ import io.prometheus.metrics.model.snapshots.GaugeSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; -import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -79,7 +79,7 @@ public MetricSnapshots collect() { @Override public List getPrometheusNames() { - return Arrays.asList(gaugeA.getPrometheusName(), counterB.getPrometheusName()); + return asList(gaugeA.getPrometheusName(), counterB.getPrometheusName()); } }; @@ -162,7 +162,7 @@ public MetricType getMetricType() { @Override public Set getLabelNames() { - return new HashSet<>(Arrays.asList("path", "status")); + return new HashSet<>(asList("path", "status")); } }; @@ -185,7 +185,7 @@ public MetricType getMetricType() { @Override public Set getLabelNames() { - return new HashSet<>(Arrays.asList("region")); + return new HashSet<>(asList("region")); } }; @@ -218,7 +218,7 @@ public MetricType getMetricType() { @Override public Set getLabelNames() { - return new HashSet<>(Arrays.asList("path", "status")); + return new HashSet<>(asList("path", "status")); } }; @@ -241,7 +241,7 @@ public MetricType getMetricType() { @Override public Set getLabelNames() { - return new HashSet<>(Arrays.asList("path", "status")); + return new HashSet<>(asList("path", "status")); } }; @@ -250,7 +250,7 @@ public Set getLabelNames() { // Second collector has same name, type, and label schema - should fail assertThatThrownBy(() -> registry.register(counter2)) .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Duplicate label schema"); + .hasMessageContaining("duplicate metric name with identical label schema"); } @Test @@ -321,7 +321,7 @@ public MetricSnapshots collect() { @Override public List getPrometheusNames() { - return Arrays.asList("shared_metric"); + return asList("shared_metric"); } @Override @@ -389,4 +389,252 @@ public void clearOk() { registry.clear(); assertThat(registry.scrape().size()).isZero(); } + + @Test + public void unregister_shouldRemoveLabelSchemaFromRegistrationInfo() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector counterWithPathLabel = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests_total").build(); + } + + @Override + public String getPrometheusName() { + return "requests_total"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("path", "status")); + } + }; + + Collector counterWithRegionLabel = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests_total").build(); + } + + @Override + public String getPrometheusName() { + return "requests_total"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(List.of("region")); + } + }; + + Collector counterWithPathLabelAgain = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests_total").build(); + } + + @Override + public String getPrometheusName() { + return "requests_total"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("path", "status")); + } + }; + + registry.register(counterWithPathLabel); + registry.register(counterWithRegionLabel); + + registry.unregister(counterWithPathLabel); + + assertThatCode(() -> registry.register(counterWithPathLabelAgain)).doesNotThrowAnyException(); + } + + @Test + public void scrape_withFilter_shouldValidateDuplicateLabelSchemas() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector collector1 = + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder() + .name("requests") + .dataPoint( + GaugeSnapshot.GaugeDataPointSnapshot.builder() + .value(100) + .labels(io.prometheus.metrics.model.snapshots.Labels.of("path", "/api")) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + // No getMetricType() or getLabelNames() - returns null by default + }; + + Collector collector2 = + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder() + .name("requests") + .dataPoint( + GaugeSnapshot.GaugeDataPointSnapshot.builder() + .value(200) + .labels(io.prometheus.metrics.model.snapshots.Labels.of("path", "/home")) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + // No getMetricType() or getLabelNames() - returns null by default + }; + + // Both collectors can register because they don't provide type/label info + registry.register(collector1); + registry.register(collector2); + + assertThatThrownBy(registry::scrape) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("duplicate metric name with identical label schema"); + + // Filtered scrape should also detect duplicate label schemas + assertThatThrownBy(() -> registry.scrape(name -> name.equals("requests"))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("duplicate metric name with identical label schema"); + } + + @Test + public void register_withEmptyLabelSets_shouldDetectDuplicates() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector collector1 = + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.GAUGE; + } + + // getLabelNames() returns null by default + }; + + // Register another collector with same name and type, also no labels + Collector collector2 = + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.GAUGE; + } + + // getLabelNames() returns null by default + }; + + registry.register(collector1); + + assertThatThrownBy(() -> registry.register(collector2)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("duplicate metric name with identical label schema"); + } + + @Test + public void register_withMixedNullAndEmptyLabelSets_shouldDetectDuplicates() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector collector1 = + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.GAUGE; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(); + } + }; + + Collector collector2 = + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.GAUGE; + } + + // getLabelNames() returns null by default + }; + + registry.register(collector1); + + // null and empty should be treated the same + assertThatThrownBy(() -> registry.register(collector2)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("duplicate metric name with identical label schema"); + } } From 6765715c203ef9eb953332b71e5fbf9c24526871 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Sat, 24 Jan 2026 14:54:14 -0500 Subject: [PATCH 07/23] fix linting Signed-off-by: Jay DeLuca --- .../generate-protobuf.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/prometheus-metrics-exposition-formats/generate-protobuf.sh b/prometheus-metrics-exposition-formats/generate-protobuf.sh index 7f14d703e..ed7b4aafd 100755 --- a/prometheus-metrics-exposition-formats/generate-protobuf.sh +++ b/prometheus-metrics-exposition-formats/generate-protobuf.sh @@ -8,22 +8,22 @@ set -euo pipefail # Use gsed and ggrep on macOS (requires: brew install gnu-sed grep) if [[ "$OSTYPE" == "darwin"* ]] && command -v gsed >/dev/null 2>&1; then - SED=gsed + SED='gsed' else - SED=sed + SED='sed' fi if [[ "$OSTYPE" == "darwin"* ]] && command -v ggrep >/dev/null 2>&1; then - GREP=ggrep + GREP='ggrep' else - GREP=grep + GREP='grep' fi # Use mise-provided protoc if available if command -v mise >/dev/null 2>&1; then - PROTOC="mise exec -- protoc" + PROTOC="mise exec -- protoc" else - PROTOC=protoc + PROTOC='protoc' fi TARGET_DIR=$1 From 11efcfb3af42fed788519db7a8a904eeccb7b6fe Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Thu, 29 Jan 2026 13:40:25 -0500 Subject: [PATCH 08/23] fix merge Signed-off-by: Jay DeLuca --- mise.toml | 2 +- .../model/registry/PrometheusRegistryTest.java | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/mise.toml b/mise.toml index 9cc01deee..5398f380d 100644 --- a/mise.toml +++ b/mise.toml @@ -1,7 +1,7 @@ [tools] "go:github.com/gohugoio/hugo" = "v0.155.0" "go:github.com/grafana/oats" = "0.6.0" -java = "temurin-25.0.2+10.0.LTS" +java = "temurin-25.0.1+8.0.LTS" lychee = "0.22.0" protoc = "33.4" diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index 2c61b933a..3ecfa9156 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -9,6 +9,8 @@ import io.prometheus.metrics.model.snapshots.GaugeSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; + +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -84,17 +86,13 @@ public List getPrometheusNames() { }; @Test - void registerDuplicateName_withoutTypeInfo_allowedForBackwardCompatibility() { + void register_duplicateName_withoutTypeInfo_notAllowed() { PrometheusRegistry registry = new PrometheusRegistry(); - // If the collector does not have a name at registration time, there is no conflict during - // registration. - registry.register(noName); + registry.register(noName); - // However, at scrape time the collector has to provide a metric name, and then we'll get a - // duplicate name error. - assertThatCode(registry::scrape) - .hasMessageContaining("duplicate") - .hasMessageContaining("no_name_gauge"); + + assertThatThrownBy(() -> registry.register(noName)) + .hasMessageContaining("Collector instance is already registered"); } @Test From 02c0db0f1e324d94f440161a370033e17cf7eba2 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Thu, 29 Jan 2026 14:58:58 -0500 Subject: [PATCH 09/23] remove scrape time validation Signed-off-by: Jay DeLuca --- docs/content/getting-started/metric-types.md | 3 + docs/content/getting-started/registry.md | 11 +++ docs/content/internals/model.md | 5 +- .../DuplicateNamesExpositionTest.java | 54 -------------- .../metrics/model/registry/Collector.java | 9 +++ .../model/registry/MultiCollector.java | 9 +++ .../model/registry/PrometheusRegistry.java | 73 ------------------- .../registry/PrometheusRegistryTest.java | 63 +--------------- 8 files changed, 37 insertions(+), 190 deletions(-) diff --git a/docs/content/getting-started/metric-types.md b/docs/content/getting-started/metric-types.md index 46d53ece1..cf3689a75 100644 --- a/docs/content/getting-started/metric-types.md +++ b/docs/content/getting-started/metric-types.md @@ -276,3 +276,6 @@ in the `prometheus-metrics-core` API. However, `prometheus-metrics-model` implements the underlying data model for these types. To use these types, you need to implement your own `Collector` where the `collect()` method returns an `UnknownSnapshot` or a `HistogramSnapshot` with `.gaugeHistogram(true)`. +If your custom collector does not implement `getMetricType()` and `getLabelNames()`, ensure it does +not produce the same metric name and label set as another collector, or the exposition may contain +duplicate time series. diff --git a/docs/content/getting-started/registry.md b/docs/content/getting-started/registry.md index afebbb304..f51cd521f 100644 --- a/docs/content/getting-started/registry.md +++ b/docs/content/getting-started/registry.md @@ -78,6 +78,17 @@ Counter eventsTotal2 = Counter.builder() .register(); // IllegalArgumentException, because a metric with that name is already registered ``` +## Validation at registration only + +Validation of duplicate metric names and label schemas happens at registration time only. +Built-in metrics (Counter, Gauge, Histogram, etc.) participate in this validation. + +Custom collectors that implement the `Collector` or `MultiCollector` interface can optionally +implement `getMetricType()` and `getLabelNames()` (or the MultiCollector per-name variants) so the +registry can enforce consistency. If those methods return `null`, the registry does not validate +that collector. If two such collectors produce the same metric name and same label set at scrape +time, the exposition output may contain duplicate time series and be invalid for Prometheus. + ## Unregistering a Metric There is no automatic expiry of unused metrics (yet), once a metric is registered it will remain diff --git a/docs/content/internals/model.md b/docs/content/internals/model.md index c54e79ee3..e1b2af644 100644 --- a/docs/content/internals/model.md +++ b/docs/content/internals/model.md @@ -19,7 +19,10 @@ All metric types implement the [Collector](/client_java/api/io/prometheus/metrics/model/registry/Collector.html) interface, i.e. they provide a [collect()]() -method to produce snapshots. +method to produce snapshots. Implementers that do not provide metric type or label names (returning +null from `getMetricType()` and `getLabelNames()`) are not validated at registration; they must +avoid producing the same metric name and label schema as another collector, or exposition may be +invalid. ## prometheus-metrics-model diff --git a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java index e13355079..038e26327 100644 --- a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java +++ b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java @@ -2,7 +2,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; import io.prometheus.metrics.model.registry.Collector; import io.prometheus.metrics.model.registry.PrometheusRegistry; @@ -181,57 +180,4 @@ void testOpenMetricsFormat_withDuplicateNames() throws IOException { """; assertThat(output).isEqualTo(expected); } - - @Test - void testDuplicateNames_sameLabels_throwsException() { - PrometheusRegistry registry = new PrometheusRegistry(); - - registry.register( - new Collector() { - @Override - public MetricSnapshot collect() { - return CounterSnapshot.builder() - .name("api_responses") - .help("API responses") - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) - .value(100) - .build()) - .build(); - } - - @Override - public String getPrometheusName() { - return "api_responses_total"; - } - }); - - registry.register( - new Collector() { - @Override - public MetricSnapshot collect() { - return CounterSnapshot.builder() - .name("api_responses") - .help("API responses") - .dataPoint( - CounterSnapshot.CounterDataPointSnapshot.builder() - .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) - .value(50) - .build()) - .build(); - } - - @Override - public String getPrometheusName() { - return "api_responses_total"; - } - }); - - // Scrape should throw exception due to duplicate time series (same name + same labels) - assertThatThrownBy(registry::scrape) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("duplicate metric name with identical label schema [uri, outcome]") - .hasMessageContaining("api_responses"); - } } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java index b2551c81c..bae7982da 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java @@ -86,6 +86,10 @@ default String getPrometheusName() { *

This is used to prevent different metric types (e.g., Counter and Gauge) from sharing the * same name. Returning {@code null} means type validation is skipped for this collector. * + *

Validation is performed only at registration time. If this method returns {@code null}, no + * type validation is performed for this collector, and duplicate or conflicting metrics may + * result in invalid exposition output. + * * @return the metric type, or {@code null} to skip validation */ @Nullable @@ -106,6 +110,11 @@ default MetricType getMetricType() { * *

Returning {@code null} means label schema validation is skipped for this collector. * + *

Validation is performed only at registration time. If this method returns {@code null}, no + * label-schema validation is performed for this collector. If such a collector produces the same + * metric name and label schema as another at scrape time, the exposition may contain duplicate + * time series, which is invalid in Prometheus. + * * @return the set of all label names, or {@code null} to skip validation */ @Nullable diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java index 879efaba6..d3e3c8145 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java @@ -78,6 +78,10 @@ default List getPrometheusNames() { *

This is used for per-name type validation during registration. Returning {@code null} means * type validation is skipped for that specific metric name. * + *

Validation is performed only at registration time. If this method returns {@code null}, no + * type validation is performed for that name, and duplicate or conflicting metrics may result in + * invalid exposition output. + * * @param prometheusName the Prometheus metric name * @return the metric type for the given name, or {@code null} to skip validation */ @@ -98,6 +102,11 @@ default MetricType getMetricType(String prometheusName) { *

Returning {@code null} means label schema validation is skipped for that specific metric * name. * + *

Validation is performed only at registration time. If this method returns {@code null}, no + * label-schema validation is performed for that name. If such a collector produces the same + * metric name and label schema as another at scrape time, the exposition may contain duplicate + * time series, which is invalid in Prometheus. + * * @param prometheusName the Prometheus metric name * @return the set of all label names for the given name, or {@code null} to skip validation */ diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 5738babba..7b4de5f8b 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -1,14 +1,11 @@ package io.prometheus.metrics.model.registry; -import io.prometheus.metrics.model.snapshots.DataPointSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Predicate; @@ -254,8 +251,6 @@ public MetricSnapshots scrape(@Nullable PrometheusScrapeRequest scrapeRequest) { } } - validateNoDuplicateLabelSchemas(allSnapshots); - MetricSnapshots.Builder result = MetricSnapshots.builder(); for (MetricSnapshot snapshot : allSnapshots) { result.metricSnapshot(snapshot); @@ -316,78 +311,10 @@ public MetricSnapshots scrape( } } - validateNoDuplicateLabelSchemas(allSnapshots); - MetricSnapshots.Builder result = MetricSnapshots.builder(); for (MetricSnapshot snapshot : allSnapshots) { result.metricSnapshot(snapshot); } return result.build(); } - - /** - * Validates that snapshots with the same metric name don't have identical label schemas. This - * prevents duplicate time series which would occur if two snapshots produce data points with - * identical label sets. - */ - private void validateNoDuplicateLabelSchemas(List snapshots) { - Map> snapshotsByName = new HashMap<>(); - for (MetricSnapshot snapshot : snapshots) { - String name = snapshot.getMetadata().getPrometheusName(); - snapshotsByName.computeIfAbsent(name, k -> new ArrayList<>()).add(snapshot); - } - - // For each group with the same name, check for duplicate label schemas - for (Map.Entry> entry : snapshotsByName.entrySet()) { - List group = entry.getValue(); - if (group.size() <= 1) { - continue; - } - - List> labelSchemas = new ArrayList<>(); - for (MetricSnapshot snapshot : group) { - Set labelSchema = extractLabelSchema(snapshot); - if (labelSchema != null) { - if (labelSchemas.contains(labelSchema)) { - throw new IllegalStateException( - snapshot.getMetadata().getPrometheusName() - + ": duplicate metric name with identical label schema " - + labelSchema); - } - labelSchemas.add(labelSchema); - } - } - } - } - - /** - * Extracts the label schema (set of label names) from a snapshot's data points. Returns null if - * the snapshot has no data points or if data points have inconsistent label schemas. - */ - @Nullable - private Set extractLabelSchema(MetricSnapshot snapshot) { - if (snapshot.getDataPoints().isEmpty()) { - return null; - } - - DataPointSnapshot firstDataPoint = snapshot.getDataPoints().get(0); - Set labelNames = new HashSet<>(); - for (int i = 0; i < firstDataPoint.getLabels().size(); i++) { - labelNames.add(firstDataPoint.getLabels().getName(i)); - } - - for (DataPointSnapshot dataPoint : snapshot.getDataPoints()) { - Set currentLabelNames = new HashSet<>(); - for (int i = 0; i < dataPoint.getLabels().size(); i++) { - currentLabelNames.add(dataPoint.getLabels().getName(i)); - } - if (!currentLabelNames.equals(labelNames)) { - // Data points have inconsistent label schemas - this is unusual but valid - // We can't determine a single label schema, so return null - return null; - } - } - - return labelNames; - } } diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index 3ecfa9156..e6f6c4267 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -9,7 +9,6 @@ import io.prometheus.metrics.model.snapshots.GaugeSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; - import java.util.Arrays; import java.util.HashSet; import java.util.List; @@ -439,7 +438,7 @@ public MetricType getMetricType() { @Override public Set getLabelNames() { - return new HashSet<>(List.of("region")); + return new HashSet<>(asList("region")); } }; @@ -474,66 +473,6 @@ public Set getLabelNames() { assertThatCode(() -> registry.register(counterWithPathLabelAgain)).doesNotThrowAnyException(); } - @Test - public void scrape_withFilter_shouldValidateDuplicateLabelSchemas() { - PrometheusRegistry registry = new PrometheusRegistry(); - - Collector collector1 = - new Collector() { - @Override - public MetricSnapshot collect() { - return GaugeSnapshot.builder() - .name("requests") - .dataPoint( - GaugeSnapshot.GaugeDataPointSnapshot.builder() - .value(100) - .labels(io.prometheus.metrics.model.snapshots.Labels.of("path", "/api")) - .build()) - .build(); - } - - @Override - public String getPrometheusName() { - return "requests"; - } - // No getMetricType() or getLabelNames() - returns null by default - }; - - Collector collector2 = - new Collector() { - @Override - public MetricSnapshot collect() { - return GaugeSnapshot.builder() - .name("requests") - .dataPoint( - GaugeSnapshot.GaugeDataPointSnapshot.builder() - .value(200) - .labels(io.prometheus.metrics.model.snapshots.Labels.of("path", "/home")) - .build()) - .build(); - } - - @Override - public String getPrometheusName() { - return "requests"; - } - // No getMetricType() or getLabelNames() - returns null by default - }; - - // Both collectors can register because they don't provide type/label info - registry.register(collector1); - registry.register(collector2); - - assertThatThrownBy(registry::scrape) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("duplicate metric name with identical label schema"); - - // Filtered scrape should also detect duplicate label schemas - assertThatThrownBy(() -> registry.scrape(name -> name.equals("requests"))) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("duplicate metric name with identical label schema"); - } - @Test public void register_withEmptyLabelSets_shouldDetectDuplicates() { PrometheusRegistry registry = new PrometheusRegistry(); From 8b6bf201c0503b7c2168172112a4039f37f9d783 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Fri, 30 Jan 2026 11:47:04 -0500 Subject: [PATCH 10/23] cleanup registration Signed-off-by: Jay DeLuca --- .gitignore | 2 + .../core/metrics/MetricWithFixedMetadata.java | 3 +- .../metrics/model/registry/Collector.java | 15 + .../model/registry/MultiCollector.java | 16 + .../model/registry/PrometheusRegistry.java | 289 ++++++++++++------ .../SnapshotRegistrationExtractor.java | 78 +++++ .../registry/PrometheusRegistryTest.java | 143 ++++++++- 7 files changed, 450 insertions(+), 96 deletions(-) create mode 100644 prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/SnapshotRegistrationExtractor.java diff --git a/.gitignore b/.gitignore index 83f5595ba..b98fa5703 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,5 @@ docs/public benchmark-results/ benchmark-results.json benchmark-output.log + +*.DS_Store \ No newline at end of file diff --git a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java index a88ce642a..12c48c51d 100644 --- a/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java +++ b/prometheus-metrics-core/src/main/java/io/prometheus/metrics/core/metrics/MetricWithFixedMetadata.java @@ -30,7 +30,8 @@ protected MetricWithFixedMetadata(Builder builder) { this.labelNames = Arrays.copyOf(builder.labelNames, builder.labelNames.length); } - protected MetricMetadata getMetadata() { + @Override + public MetricMetadata getMetadata() { return metadata; } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java index bae7982da..741364fe4 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java @@ -1,5 +1,6 @@ package io.prometheus.metrics.model.registry; +import io.prometheus.metrics.model.snapshots.MetricMetadata; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import java.util.Set; import java.util.function.Predicate; @@ -121,4 +122,18 @@ default MetricType getMetricType() { default Set getLabelNames() { return null; } + + /** + * Returns the metric metadata (name, help, unit) for registration-time validation. + * + *

When non-null, the registry uses this to validate that metrics with the same name have + * consistent help and unit. Returning {@code null} means help/unit validation is skipped for this + * collector. + * + * @return the metric metadata, or {@code null} to skip help/unit validation + */ + @Nullable + default MetricMetadata getMetadata() { + return null; + } } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java index d3e3c8145..6c4759995 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java @@ -1,5 +1,6 @@ package io.prometheus.metrics.model.registry; +import io.prometheus.metrics.model.snapshots.MetricMetadata; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; import java.util.Collections; @@ -114,4 +115,19 @@ default MetricType getMetricType(String prometheusName) { default Set getLabelNames(String prometheusName) { return null; } + + /** + * Returns the metric metadata (name, help, unit) for the given Prometheus name. + * + *

When non-null, the registry uses this to validate that metrics with the same name have + * consistent help and unit. Returning {@code null} means help/unit validation is skipped for that + * name. + * + * @param prometheusName the Prometheus metric name + * @return the metric metadata for that name, or {@code null} to skip help/unit validation + */ + @Nullable + default MetricMetadata getMetadata(String prometheusName) { + return null; + } } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 7b4de5f8b..9d3490ecb 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -1,11 +1,13 @@ package io.prometheus.metrics.model.registry; +import io.prometheus.metrics.model.snapshots.MetricMetadata; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; +import io.prometheus.metrics.model.snapshots.Unit; import java.util.ArrayList; import java.util.Collections; -import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Predicate; @@ -19,49 +21,135 @@ public class PrometheusRegistry { private final Set collectors = ConcurrentHashMap.newKeySet(); private final Set multiCollectors = ConcurrentHashMap.newKeySet(); private final ConcurrentHashMap registered = new ConcurrentHashMap<>(); + private final ConcurrentHashMap collectorMetadata = + new ConcurrentHashMap<>(); + private final ConcurrentHashMap> + multiCollectorMetadata = new ConcurrentHashMap<>(); /** - * Tracks registration information for each metric name to enable validation of type consistency - * and label schema uniqueness. + * Stores the registration details for a Collector at registration time. + */ + private static class CollectorRegistration { + final String prometheusName; + final MetricType metricType; + final Set labelNames; + + CollectorRegistration(String prometheusName, MetricType metricType, Set labelNames) { + this.prometheusName = prometheusName; + this.metricType = metricType; + this.labelNames = + (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; + } + } + + /** + * Stores the registration details for a single metric within a MultiCollector. A MultiCollector + * can produce multiple metrics, so we need one of these per metric name. + */ + private static class MultiCollectorRegistration { + final String prometheusName; + final MetricType metricType; + final Set labelNames; + + MultiCollectorRegistration( + String prometheusName, MetricType metricType, Set labelNames) { + this.prometheusName = prometheusName; + this.metricType = metricType; + this.labelNames = + (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; + } + } + + /** + * Tracks registration information for each metric name to enable validation of type consistency, + * label schema uniqueness, and help/unit consistency. Stores metadata to enable O(1) unregistration + * without iterating through all collectors. */ private static class RegistrationInfo { private final MetricType type; - private final Set> labelSets; - - private RegistrationInfo(MetricType type, Set> labelSets) { + private final Set> labelSchemas; + @Nullable private final String help; + @Nullable private final Unit unit; + + private RegistrationInfo( + MetricType type, + Set> labelSchemas, + @Nullable String help, + @Nullable Unit unit) { this.type = type; - this.labelSets = labelSets; + this.labelSchemas = labelSchemas; + this.help = help; + this.unit = unit; } - static RegistrationInfo of(MetricType type, @Nullable Set labelNames) { - Set> labelSets = ConcurrentHashMap.newKeySet(); + static RegistrationInfo of( + MetricType type, + @Nullable Set labelNames, + @Nullable String help, + @Nullable Unit unit) { + Set> labelSchemas = ConcurrentHashMap.newKeySet(); Set normalized = (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; - labelSets.add(normalized); - return new RegistrationInfo(type, labelSets); + labelSchemas.add(normalized); + return new RegistrationInfo(type, labelSchemas, help, unit); } - static RegistrationInfo withLabelSets(MetricType type, Set> labelSets) { - Set> newLabelSets = ConcurrentHashMap.newKeySet(); - newLabelSets.addAll(labelSets); - return new RegistrationInfo(type, newLabelSets); + /** + * Validates that the given help and unit are consistent with this registration. Throws if both + * sides have non-null help/unit and they differ. + */ + void validateMetadata(@Nullable String newHelp, @Nullable Unit newUnit) { + if (help != null && newHelp != null && !Objects.equals(help, newHelp)) { + throw new IllegalArgumentException( + "Conflicting help strings. Existing: \"" + help + "\", new: \"" + newHelp + "\""); + } + if (unit != null && newUnit != null && !Objects.equals(unit, newUnit)) { + throw new IllegalArgumentException( + "Conflicting unit. Existing: " + unit + ", new: " + newUnit); + } } /** - * Adds a new label schema to this registration. + * Adds a label schema to this registration. * * @param labelNames the label names to add (null or empty sets are normalized to empty set) - * @return true if the label schema was added, false if it already exists + * @return true if the schema was added (new), false if it already existed */ boolean addLabelSet(@Nullable Set labelNames) { Set normalized = (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; - return labelSets.add(normalized); + return labelSchemas.add(normalized); + } + + /** + * Removes a label schema from this registration. + * + * @param labelNames the label names to remove (null or empty sets are normalized to empty set) + */ + void removeLabelSet(@Nullable Set labelNames) { + Set normalized = + (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; + labelSchemas.remove(normalized); + } + + /** Returns true if all label schemas have been unregistered. */ + boolean isEmpty() { + return labelSchemas.isEmpty(); } MetricType getType() { return type; } + + @Nullable + String getHelp() { + return help; + } + + @Nullable + Unit getUnit() { + return unit; + } } public void register(Collector collector) { @@ -72,37 +160,69 @@ public void register(Collector collector) { String prometheusName = collector.getPrometheusName(); MetricType metricType = collector.getMetricType(); Set labelNames = collector.getLabelNames(); + MetricMetadata metadata = collector.getMetadata(); + String help = metadata != null ? metadata.getHelp() : null; + Unit unit = metadata != null ? metadata.getUnit() : null; + + // When getters return null, fall back to one collect() to derive type/labels/metadata. + // Collectors whose collect() has side effects (e.g. callbacks) should implement the getters. + if (metricType == null && prometheusName != null) { + MetricSnapshot snapshot = collector.collect(); + if (snapshot != null) { + prometheusName = snapshot.getMetadata().getPrometheusName(); + metricType = SnapshotRegistrationExtractor.metricTypeFromSnapshot(snapshot); + labelNames = SnapshotRegistrationExtractor.labelNamesFromSnapshot(snapshot); + MetricMetadata snapshotMetadata = + SnapshotRegistrationExtractor.metadataFromSnapshot(snapshot); + help = snapshotMetadata.getHelp(); + unit = snapshotMetadata.getUnit(); + } + } else if (metricType == null && prometheusName == null) { + MetricSnapshot snapshot = collector.collect(); + if (snapshot != null) { + prometheusName = snapshot.getMetadata().getPrometheusName(); + metricType = SnapshotRegistrationExtractor.metricTypeFromSnapshot(snapshot); + labelNames = SnapshotRegistrationExtractor.labelNamesFromSnapshot(snapshot); + MetricMetadata snapshotMetadata = + SnapshotRegistrationExtractor.metadataFromSnapshot(snapshot); + help = snapshotMetadata.getHelp(); + unit = snapshotMetadata.getUnit(); + } + } if (prometheusName != null && metricType != null) { + final String name = prometheusName; + final MetricType type = metricType; + final Set names = labelNames; + final String helpForValidation = help; + final Unit unitForValidation = unit; registered.compute( prometheusName, - (name, existingInfo) -> { + (n, existingInfo) -> { if (existingInfo == null) { - return RegistrationInfo.of(metricType, labelNames); + return RegistrationInfo.of(type, names, helpForValidation, unitForValidation); } else { - if (existingInfo.getType() != metricType) { + if (existingInfo.getType() != type) { throw new IllegalArgumentException( - prometheusName + name + ": Conflicting metric types. Existing: " + existingInfo.getType() + ", new: " - + metricType); + + type); } - - if (!existingInfo.addLabelSet(labelNames)) { + existingInfo.validateMetadata(helpForValidation, unitForValidation); + if (!existingInfo.addLabelSet(names)) { Set normalized = - (labelNames == null || labelNames.isEmpty()) - ? Collections.emptySet() - : labelNames; + (names == null || names.isEmpty()) ? Collections.emptySet() : names; throw new IllegalArgumentException( - prometheusName - + ": duplicate metric name with identical label schema " - + normalized); + name + ": duplicate metric name with identical label schema " + normalized); } - return existingInfo; } }); + + collectorMetadata.put( + collector, new CollectorRegistration(prometheusName, metricType, labelNames)); } if (prometheusName != null) { @@ -117,110 +237,97 @@ public void register(MultiCollector collector) { throw new IllegalArgumentException("MultiCollector instance is already registered"); } - List names = collector.getPrometheusNames(); + List prometheusNamesList = collector.getPrometheusNames(); + List registrations = new ArrayList<>(); - for (String prometheusName : names) { + for (String prometheusName : prometheusNamesList) { MetricType metricType = collector.getMetricType(prometheusName); Set labelNames = collector.getLabelNames(prometheusName); + MetricMetadata metadata = collector.getMetadata(prometheusName); + String help = metadata != null ? metadata.getHelp() : null; + Unit unit = metadata != null ? metadata.getUnit() : null; if (metricType != null) { + final MetricType type = metricType; + final Set labelNamesForValidation = labelNames; + final String helpForValidation = help; + final Unit unitForValidation = unit; registered.compute( prometheusName, (name, existingInfo) -> { if (existingInfo == null) { - return RegistrationInfo.of(metricType, labelNames); + return RegistrationInfo.of( + type, labelNamesForValidation, helpForValidation, unitForValidation); } else { - if (existingInfo.getType() != metricType) { + if (existingInfo.getType() != type) { throw new IllegalArgumentException( prometheusName + ": Conflicting metric types. Existing: " + existingInfo.getType() + ", new: " - + metricType); + + type); } - - if (!existingInfo.addLabelSet(labelNames)) { + existingInfo.validateMetadata(helpForValidation, unitForValidation); + if (!existingInfo.addLabelSet(labelNamesForValidation)) { Set normalized = - (labelNames == null || labelNames.isEmpty()) + (labelNamesForValidation == null || labelNamesForValidation.isEmpty()) ? Collections.emptySet() - : labelNames; + : labelNamesForValidation; throw new IllegalArgumentException( prometheusName + ": duplicate metric name with identical label schema " + normalized); } - return existingInfo; } }); + + registrations.add(new MultiCollectorRegistration(prometheusName, metricType, labelNames)); } prometheusNames.add(prometheusName); } + multiCollectorMetadata.put(collector, registrations); multiCollectors.add(collector); } public void unregister(Collector collector) { collectors.remove(collector); - String prometheusName = collector.getPrometheusName(); - if (prometheusName != null) { - // Check if any other collectors are still using this name - nameInUse(prometheusName); + + CollectorRegistration registration = collectorMetadata.remove(collector); + if (registration != null && registration.prometheusName != null) { + unregisterLabelSchema(registration.prometheusName, registration.labelNames); } } public void unregister(MultiCollector collector) { multiCollectors.remove(collector); - for (String prometheusName : collector.getPrometheusNames()) { - // Check if any other collectors are still using this name - nameInUse(prometheusName); - } - } - private void nameInUse(String prometheusName) { - List remainingCollectors = new ArrayList<>(); - for (Collector c : collectors) { - if (prometheusName.equals(c.getPrometheusName())) { - remainingCollectors.add(c); - } - } - - List remainingMultiCollectors = new ArrayList<>(); - if (remainingCollectors.isEmpty()) { - for (MultiCollector mc : multiCollectors) { - if (mc.getPrometheusNames().contains(prometheusName)) { - remainingMultiCollectors.add(mc); - } + List registrations = multiCollectorMetadata.remove(collector); + if (registrations != null) { + for (MultiCollectorRegistration registration : registrations) { + unregisterLabelSchema(registration.prometheusName, registration.labelNames); } } + } - if (remainingCollectors.isEmpty() && remainingMultiCollectors.isEmpty()) { - prometheusNames.remove(prometheusName); - // Also remove from registered since no collectors use this name anymore - registered.remove(prometheusName); - } else { - // Rebuild labelSets from remaining collectors - RegistrationInfo info = registered.get(prometheusName); - if (info != null) { - Set> newLabelSets = new HashSet<>(); - for (Collector c : remainingCollectors) { - Set labelNames = c.getLabelNames(); - if (labelNames != null) { - newLabelSets.add(labelNames); - } - } - for (MultiCollector mc : remainingMultiCollectors) { - Set labelNames = mc.getLabelNames(prometheusName); - if (labelNames != null) { - newLabelSets.add(labelNames); + /** + * Decrements the reference count for a label schema and removes the metric name entirely if no + * schemas remain. + */ + private void unregisterLabelSchema(String prometheusName, Set labelNames) { + registered.computeIfPresent( + prometheusName, + (name, info) -> { + info.removeLabelSet(labelNames); + if (info.isEmpty()) { + // No more label schemas for this name, remove it entirely + prometheusNames.remove(prometheusName); + return null; // remove from registered map } - } - // Replace the RegistrationInfo with updated labelSets - registered.put( - prometheusName, RegistrationInfo.withLabelSets(info.getType(), newLabelSets)); - } - } + return info; // keep the RegistrationInfo, just with decremented count + }); } public void clear() { @@ -228,6 +335,8 @@ public void clear() { multiCollectors.clear(); prometheusNames.clear(); registered.clear(); + collectorMetadata.clear(); + multiCollectorMetadata.clear(); } public MetricSnapshots scrape() { diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/SnapshotRegistrationExtractor.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/SnapshotRegistrationExtractor.java new file mode 100644 index 000000000..746103fd7 --- /dev/null +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/SnapshotRegistrationExtractor.java @@ -0,0 +1,78 @@ +package io.prometheus.metrics.model.registry; + +import io.prometheus.metrics.model.snapshots.CounterSnapshot; +import io.prometheus.metrics.model.snapshots.GaugeSnapshot; +import io.prometheus.metrics.model.snapshots.HistogramSnapshot; +import io.prometheus.metrics.model.snapshots.InfoSnapshot; +import io.prometheus.metrics.model.snapshots.Labels; +import io.prometheus.metrics.model.snapshots.MetricMetadata; +import io.prometheus.metrics.model.snapshots.MetricSnapshot; +import io.prometheus.metrics.model.snapshots.StateSetSnapshot; +import io.prometheus.metrics.model.snapshots.SummarySnapshot; +import io.prometheus.metrics.model.snapshots.UnknownSnapshot; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +/** + * Extracts registration-relevant data (metric type, label names) from a {@link MetricSnapshot}. + * Used when a collector does not implement {@link Collector#getMetricType()} / {@link + * Collector#getLabelNames()} so the registry can validate from a one-time {@link + * Collector#collect()} at registration. + * + *

Collectors whose {@code collect()} has side effects (e.g. callbacks) should implement the + * getters so the registry does not call {@code collect()} at registration. + */ +final class SnapshotRegistrationExtractor { + + private SnapshotRegistrationExtractor() {} + + static MetricType metricTypeFromSnapshot(MetricSnapshot snapshot) { + if (snapshot instanceof CounterSnapshot) { + return MetricType.COUNTER; + } + if (snapshot instanceof GaugeSnapshot) { + return MetricType.GAUGE; + } + if (snapshot instanceof HistogramSnapshot) { + return MetricType.HISTOGRAM; + } + if (snapshot instanceof SummarySnapshot) { + return MetricType.SUMMARY; + } + if (snapshot instanceof InfoSnapshot) { + return MetricType.INFO; + } + if (snapshot instanceof StateSetSnapshot) { + return MetricType.STATESET; + } + if (snapshot instanceof UnknownSnapshot) { + return MetricType.UNKNOWN; + } + return MetricType.UNKNOWN; + } + + /** + * Returns the set of label names (Prometheus-normalized) from the snapshot's first data point, or + * empty set if there are no data points. + */ + static Set labelNamesFromSnapshot(MetricSnapshot snapshot) { + if (snapshot.getDataPoints().isEmpty()) { + return Collections.emptySet(); + } + Labels labels = snapshot.getDataPoints().get(0).getLabels(); + if (labels.isEmpty()) { + return Collections.emptySet(); + } + Set names = new HashSet<>(); + for (int i = 0; i < labels.size(); i++) { + names.add(labels.getPrometheusName(i)); + } + return names; + } + + /** Returns metadata from the snapshot for help/unit validation (snapshot always has metadata). */ + static MetricMetadata metadataFromSnapshot(MetricSnapshot snapshot) { + return snapshot.getMetadata(); + } +} diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index e6f6c4267..7c53f358f 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -7,6 +7,7 @@ import io.prometheus.metrics.model.snapshots.CounterSnapshot; import io.prometheus.metrics.model.snapshots.GaugeSnapshot; +import io.prometheus.metrics.model.snapshots.MetricMetadata; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; import java.util.Arrays; @@ -256,10 +257,11 @@ public Set getLabelNames() { } @Test - public void register_backwardCompatibility_nullTypeAndLabels_skipsValidation() { + public void register_nullTypeAndLabels_fallbackToCollect_validatesFromSnapshot() { PrometheusRegistry registry = new PrometheusRegistry(); - // Collector without getMetricType() and getLabelNames() - returns null (default) + // Collector without getMetricType() and getLabelNames() - returns null (default). + // Registry falls back to collect() and derives type/labels from snapshot. Collector legacyCollector1 = new Collector() { @Override @@ -286,10 +288,11 @@ public String getPrometheusName() { } }; - // Both collectors have the same name but no type/label info - // Should succeed because validation is skipped registry.register(legacyCollector1); - assertThatCode(() -> registry.register(legacyCollector2)).doesNotThrowAnyException(); + // Second collector has same name but different type (from snapshot) - should fail + assertThatThrownBy(() -> registry.register(legacyCollector2)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Conflicting metric types"); } @Test @@ -579,4 +582,134 @@ public MetricType getMetricType() { .isInstanceOf(IllegalArgumentException.class) .hasMessageContaining("duplicate metric name with identical label schema"); } + + @Test + public void register_sameName_differentHelp_notAllowed() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector withHelpOne = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").help("First help").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("path")); + } + + @Override + public MetricMetadata getMetadata() { + return new MetricMetadata("requests", "First help", null); + } + }; + + Collector withHelpTwo = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").help("Second help").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("status")); + } + + @Override + public MetricMetadata getMetadata() { + return new MetricMetadata("requests", "Second help", null); + } + }; + + registry.register(withHelpOne); + assertThatThrownBy(() -> registry.register(withHelpTwo)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Conflicting help strings"); + } + + @Test + public void register_sameName_sameHelpAndUnit_allowed() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector withPath = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").help("Total requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("path")); + } + + @Override + public MetricMetadata getMetadata() { + return new MetricMetadata("requests", "Total requests", null); + } + }; + + Collector withStatus = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").help("Total requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("status")); + } + + @Override + public MetricMetadata getMetadata() { + return new MetricMetadata("requests", "Total requests", null); + } + }; + + registry.register(withPath); + assertThatCode(() -> registry.register(withStatus)).doesNotThrowAnyException(); + } } From 54f5ecbb5f726321c406af649e890b416fee1aa6 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Fri, 30 Jan 2026 12:07:29 -0500 Subject: [PATCH 11/23] cleanup fallback Signed-off-by: Jay DeLuca --- .../model/registry/PrometheusRegistry.java | 36 ++------- .../SnapshotRegistrationExtractor.java | 78 ------------------- .../registry/PrometheusRegistryTest.java | 14 ++-- 3 files changed, 11 insertions(+), 117 deletions(-) delete mode 100644 prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/SnapshotRegistrationExtractor.java diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 9d3490ecb..2df368150 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -26,9 +26,7 @@ public class PrometheusRegistry { private final ConcurrentHashMap> multiCollectorMetadata = new ConcurrentHashMap<>(); - /** - * Stores the registration details for a Collector at registration time. - */ + /** Stores the registration details for a Collector at registration time. */ private static class CollectorRegistration { final String prometheusName; final MetricType metricType; @@ -62,8 +60,8 @@ private static class MultiCollectorRegistration { /** * Tracks registration information for each metric name to enable validation of type consistency, - * label schema uniqueness, and help/unit consistency. Stores metadata to enable O(1) unregistration - * without iterating through all collectors. + * label schema uniqueness, and help/unit consistency. Stores metadata to enable O(1) + * unregistration without iterating through all collectors. */ private static class RegistrationInfo { private final MetricType type; @@ -164,32 +162,8 @@ public void register(Collector collector) { String help = metadata != null ? metadata.getHelp() : null; Unit unit = metadata != null ? metadata.getUnit() : null; - // When getters return null, fall back to one collect() to derive type/labels/metadata. - // Collectors whose collect() has side effects (e.g. callbacks) should implement the getters. - if (metricType == null && prometheusName != null) { - MetricSnapshot snapshot = collector.collect(); - if (snapshot != null) { - prometheusName = snapshot.getMetadata().getPrometheusName(); - metricType = SnapshotRegistrationExtractor.metricTypeFromSnapshot(snapshot); - labelNames = SnapshotRegistrationExtractor.labelNamesFromSnapshot(snapshot); - MetricMetadata snapshotMetadata = - SnapshotRegistrationExtractor.metadataFromSnapshot(snapshot); - help = snapshotMetadata.getHelp(); - unit = snapshotMetadata.getUnit(); - } - } else if (metricType == null && prometheusName == null) { - MetricSnapshot snapshot = collector.collect(); - if (snapshot != null) { - prometheusName = snapshot.getMetadata().getPrometheusName(); - metricType = SnapshotRegistrationExtractor.metricTypeFromSnapshot(snapshot); - labelNames = SnapshotRegistrationExtractor.labelNamesFromSnapshot(snapshot); - MetricMetadata snapshotMetadata = - SnapshotRegistrationExtractor.metadataFromSnapshot(snapshot); - help = snapshotMetadata.getHelp(); - unit = snapshotMetadata.getUnit(); - } - } - + // Only perform validation if collector provides sufficient metadata. + // Collectors that don't implement getPrometheusName()/getMetricType() will skip validation. if (prometheusName != null && metricType != null) { final String name = prometheusName; final MetricType type = metricType; diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/SnapshotRegistrationExtractor.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/SnapshotRegistrationExtractor.java deleted file mode 100644 index 746103fd7..000000000 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/SnapshotRegistrationExtractor.java +++ /dev/null @@ -1,78 +0,0 @@ -package io.prometheus.metrics.model.registry; - -import io.prometheus.metrics.model.snapshots.CounterSnapshot; -import io.prometheus.metrics.model.snapshots.GaugeSnapshot; -import io.prometheus.metrics.model.snapshots.HistogramSnapshot; -import io.prometheus.metrics.model.snapshots.InfoSnapshot; -import io.prometheus.metrics.model.snapshots.Labels; -import io.prometheus.metrics.model.snapshots.MetricMetadata; -import io.prometheus.metrics.model.snapshots.MetricSnapshot; -import io.prometheus.metrics.model.snapshots.StateSetSnapshot; -import io.prometheus.metrics.model.snapshots.SummarySnapshot; -import io.prometheus.metrics.model.snapshots.UnknownSnapshot; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; - -/** - * Extracts registration-relevant data (metric type, label names) from a {@link MetricSnapshot}. - * Used when a collector does not implement {@link Collector#getMetricType()} / {@link - * Collector#getLabelNames()} so the registry can validate from a one-time {@link - * Collector#collect()} at registration. - * - *

Collectors whose {@code collect()} has side effects (e.g. callbacks) should implement the - * getters so the registry does not call {@code collect()} at registration. - */ -final class SnapshotRegistrationExtractor { - - private SnapshotRegistrationExtractor() {} - - static MetricType metricTypeFromSnapshot(MetricSnapshot snapshot) { - if (snapshot instanceof CounterSnapshot) { - return MetricType.COUNTER; - } - if (snapshot instanceof GaugeSnapshot) { - return MetricType.GAUGE; - } - if (snapshot instanceof HistogramSnapshot) { - return MetricType.HISTOGRAM; - } - if (snapshot instanceof SummarySnapshot) { - return MetricType.SUMMARY; - } - if (snapshot instanceof InfoSnapshot) { - return MetricType.INFO; - } - if (snapshot instanceof StateSetSnapshot) { - return MetricType.STATESET; - } - if (snapshot instanceof UnknownSnapshot) { - return MetricType.UNKNOWN; - } - return MetricType.UNKNOWN; - } - - /** - * Returns the set of label names (Prometheus-normalized) from the snapshot's first data point, or - * empty set if there are no data points. - */ - static Set labelNamesFromSnapshot(MetricSnapshot snapshot) { - if (snapshot.getDataPoints().isEmpty()) { - return Collections.emptySet(); - } - Labels labels = snapshot.getDataPoints().get(0).getLabels(); - if (labels.isEmpty()) { - return Collections.emptySet(); - } - Set names = new HashSet<>(); - for (int i = 0; i < labels.size(); i++) { - names.add(labels.getPrometheusName(i)); - } - return names; - } - - /** Returns metadata from the snapshot for help/unit validation (snapshot always has metadata). */ - static MetricMetadata metadataFromSnapshot(MetricSnapshot snapshot) { - return snapshot.getMetadata(); - } -} diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index 7c53f358f..f95d305f8 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -257,11 +257,11 @@ public Set getLabelNames() { } @Test - public void register_nullTypeAndLabels_fallbackToCollect_validatesFromSnapshot() { + public void register_nullType_skipsValidation() { PrometheusRegistry registry = new PrometheusRegistry(); - // Collector without getMetricType() and getLabelNames() - returns null (default). - // Registry falls back to collect() and derives type/labels from snapshot. + // Collectors without getMetricType() skip registration-time validation. + // This allows legacy collectors to work without implementing all getters. Collector legacyCollector1 = new Collector() { @Override @@ -288,11 +288,9 @@ public String getPrometheusName() { } }; - registry.register(legacyCollector1); - // Second collector has same name but different type (from snapshot) - should fail - assertThatThrownBy(() -> registry.register(legacyCollector2)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Conflicting metric types"); + // Both collectors can register successfully since validation is skipped + assertThatCode(() -> registry.register(legacyCollector1)).doesNotThrowAnyException(); + assertThatCode(() -> registry.register(legacyCollector2)).doesNotThrowAnyException(); } @Test From 74a46f35b5cc8513f685f68200783b1b4e8c17a9 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Fri, 30 Jan 2026 12:23:37 -0500 Subject: [PATCH 12/23] add tests Signed-off-by: Jay DeLuca --- .../registry/PrometheusRegistryTest.java | 210 ++++++++++++++++++ 1 file changed, 210 insertions(+) diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index f95d305f8..126f29b10 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -710,4 +710,214 @@ public MetricMetadata getMetadata() { registry.register(withPath); assertThatCode(() -> registry.register(withStatus)).doesNotThrowAnyException(); } + + @Test + public void register_sameName_oneNullHelp_allowed() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector withHelp = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").help("Total requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("path")); + } + + @Override + public MetricMetadata getMetadata() { + return new MetricMetadata("requests", "Total requests", null); + } + }; + + Collector withoutHelp = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("status")); + } + + @Override + public MetricMetadata getMetadata() { + return new MetricMetadata("requests", null, null); + } + }; + + registry.register(withHelp); + // One has help, one doesn't - should be allowed + assertThatCode(() -> registry.register(withoutHelp)).doesNotThrowAnyException(); + } + + @Test + public void unregister_lastCollector_removesPrometheusName() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector counter1 = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("path")); + } + }; + + Collector counter2 = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("status")); + } + }; + + registry.register(counter1); + registry.register(counter2); + + // Unregister first collector - name should still be registered + registry.unregister(counter1); + MetricSnapshots snapshots = registry.scrape(); + assertThat(snapshots.size()).isEqualTo(1); + + // Unregister second collector - name should be removed + registry.unregister(counter2); + snapshots = registry.scrape(); + assertThat(snapshots.size()).isEqualTo(0); + + // Should be able to register again with same name + assertThatCode(() -> registry.register(counter1)).doesNotThrowAnyException(); + } + + @Test + public void unregister_multiCollector_removesAllLabelSchemas() { + PrometheusRegistry registry = new PrometheusRegistry(); + + MultiCollector multi = + new MultiCollector() { + @Override + public MetricSnapshots collect() { + return new MetricSnapshots( + CounterSnapshot.builder().name("requests").build(), + GaugeSnapshot.builder().name("connections").build()); + } + + @Override + public List getPrometheusNames() { + return asList("requests", "connections"); + } + + @Override + public MetricType getMetricType(String prometheusName) { + return prometheusName.equals("requests") ? MetricType.COUNTER : MetricType.GAUGE; + } + }; + + registry.register(multi); + assertThat(registry.scrape().size()).isEqualTo(2); + + registry.unregister(multi); + assertThat(registry.scrape().size()).isEqualTo(0); + + // Should be able to register collectors with same names again + Collector counter = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + }; + + assertThatCode(() -> registry.register(counter)).doesNotThrowAnyException(); + } + + @Test + public void unregister_legacyCollector_noErrors() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector legacy = + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder().name("legacy_metric").build(); + } + + @Override + public String getPrometheusName() { + return "legacy_metric"; + } + // No getMetricType() - returns null + }; + + registry.register(legacy); + assertThat(registry.scrape().size()).isEqualTo(1); + + // Unregister should work without errors even for legacy collectors + assertThatCode(() -> registry.unregister(legacy)).doesNotThrowAnyException(); + assertThat(registry.scrape().size()).isEqualTo(0); + } } From dfa2114d0641c65f4d0769cc4f1260257403c972 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Fri, 30 Jan 2026 12:27:28 -0500 Subject: [PATCH 13/23] test visibility Signed-off-by: Jay DeLuca --- .../io/prometheus/metrics/core/metrics/CounterTest.java | 4 ++-- .../metrics/expositionformats/TextFormatUtilTest.java | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterTest.java b/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterTest.java index a45b5f578..80a210ea3 100644 --- a/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterTest.java +++ b/prometheus-metrics-core/src/test/java/io/prometheus/metrics/core/metrics/CounterTest.java @@ -116,7 +116,7 @@ void testLabels() { "my_counter", "my_counter_seconds", }) - public void testTotalStrippedFromName(String name) { + void testTotalStrippedFromName(String name) { Counter counter = Counter.builder().name(name).unit(Unit.SECONDS).build(); Metrics.MetricFamily protobufData = new PrometheusProtobufWriterImpl().convert(counter.collect(), EscapingScheme.ALLOW_UTF8); @@ -380,7 +380,7 @@ void testConstLabelsSecond() { } @Test - public void testLabelNormalizationInRegistration() { + void testLabelNormalizationInRegistration() { PrometheusRegistry registry = new PrometheusRegistry(); Counter.builder().name("requests").labelNames("request.count").register(registry); diff --git a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java index e8571bac1..3fe1f4bee 100644 --- a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java +++ b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java @@ -39,7 +39,7 @@ private static String writePrometheusTimestamp(boolean timestampsInMs) throws IO } @Test - public void testMergeDuplicates_sameName_mergesDataPoints() { + void testMergeDuplicates_sameName_mergesDataPoints() { CounterSnapshot counter1 = CounterSnapshot.builder() .name("api_responses") @@ -81,7 +81,7 @@ public void testMergeDuplicates_sameName_mergesDataPoints() { } @Test - public void testMergeDuplicates_multipleDataPoints_allMerged() { + void testMergeDuplicates_multipleDataPoints_allMerged() { CounterSnapshot counter1 = CounterSnapshot.builder() .name("api_responses") @@ -120,7 +120,7 @@ public void testMergeDuplicates_multipleDataPoints_allMerged() { } @Test - public void testMergeDuplicates_emptySnapshots_returnsEmpty() { + void testMergeDuplicates_emptySnapshots_returnsEmpty() { MetricSnapshots snapshots = MetricSnapshots.builder().build(); MetricSnapshots result = TextFormatUtil.mergeDuplicates(snapshots); From cbcd1ec7a4503474f6737feb4ef7a49c28957eb8 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Fri, 30 Jan 2026 12:41:00 -0500 Subject: [PATCH 14/23] cleanup Signed-off-by: Jay DeLuca --- .../model/registry/PrometheusRegistry.java | 24 ++++--------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 2df368150..f1f94e56e 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -29,12 +29,10 @@ public class PrometheusRegistry { /** Stores the registration details for a Collector at registration time. */ private static class CollectorRegistration { final String prometheusName; - final MetricType metricType; final Set labelNames; - CollectorRegistration(String prometheusName, MetricType metricType, Set labelNames) { + CollectorRegistration(String prometheusName, @Nullable Set labelNames) { this.prometheusName = prometheusName; - this.metricType = metricType; this.labelNames = (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; } @@ -46,13 +44,10 @@ private static class CollectorRegistration { */ private static class MultiCollectorRegistration { final String prometheusName; - final MetricType metricType; final Set labelNames; - MultiCollectorRegistration( - String prometheusName, MetricType metricType, Set labelNames) { + MultiCollectorRegistration(String prometheusName, @Nullable Set labelNames) { this.prometheusName = prometheusName; - this.metricType = metricType; this.labelNames = (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; } @@ -138,16 +133,6 @@ boolean isEmpty() { MetricType getType() { return type; } - - @Nullable - String getHelp() { - return help; - } - - @Nullable - Unit getUnit() { - return unit; - } } public void register(Collector collector) { @@ -195,8 +180,7 @@ public void register(Collector collector) { } }); - collectorMetadata.put( - collector, new CollectorRegistration(prometheusName, metricType, labelNames)); + collectorMetadata.put(collector, new CollectorRegistration(prometheusName, labelNames)); } if (prometheusName != null) { @@ -256,7 +240,7 @@ public void register(MultiCollector collector) { } }); - registrations.add(new MultiCollectorRegistration(prometheusName, metricType, labelNames)); + registrations.add(new MultiCollectorRegistration(prometheusName, labelNames)); } prometheusNames.add(prometheusName); From 1e897dc74c338804c765bcc65645c5faa27b2afa Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Fri, 30 Jan 2026 12:51:57 -0500 Subject: [PATCH 15/23] add otel sdk compatibility test Signed-off-by: Jay DeLuca --- ...etryExporterRegistryCompatibilityTest.java | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/OpenTelemetryExporterRegistryCompatibilityTest.java diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/OpenTelemetryExporterRegistryCompatibilityTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/OpenTelemetryExporterRegistryCompatibilityTest.java new file mode 100644 index 000000000..166b374b8 --- /dev/null +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/OpenTelemetryExporterRegistryCompatibilityTest.java @@ -0,0 +1,116 @@ +package io.prometheus.metrics.model.registry; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; + +import io.prometheus.metrics.model.snapshots.CounterSnapshot; +import io.prometheus.metrics.model.snapshots.GaugeSnapshot; +import io.prometheus.metrics.model.snapshots.MetricSnapshot; +import io.prometheus.metrics.model.snapshots.MetricSnapshots; +import java.util.Collections; +import org.junit.jupiter.api.Test; + +/** + * Tests that use the Prometheus registry in the same way as the OpenTelemetry Java SDK Prometheus + * exporter ({@code io.opentelemetry.exporter.prometheus}). The SDK's {@code PrometheusMetricReader} + * implements {@link MultiCollector} with default implementations for all optional methods: {@link + * MultiCollector#getPrometheusNames()} returns an empty list, and {@link + * MultiCollector#getMetricType(String)}, {@link MultiCollector#getLabelNames(String)}, and {@link + * MultiCollector#getMetadata(String)} return null. This test suite ensures that registration, + * scrape, and unregister continue to work for that usage pattern and that a shared registry with + * both SDK-style and validated collectors behaves correctly. + */ +class OpenTelemetryExporterRegistryCompatibilityTest { + + /** + * A MultiCollector that mimics the OpenTelemetry Java SDK's PrometheusMetricReader: it does not + * override getPrometheusNames() (empty list), getMetricType(String), getLabelNames(String), or + * getMetadata(String) (all null). Only collect() is implemented and returns MetricSnapshots. + */ + private static final MultiCollector OTEL_STYLE_MULTI_COLLECTOR = + new MultiCollector() { + @Override + public MetricSnapshots collect() { + return new MetricSnapshots( + CounterSnapshot.builder() + .name("otel_metric") + .help("A metric produced by an OTel-style converter") + .dataPoint(CounterSnapshot.CounterDataPointSnapshot.builder().value(42.0).build()) + .build()); + } + }; + + @Test + void registerOtelStyleMultiCollector_succeeds() { + PrometheusRegistry registry = new PrometheusRegistry(); + + assertThatCode(() -> registry.register(OTEL_STYLE_MULTI_COLLECTOR)).doesNotThrowAnyException(); + } + + @Test + void scrape_afterRegisteringOtelStyleMultiCollector_returnsSnapshotsFromCollector() { + PrometheusRegistry registry = new PrometheusRegistry(); + registry.register(OTEL_STYLE_MULTI_COLLECTOR); + + MetricSnapshots snapshots = registry.scrape(); + + assertThat(snapshots).hasSize(1); + MetricSnapshot snapshot = snapshots.get(0); + assertThat(snapshot.getMetadata().getPrometheusName()).isEqualTo("otel_metric"); + } + + @Test + void unregisterOtelStyleMultiCollector_succeedsAndScrapeNoLongerIncludesIt() { + PrometheusRegistry registry = new PrometheusRegistry(); + registry.register(OTEL_STYLE_MULTI_COLLECTOR); + + assertThat(registry.scrape()).hasSize(1); + + assertThatCode(() -> registry.unregister(OTEL_STYLE_MULTI_COLLECTOR)) + .doesNotThrowAnyException(); + + assertThat(registry.scrape()).isEmpty(); + } + + @Test + void sharedRegistry_otelStyleMultiCollectorAndValidatedCollector_bothParticipateInScrape() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector validatedCollector = + new Collector() { + @Override + public MetricSnapshot collect() { + return GaugeSnapshot.builder().name("app_gauge").help("App gauge").build(); + } + + @Override + public String getPrometheusName() { + return "app_gauge"; + } + + @Override + public MetricType getMetricType() { + return MetricType.GAUGE; + } + + @Override + public java.util.Set getLabelNames() { + return Collections.emptySet(); + } + }; + + registry.register(validatedCollector); + registry.register(OTEL_STYLE_MULTI_COLLECTOR); + + MetricSnapshots snapshots = registry.scrape(); + + assertThat(snapshots).hasSize(2); + assertThat(snapshots) + .extracting(s -> s.getMetadata().getPrometheusName()) + .containsExactlyInAnyOrder("app_gauge", "otel_metric"); + + registry.unregister(OTEL_STYLE_MULTI_COLLECTOR); + assertThat(registry.scrape()).hasSize(1); + assertThat(registry.scrape().get(0).getMetadata().getPrometheusName()).isEqualTo("app_gauge"); + } +} From 707e4dc387d2460a0412ba3f7d66c0745d13d2b2 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Mon, 2 Feb 2026 10:14:04 -0500 Subject: [PATCH 16/23] make copys of labels, fix multicollector processing with try catch, fix build order to avoid local issues Signed-off-by: Jay DeLuca --- docs/content/getting-started/metric-types.md | 2 +- prometheus-metrics-core/pom.xml | 4 +- .../pom.xml | 15 ++- .../generate-protobuf.sh | 23 ++-- .../model/registry/PrometheusRegistry.java | 125 ++++++++++-------- 5 files changed, 100 insertions(+), 69 deletions(-) diff --git a/docs/content/getting-started/metric-types.md b/docs/content/getting-started/metric-types.md index cf3689a75..05a5d4949 100644 --- a/docs/content/getting-started/metric-types.md +++ b/docs/content/getting-started/metric-types.md @@ -278,4 +278,4 @@ To use these types, you need to implement your own `Collector` where the `collec an `UnknownSnapshot` or a `HistogramSnapshot` with `.gaugeHistogram(true)`. If your custom collector does not implement `getMetricType()` and `getLabelNames()`, ensure it does not produce the same metric name and label set as another collector, or the exposition may contain -duplicate time series. +duplicate time series. \ No newline at end of file diff --git a/prometheus-metrics-core/pom.xml b/prometheus-metrics-core/pom.xml index 3a61050de..b3a043574 100644 --- a/prometheus-metrics-core/pom.xml +++ b/prometheus-metrics-core/pom.xml @@ -38,10 +38,10 @@ ${project.version} - + io.prometheus - prometheus-metrics-exposition-formats + prometheus-metrics-exposition-formats-no-protobuf ${project.version} test diff --git a/prometheus-metrics-exposition-formats-shaded/pom.xml b/prometheus-metrics-exposition-formats-shaded/pom.xml index 64e033366..2b938b761 100644 --- a/prometheus-metrics-exposition-formats-shaded/pom.xml +++ b/prometheus-metrics-exposition-formats-shaded/pom.xml @@ -23,6 +23,13 @@ + + + io.prometheus + prometheus-metrics-exposition-formats-no-protobuf + ${project.version} + provided + io.prometheus prometheus-metrics-exposition-textformats @@ -69,22 +76,26 @@ copy-metrics-exposition-formats-main - validate + generate-sources copy-resources target/metrics-exposition-formats/src/main + true ../prometheus-metrics-exposition-formats/src/main + + **/* + copy-metrics-exposition-formats-test - validate + generate-sources copy-resources diff --git a/prometheus-metrics-exposition-formats/generate-protobuf.sh b/prometheus-metrics-exposition-formats/generate-protobuf.sh index ed7b4aafd..b5493ae2a 100755 --- a/prometheus-metrics-exposition-formats/generate-protobuf.sh +++ b/prometheus-metrics-exposition-formats/generate-protobuf.sh @@ -6,17 +6,15 @@ set -euo pipefail # I could not figure out how to use a protoc Maven plugin to use the shaded module, # so I ran this command to generate the sources manually. -# Use gsed and ggrep on macOS (requires: brew install gnu-sed grep) +# Use gsed on macOS (requires: brew install gnu-sed) for in-place edits +# BSD sed requires -i '' for in-place with no backup; GNU sed uses -i alone. if [[ "$OSTYPE" == "darwin"* ]] && command -v gsed >/dev/null 2>&1; then SED='gsed' + SED_I=(-i) else SED='sed' -fi - -if [[ "$OSTYPE" == "darwin"* ]] && command -v ggrep >/dev/null 2>&1; then - GREP='ggrep' -else - GREP='grep' + # BSD sed: -i requires backup extension; '' = no backup + [[ "$OSTYPE" == "darwin"* ]] && SED_I=(-i '') || SED_I=(-i) fi # Use mise-provided protoc if available @@ -43,17 +41,18 @@ PACKAGE="io.prometheus.metrics.expositionformats.generated.com_google_protobuf_$ if [[ $OLD_PACKAGE != "$PACKAGE" ]]; then echo "Replacing package $OLD_PACKAGE with $PACKAGE in all java files" - find .. -type f -name "*.java" -exec $SED -i "s/$OLD_PACKAGE/$PACKAGE/g" {} + + find .. -type f -name "*.java" -exec "${SED}" "${SED_I[@]}" "s/$OLD_PACKAGE/$PACKAGE/g" {} + fi curl -sL https://raw.githubusercontent.com/prometheus/client_model/master/io/prometheus/client/metrics.proto -o $PROTO_DIR/metrics.proto -$SED -i "s/java_package = \"io.prometheus.client\"/java_package = \"$PACKAGE\"/" $PROTO_DIR/metrics.proto +"${SED}" "${SED_I[@]}" "s/java_package = \"io.prometheus.client\"/java_package = \"$PACKAGE\"/" $PROTO_DIR/metrics.proto $PROTOC --java_out "$TARGET_DIR" $PROTO_DIR/metrics.proto -$SED -i '1 i\//CHECKSTYLE:OFF: checkstyle' "$(find src/main/generated/io -type f)" -$SED -i -e $'$a\\\n//CHECKSTYLE:ON: checkstyle' "$(find src/main/generated/io -type f)" +for f in $(find src/main/generated/io -type f); do "${SED}" "${SED_I[@]}" '1 i\ +//CHECKSTYLE:OFF: checkstyle' "$f"; done +for f in $(find src/main/generated/io -type f); do "${SED}" "${SED_I[@]}" -e $'$a\\\n//CHECKSTYLE:ON: checkstyle' "$f"; done -GENERATED_WITH=$($GREP -oP '\/\/ Protobuf Java Version: \K.*' "$TARGET_DIR/${PACKAGE//\.//}"/Metrics.java) +GENERATED_WITH=$($SED -n 's/.*\/\/ Protobuf Java Version: \(.*\)/\1/p' "$TARGET_DIR/${PACKAGE//\.//}"/Metrics.java) function help() { echo "Please use https://mise.jdx.dev/ - this will use the version specified in mise.toml" diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index f1f94e56e..6b5c0ab01 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -6,6 +6,7 @@ import io.prometheus.metrics.model.snapshots.Unit; import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Set; @@ -135,6 +136,18 @@ MetricType getType() { } } + /** + * Returns an immutable set of label names for storage. Defends against mutation of the set + * returned by {@code Collector.getLabelNames()} after registration, which would break duplicate + * detection and unregistration. + */ + private static Set immutableLabelNames(@Nullable Set labelNames) { + if (labelNames == null || labelNames.isEmpty()) { + return Collections.emptySet(); + } + return Collections.unmodifiableSet(new HashSet<>(labelNames)); + } + public void register(Collector collector) { if (collectors.contains(collector)) { throw new IllegalArgumentException("Collector instance is already registered"); @@ -142,7 +155,7 @@ public void register(Collector collector) { String prometheusName = collector.getPrometheusName(); MetricType metricType = collector.getMetricType(); - Set labelNames = collector.getLabelNames(); + Set normalizedLabels = immutableLabelNames(collector.getLabelNames()); MetricMetadata metadata = collector.getMetadata(); String help = metadata != null ? metadata.getHelp() : null; Unit unit = metadata != null ? metadata.getUnit() : null; @@ -152,7 +165,7 @@ public void register(Collector collector) { if (prometheusName != null && metricType != null) { final String name = prometheusName; final MetricType type = metricType; - final Set names = labelNames; + final Set names = normalizedLabels; final String helpForValidation = help; final Unit unitForValidation = unit; registered.compute( @@ -171,16 +184,14 @@ public void register(Collector collector) { } existingInfo.validateMetadata(helpForValidation, unitForValidation); if (!existingInfo.addLabelSet(names)) { - Set normalized = - (names == null || names.isEmpty()) ? Collections.emptySet() : names; throw new IllegalArgumentException( - name + ": duplicate metric name with identical label schema " + normalized); + name + ": duplicate metric name with identical label schema " + names); } return existingInfo; } }); - collectorMetadata.put(collector, new CollectorRegistration(prometheusName, labelNames)); + collectorMetadata.put(collector, new CollectorRegistration(prometheusName, normalizedLabels)); } if (prometheusName != null) { @@ -197,57 +208,67 @@ public void register(MultiCollector collector) { List prometheusNamesList = collector.getPrometheusNames(); List registrations = new ArrayList<>(); - - for (String prometheusName : prometheusNamesList) { - MetricType metricType = collector.getMetricType(prometheusName); - Set labelNames = collector.getLabelNames(prometheusName); - MetricMetadata metadata = collector.getMetadata(prometheusName); - String help = metadata != null ? metadata.getHelp() : null; - Unit unit = metadata != null ? metadata.getUnit() : null; - - if (metricType != null) { - final MetricType type = metricType; - final Set labelNamesForValidation = labelNames; - final String helpForValidation = help; - final Unit unitForValidation = unit; - registered.compute( - prometheusName, - (name, existingInfo) -> { - if (existingInfo == null) { - return RegistrationInfo.of( - type, labelNamesForValidation, helpForValidation, unitForValidation); - } else { - if (existingInfo.getType() != type) { - throw new IllegalArgumentException( - prometheusName - + ": Conflicting metric types. Existing: " - + existingInfo.getType() - + ", new: " - + type); - } - existingInfo.validateMetadata(helpForValidation, unitForValidation); - if (!existingInfo.addLabelSet(labelNamesForValidation)) { - Set normalized = - (labelNamesForValidation == null || labelNamesForValidation.isEmpty()) - ? Collections.emptySet() - : labelNamesForValidation; - throw new IllegalArgumentException( - prometheusName - + ": duplicate metric name with identical label schema " - + normalized); + Set namesOnlyInPrometheusNames = new HashSet<>(); + + try { + for (String prometheusName : prometheusNamesList) { + MetricType metricType = collector.getMetricType(prometheusName); + Set normalizedLabels = immutableLabelNames(collector.getLabelNames(prometheusName)); + MetricMetadata metadata = collector.getMetadata(prometheusName); + String help = metadata != null ? metadata.getHelp() : null; + Unit unit = metadata != null ? metadata.getUnit() : null; + + if (metricType != null) { + final MetricType type = metricType; + final Set labelNamesForValidation = normalizedLabels; + final String helpForValidation = help; + final Unit unitForValidation = unit; + registered.compute( + prometheusName, + (name, existingInfo) -> { + if (existingInfo == null) { + return RegistrationInfo.of( + type, labelNamesForValidation, helpForValidation, unitForValidation); + } else { + if (existingInfo.getType() != type) { + throw new IllegalArgumentException( + prometheusName + + ": Conflicting metric types. Existing: " + + existingInfo.getType() + + ", new: " + + type); + } + existingInfo.validateMetadata(helpForValidation, unitForValidation); + if (!existingInfo.addLabelSet(labelNamesForValidation)) { + throw new IllegalArgumentException( + prometheusName + + ": duplicate metric name with identical label schema " + + labelNamesForValidation); + } + return existingInfo; } - return existingInfo; - } - }); + }); - registrations.add(new MultiCollectorRegistration(prometheusName, labelNames)); + registrations.add(new MultiCollectorRegistration(prometheusName, normalizedLabels)); + } + + boolean addedToPrometheusNames = prometheusNames.add(prometheusName); + if (addedToPrometheusNames && metricType == null) { + namesOnlyInPrometheusNames.add(prometheusName); + } } - prometheusNames.add(prometheusName); + multiCollectorMetadata.put(collector, registrations); + multiCollectors.add(collector); + } catch (Exception e) { + for (MultiCollectorRegistration registration : registrations) { + unregisterLabelSchema(registration.prometheusName, registration.labelNames); + } + for (String name : namesOnlyInPrometheusNames) { + prometheusNames.remove(name); + } + throw e; } - - multiCollectorMetadata.put(collector, registrations); - multiCollectors.add(collector); } public void unregister(Collector collector) { From 73b47bdc47c944a5056e4ff2859811448cf945ff Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Mon, 2 Feb 2026 10:33:26 -0500 Subject: [PATCH 17/23] fix histogram detection when merging snapshots, ensure created with timestamps logic includes merging Signed-off-by: Jay DeLuca --- .../PrometheusTextFormatWriter.java | 2 +- .../expositionformats/TextFormatUtil.java | 10 ++ .../DuplicateNamesExpositionTest.java | 75 +++++++++++ .../expositionformats/TextFormatUtilTest.java | 37 ++++++ .../model/registry/PrometheusRegistry.java | 21 ++-- .../model/snapshots/MetricSnapshots.java | 16 ++- .../registry/PrometheusRegistryTest.java | 119 +++++++++++++++--- .../model/snapshots/MetricSnapshotsTest.java | 28 +++++ 8 files changed, 283 insertions(+), 25 deletions(-) diff --git a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/PrometheusTextFormatWriter.java b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/PrometheusTextFormatWriter.java index 861ebcf75..cc9f067ba 100644 --- a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/PrometheusTextFormatWriter.java +++ b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/PrometheusTextFormatWriter.java @@ -137,7 +137,7 @@ public void write(OutputStream out, MetricSnapshots metricSnapshots, EscapingSch } } if (writeCreatedTimestamps) { - for (MetricSnapshot s : metricSnapshots) { + for (MetricSnapshot s : merged) { MetricSnapshot snapshot = escapeMetricSnapshot(s, scheme); if (!snapshot.getDataPoints().isEmpty()) { if (snapshot instanceof CounterSnapshot) { diff --git a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java index 2dcaf9013..e5cbe8b40 100644 --- a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java +++ b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java @@ -227,6 +227,14 @@ private static MetricSnapshot mergeSnapshots(List snapshots) { + " and " + snapshot.getClass().getName()); } + if (first instanceof HistogramSnapshot) { + HistogramSnapshot histogramFirst = (HistogramSnapshot) first; + HistogramSnapshot histogramSnapshot = (HistogramSnapshot) snapshot; + if (histogramFirst.isGaugeHistogram() != histogramSnapshot.isGaugeHistogram()) { + throw new IllegalArgumentException( + "Cannot merge histograms: gauge histogram and classic histogram"); + } + } totalDataPoints += snapshot.getDataPoints().size(); } @@ -244,7 +252,9 @@ private static MetricSnapshot mergeSnapshots(List snapshots) { first.getMetadata(), (Collection) (Object) allDataPoints); } else if (first instanceof HistogramSnapshot) { + HistogramSnapshot histFirst = (HistogramSnapshot) first; return new HistogramSnapshot( + histFirst.isGaugeHistogram(), first.getMetadata(), (Collection) (Object) allDataPoints); } else if (first instanceof SummarySnapshot) { diff --git a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java index 038e26327..31b029fa5 100644 --- a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java +++ b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java @@ -180,4 +180,79 @@ void testOpenMetricsFormat_withDuplicateNames() throws IOException { """; assertThat(output).isEqualTo(expected); } + + @Test + void testDuplicateNames_withCreatedTimestamps_emitsSingleHelpTypeAndNoDuplicateCreatedSeries() + throws IOException { + long createdTs = 1672850385800L; + PrometheusRegistry registry = new PrometheusRegistry(); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels(Labels.of("uri", "/hello", "outcome", "SUCCESS")) + .value(100) + .createdTimestampMillis(createdTs) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + + registry.register( + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder() + .name("api_responses") + .help("API responses") + .dataPoint( + CounterSnapshot.CounterDataPointSnapshot.builder() + .labels( + Labels.of("uri", "/hello", "outcome", "FAILURE", "error", "TIMEOUT")) + .value(10) + .createdTimestampMillis(createdTs + 1000) + .build()) + .build(); + } + + @Override + public String getPrometheusName() { + return "api_responses_total"; + } + }); + + MetricSnapshots snapshots = registry.scrape(); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + PrometheusTextFormatWriter writer = + PrometheusTextFormatWriter.builder().setIncludeCreatedTimestamps(true).build(); + writer.write(out, snapshots); + String output = out.toString(UTF_8); + + // Merged snapshots: one metric family with two data points. Created-timestamp section uses + // merged snapshots too, so single HELP/TYPE for _created and one _created line per label set. + String expected = + """ + # HELP api_responses_total API responses + # TYPE api_responses_total counter + api_responses_total{error="TIMEOUT",outcome="FAILURE",uri="/hello"} 10.0 + api_responses_total{outcome="SUCCESS",uri="/hello"} 100.0 + # HELP api_responses_created API responses + # TYPE api_responses_created gauge + api_responses_created{error="TIMEOUT",outcome="FAILURE",uri="/hello"} 1672850386800 + api_responses_created{outcome="SUCCESS",uri="/hello"} 1672850385800 + """; + + assertThat(output).isEqualTo(expected); + } } diff --git a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java index 3fe1f4bee..3a6fea740 100644 --- a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java +++ b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/TextFormatUtilTest.java @@ -3,7 +3,9 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import io.prometheus.metrics.model.snapshots.ClassicHistogramBuckets; import io.prometheus.metrics.model.snapshots.CounterSnapshot; +import io.prometheus.metrics.model.snapshots.HistogramSnapshot; import io.prometheus.metrics.model.snapshots.Labels; import io.prometheus.metrics.model.snapshots.MetricSnapshots; import java.io.IOException; @@ -126,4 +128,39 @@ void testMergeDuplicates_emptySnapshots_returnsEmpty() { assertThat(result).isEmpty(); } + + @Test + void testMergeDuplicates_histogramSameGaugeFlag_preservesGaugeHistogram() { + HistogramSnapshot gauge1 = + HistogramSnapshot.builder() + .name("my_histogram") + .gaugeHistogram(true) + .dataPoint( + HistogramSnapshot.HistogramDataPointSnapshot.builder() + .labels(Labels.of("a", "1")) + .classicHistogramBuckets( + ClassicHistogramBuckets.of( + new double[] {Double.POSITIVE_INFINITY}, new long[] {0})) + .build()) + .build(); + HistogramSnapshot gauge2 = + HistogramSnapshot.builder() + .name("my_histogram") + .gaugeHistogram(true) + .dataPoint( + HistogramSnapshot.HistogramDataPointSnapshot.builder() + .labels(Labels.of("a", "2")) + .classicHistogramBuckets( + ClassicHistogramBuckets.of( + new double[] {Double.POSITIVE_INFINITY}, new long[] {0})) + .build()) + .build(); + MetricSnapshots snapshots = new MetricSnapshots(gauge1, gauge2); + MetricSnapshots result = TextFormatUtil.mergeDuplicates(snapshots); + + assertThat(result).hasSize(1); + HistogramSnapshot merged = (HistogramSnapshot) result.get(0); + assertThat(merged.isGaugeHistogram()).isTrue(); + assertThat(merged.getDataPoints()).hasSize(2); + } } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 6b5c0ab01..6c9adadb2 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -34,8 +34,7 @@ private static class CollectorRegistration { CollectorRegistration(String prometheusName, @Nullable Set labelNames) { this.prometheusName = prometheusName; - this.labelNames = - (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; + this.labelNames = immutableLabelNames(labelNames); } } @@ -49,8 +48,7 @@ private static class MultiCollectorRegistration { MultiCollectorRegistration(String prometheusName, @Nullable Set labelNames) { this.prometheusName = prometheusName; - this.labelNames = - (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; + this.labelNames = immutableLabelNames(labelNames); } } @@ -62,8 +60,8 @@ private static class MultiCollectorRegistration { private static class RegistrationInfo { private final MetricType type; private final Set> labelSchemas; - @Nullable private final String help; - @Nullable private final Unit unit; + @Nullable private String help; + @Nullable private Unit unit; private RegistrationInfo( MetricType type, @@ -89,8 +87,9 @@ static RegistrationInfo of( } /** - * Validates that the given help and unit are consistent with this registration. Throws if both - * sides have non-null help/unit and they differ. + * Validates that the given help and unit are consistent with this registration. Throws if + * non-null values conflict. When stored help/unit is null and the new value is non-null, + * captures the first non-null so subsequent registrations are validated consistently. */ void validateMetadata(@Nullable String newHelp, @Nullable Unit newUnit) { if (help != null && newHelp != null && !Objects.equals(help, newHelp)) { @@ -101,6 +100,12 @@ void validateMetadata(@Nullable String newHelp, @Nullable Unit newUnit) { throw new IllegalArgumentException( "Conflicting unit. Existing: " + unit + ", new: " + newUnit); } + if (help == null && newHelp != null) { + this.help = newHelp; + } + if (unit == null && newUnit != null) { + this.unit = newUnit; + } } /** diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java index 091e9cd65..949c65f91 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java @@ -41,8 +41,10 @@ public MetricSnapshots(Collection snapshots) { String name2 = list.get(i + 1).getMetadata().getPrometheusName(); if (name1.equals(name2)) { - Class type1 = list.get(i).getClass(); - Class type2 = list.get(i + 1).getClass(); + MetricSnapshot s1 = list.get(i); + MetricSnapshot s2 = list.get(i + 1); + Class type1 = s1.getClass(); + Class type2 = s2.getClass(); if (!type1.equals(type2)) { throw new IllegalArgumentException( @@ -52,6 +54,16 @@ public MetricSnapshots(Collection snapshots) { + " and " + type2.getSimpleName()); } + + // HistogramSnapshot: gauge histogram vs classic histogram are semantically different + if (s1 instanceof HistogramSnapshot) { + HistogramSnapshot h1 = (HistogramSnapshot) s1; + HistogramSnapshot h2 = (HistogramSnapshot) s2; + if (h1.isGaugeHistogram() != h2.isGaugeHistogram()) { + throw new IllegalArgumentException( + name1 + ": conflicting histogram types: gauge histogram and classic histogram"); + } + } } } diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index 126f29b10..b1d8e32bf 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -143,7 +143,7 @@ public MetricType getMetricType() { } @Test - public void register_sameName_sameType_differentLabelSchemas_allowed() { + void register_sameName_sameType_differentLabelSchemas_allowed() { PrometheusRegistry registry = new PrometheusRegistry(); Collector counterWithPathLabel = @@ -199,7 +199,7 @@ public Set getLabelNames() { } @Test - public void register_sameName_sameType_sameLabelSchema_notAllowed() { + void register_sameName_sameType_sameLabelSchema_notAllowed() { PrometheusRegistry registry = new PrometheusRegistry(); Collector counter1 = @@ -257,7 +257,7 @@ public Set getLabelNames() { } @Test - public void register_nullType_skipsValidation() { + void register_nullType_skipsValidation() { PrometheusRegistry registry = new PrometheusRegistry(); // Collectors without getMetricType() skip registration-time validation. @@ -294,7 +294,7 @@ public String getPrometheusName() { } @Test - public void register_multiCollector_withTypeValidation() { + void register_multiCollector_withTypeValidation() { PrometheusRegistry registry = new PrometheusRegistry(); Collector counter = @@ -382,7 +382,7 @@ void registerOkMultiCollector() { } @Test - public void clearOk() { + void clearOk() { PrometheusRegistry registry = new PrometheusRegistry(); registry.register(counterA1); registry.register(counterB); @@ -394,7 +394,7 @@ public void clearOk() { } @Test - public void unregister_shouldRemoveLabelSchemaFromRegistrationInfo() { + void unregister_shouldRemoveLabelSchemaFromRegistrationInfo() { PrometheusRegistry registry = new PrometheusRegistry(); Collector counterWithPathLabel = @@ -475,7 +475,7 @@ public Set getLabelNames() { } @Test - public void register_withEmptyLabelSets_shouldDetectDuplicates() { + void register_withEmptyLabelSets_shouldDetectDuplicates() { PrometheusRegistry registry = new PrometheusRegistry(); Collector collector1 = @@ -527,7 +527,7 @@ public MetricType getMetricType() { } @Test - public void register_withMixedNullAndEmptyLabelSets_shouldDetectDuplicates() { + void register_withMixedNullAndEmptyLabelSets_shouldDetectDuplicates() { PrometheusRegistry registry = new PrometheusRegistry(); Collector collector1 = @@ -582,7 +582,7 @@ public MetricType getMetricType() { } @Test - public void register_sameName_differentHelp_notAllowed() { + void register_sameName_differentHelp_notAllowed() { PrometheusRegistry registry = new PrometheusRegistry(); Collector withHelpOne = @@ -648,7 +648,7 @@ public MetricMetadata getMetadata() { } @Test - public void register_sameName_sameHelpAndUnit_allowed() { + void register_sameName_sameHelpAndUnit_allowed() { PrometheusRegistry registry = new PrometheusRegistry(); Collector withPath = @@ -712,7 +712,7 @@ public MetricMetadata getMetadata() { } @Test - public void register_sameName_oneNullHelp_allowed() { + void register_sameName_oneNullHelp_allowed() { PrometheusRegistry registry = new PrometheusRegistry(); Collector withHelp = @@ -777,7 +777,98 @@ public MetricMetadata getMetadata() { } @Test - public void unregister_lastCollector_removesPrometheusName() { + void register_firstOmitsHelp_secondProvidesHelp_thirdWithDifferentHelp_throws() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector withoutHelp = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("path")); + } + }; + + Collector withHelpTotal = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").help("Total requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("status")); + } + + @Override + public MetricMetadata getMetadata() { + return new MetricMetadata("requests", "Total requests", null); + } + }; + + Collector withHelpOther = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").help("Other help").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("method")); + } + + @Override + public MetricMetadata getMetadata() { + return new MetricMetadata("requests", "Other help", null); + } + }; + + registry.register(withoutHelp); + registry.register(withHelpTotal); + // First had no help, second provided "Total requests" (captured). Third conflicts. + assertThatThrownBy(() -> registry.register(withHelpOther)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Conflicting help strings"); + } + + @Test + void unregister_lastCollector_removesPrometheusName() { PrometheusRegistry registry = new PrometheusRegistry(); Collector counter1 = @@ -844,7 +935,7 @@ public Set getLabelNames() { } @Test - public void unregister_multiCollector_removesAllLabelSchemas() { + void unregister_multiCollector_removesAllLabelSchemas() { PrometheusRegistry registry = new PrometheusRegistry(); MultiCollector multi = @@ -896,7 +987,7 @@ public MetricType getMetricType() { } @Test - public void unregister_legacyCollector_noErrors() { + void unregister_legacyCollector_noErrors() { PrometheusRegistry registry = new PrometheusRegistry(); Collector legacy = diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/MetricSnapshotsTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/MetricSnapshotsTest.java index 5d82a06a0..224ca691a 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/MetricSnapshotsTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/MetricSnapshotsTest.java @@ -61,6 +61,34 @@ void testDuplicateName() { .isThrownBy(() -> new MetricSnapshots(c, g)); } + @Test + void testDuplicateName_histogramGaugeVsClassic_throws() { + HistogramSnapshot classic = + HistogramSnapshot.builder() + .name("my_histogram") + .dataPoint( + HistogramSnapshot.HistogramDataPointSnapshot.builder() + .classicHistogramBuckets( + ClassicHistogramBuckets.of( + new double[] {Double.POSITIVE_INFINITY}, new long[] {0})) + .build()) + .build(); + HistogramSnapshot gauge = + HistogramSnapshot.builder() + .name("my_histogram") + .gaugeHistogram(true) + .dataPoint( + HistogramSnapshot.HistogramDataPointSnapshot.builder() + .classicHistogramBuckets( + ClassicHistogramBuckets.of( + new double[] {Double.POSITIVE_INFINITY}, new long[] {0})) + .build()) + .build(); + assertThatExceptionOfType(IllegalArgumentException.class) + .isThrownBy(() -> new MetricSnapshots(classic, gauge)) + .withMessageContaining("conflicting histogram types"); + } + @Test void testBuilder() { CounterSnapshot counter = From e3e0fe0fc85ac3f85334261d27d8d5c4487b5f51 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Mon, 2 Feb 2026 11:12:05 -0500 Subject: [PATCH 18/23] lint Signed-off-by: Jay DeLuca --- docs/content/getting-started/metric-types.md | 2 +- prometheus-metrics-exposition-formats/generate-protobuf.sh | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/content/getting-started/metric-types.md b/docs/content/getting-started/metric-types.md index 05a5d4949..cf3689a75 100644 --- a/docs/content/getting-started/metric-types.md +++ b/docs/content/getting-started/metric-types.md @@ -278,4 +278,4 @@ To use these types, you need to implement your own `Collector` where the `collec an `UnknownSnapshot` or a `HistogramSnapshot` with `.gaugeHistogram(true)`. If your custom collector does not implement `getMetricType()` and `getLabelNames()`, ensure it does not produce the same metric name and label set as another collector, or the exposition may contain -duplicate time series. \ No newline at end of file +duplicate time series. diff --git a/prometheus-metrics-exposition-formats/generate-protobuf.sh b/prometheus-metrics-exposition-formats/generate-protobuf.sh index b5493ae2a..7c8f8be3b 100755 --- a/prometheus-metrics-exposition-formats/generate-protobuf.sh +++ b/prometheus-metrics-exposition-formats/generate-protobuf.sh @@ -48,9 +48,9 @@ curl -sL https://raw.githubusercontent.com/prometheus/client_model/master/io/pro "${SED}" "${SED_I[@]}" "s/java_package = \"io.prometheus.client\"/java_package = \"$PACKAGE\"/" $PROTO_DIR/metrics.proto $PROTOC --java_out "$TARGET_DIR" $PROTO_DIR/metrics.proto -for f in $(find src/main/generated/io -type f); do "${SED}" "${SED_I[@]}" '1 i\ -//CHECKSTYLE:OFF: checkstyle' "$f"; done -for f in $(find src/main/generated/io -type f); do "${SED}" "${SED_I[@]}" -e $'$a\\\n//CHECKSTYLE:ON: checkstyle' "$f"; done +find src/main/generated/io -type f -exec "${SED}" "${SED_I[@]}" '1 i\ +//CHECKSTYLE:OFF: checkstyle' {} \; +find src/main/generated/io -type f -exec "${SED}" "${SED_I[@]}" -e $'$a\\\n//CHECKSTYLE:ON: checkstyle' {} \; GENERATED_WITH=$($SED -n 's/.*\/\/ Protobuf Java Version: \(.*\)/\1/p' "$TARGET_DIR/${PACKAGE//\.//}"/Metrics.java) From 64d64d47ef8faeff62c564b7001048c21f5dccd2 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Tue, 3 Feb 2026 09:55:28 -0500 Subject: [PATCH 19/23] pr review Signed-off-by: Jay DeLuca --- docs/content/getting-started/registry.md | 21 ++-- mise.toml | 2 +- .../metrics/model/registry/Collector.java | 13 ++- .../model/registry/MultiCollector.java | 14 ++- .../model/registry/PrometheusRegistry.java | 100 ++++++++++-------- .../model/snapshots/MetricSnapshots.java | 3 +- .../registry/PrometheusRegistryTest.java | 62 +++++++++++ 7 files changed, 144 insertions(+), 71 deletions(-) diff --git a/docs/content/getting-started/registry.md b/docs/content/getting-started/registry.md index f51cd521f..28857213b 100644 --- a/docs/content/getting-started/registry.md +++ b/docs/content/getting-started/registry.md @@ -6,7 +6,7 @@ weight: 2 In order to expose metrics, you need to register them with a `PrometheusRegistry`. We are using a counter as an example here, but the `register()` method is the same for all metric types. -## Registering a Metrics with the Default Registry +## Registering a Metric with the Default Registry ```java Counter eventsTotal = Counter.builder() @@ -18,7 +18,7 @@ Counter eventsTotal = Counter.builder() The `register()` call above builds the counter and registers it with the global static `PrometheusRegistry.defaultRegistry`. Using the default registry is recommended. -## Registering a Metrics with a Custom Registry +## Registering a Metric with a Custom Registry You can also register your metric with a custom registry: @@ -84,17 +84,24 @@ Validation of duplicate metric names and label schemas happens at registration t Built-in metrics (Counter, Gauge, Histogram, etc.) participate in this validation. Custom collectors that implement the `Collector` or `MultiCollector` interface can optionally -implement `getMetricType()` and `getLabelNames()` (or the MultiCollector per-name variants) so the -registry can enforce consistency. If those methods return `null`, the registry does not validate -that collector. If two such collectors produce the same metric name and same label set at scrape -time, the exposition output may contain duplicate time series and be invalid for Prometheus. +implement `getPrometheusName()` and `getMetricType()` (and the MultiCollector per-name variants) so +the registry can enforce consistency. **Validation is skipped when metric name or type is +unavailable:** if `getPrometheusName()` or `getMetricType()` returns `null`, the registry does not +validate that collector. If two such collectors produce the same metric name and same label set at +scrape time, the exposition output may contain duplicate time series and be invalid for Prometheus. + +When validation *is* performed (name and type are non-null), **null label names are treated as an +empty label schema:** `getLabelNames()` returning `null` is normalized to `Collections.emptySet()` +and full label-schema validation and duplicate detection still apply. A collector that returns a +non-null type but leaves `getLabelNames()` as `null` is still validated, with its labels treated as +empty. ## Unregistering a Metric There is no automatic expiry of unused metrics (yet), once a metric is registered it will remain registered forever. -However, you can programmatically unregistered an obsolete metric like this: +However, you can programmatically unregister an obsolete metric like this: ```java PrometheusRegistry.defaultRegistry.unregister(eventsTotal); diff --git a/mise.toml b/mise.toml index 5398f380d..9cc01deee 100644 --- a/mise.toml +++ b/mise.toml @@ -1,7 +1,7 @@ [tools] "go:github.com/gohugoio/hugo" = "v0.155.0" "go:github.com/grafana/oats" = "0.6.0" -java = "temurin-25.0.1+8.0.LTS" +java = "temurin-25.0.2+10.0.LTS" lychee = "0.22.0" protoc = "33.4" diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java index 741364fe4..dbd7c36f5 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/Collector.java @@ -109,14 +109,13 @@ default MetricType getMetricType() { * same metric name. Two collectors with the same name and type can coexist if they have different * label name sets. * - *

Returning {@code null} means label schema validation is skipped for this collector. + *

Returning {@code null} is treated as an empty label set: the registry normalizes it to + * {@code Collections.emptySet()} and performs full label-schema validation and duplicate + * detection. Two collectors with the same name, type, and {@code null} (or empty) label names are + * considered duplicate and registration of the second will fail. * - *

Validation is performed only at registration time. If this method returns {@code null}, no - * label-schema validation is performed for this collector. If such a collector produces the same - * metric name and label schema as another at scrape time, the exposition may contain duplicate - * time series, which is invalid in Prometheus. - * - * @return the set of all label names, or {@code null} to skip validation + * @return the set of all label names, or {@code null} (treated as empty) for a metric with no + * labels */ @Nullable default Set getLabelNames() { diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java index 6c4759995..e4a224bdf 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/MultiCollector.java @@ -100,16 +100,14 @@ default MetricType getMetricType(String prometheusName) { *

This is used for per-name label schema validation during registration. Two collectors with * the same name and type can coexist if they have different label name sets. * - *

Returning {@code null} means label schema validation is skipped for that specific metric - * name. - * - *

Validation is performed only at registration time. If this method returns {@code null}, no - * label-schema validation is performed for that name. If such a collector produces the same - * metric name and label schema as another at scrape time, the exposition may contain duplicate - * time series, which is invalid in Prometheus. + *

Returning {@code null} is treated as an empty label set: the registry normalizes it to + * {@code Collections.emptySet()} and performs full label-schema validation and duplicate + * detection. Two collectors with the same name, type, and {@code null} (or empty) label names are + * considered duplicate and registration of the second will fail. * * @param prometheusName the Prometheus metric name - * @return the set of all label names for the given name, or {@code null} to skip validation + * @return the set of all label names for the given name, or {@code null} (treated as empty) for a + * metric with no labels */ @Nullable default Set getLabelNames(String prometheusName) { diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 6c9adadb2..73b2d8722 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -154,63 +154,69 @@ private static Set immutableLabelNames(@Nullable Set labelNames) } public void register(Collector collector) { - if (collectors.contains(collector)) { + if (!collectors.add(collector)) { throw new IllegalArgumentException("Collector instance is already registered"); } - - String prometheusName = collector.getPrometheusName(); - MetricType metricType = collector.getMetricType(); - Set normalizedLabels = immutableLabelNames(collector.getLabelNames()); - MetricMetadata metadata = collector.getMetadata(); - String help = metadata != null ? metadata.getHelp() : null; - Unit unit = metadata != null ? metadata.getUnit() : null; - - // Only perform validation if collector provides sufficient metadata. - // Collectors that don't implement getPrometheusName()/getMetricType() will skip validation. - if (prometheusName != null && metricType != null) { - final String name = prometheusName; - final MetricType type = metricType; - final Set names = normalizedLabels; - final String helpForValidation = help; - final Unit unitForValidation = unit; - registered.compute( - prometheusName, - (n, existingInfo) -> { - if (existingInfo == null) { - return RegistrationInfo.of(type, names, helpForValidation, unitForValidation); - } else { - if (existingInfo.getType() != type) { - throw new IllegalArgumentException( - name - + ": Conflicting metric types. Existing: " - + existingInfo.getType() - + ", new: " - + type); - } - existingInfo.validateMetadata(helpForValidation, unitForValidation); - if (!existingInfo.addLabelSet(names)) { - throw new IllegalArgumentException( - name + ": duplicate metric name with identical label schema " + names); + try { + String prometheusName = collector.getPrometheusName(); + MetricType metricType = collector.getMetricType(); + Set normalizedLabels = immutableLabelNames(collector.getLabelNames()); + MetricMetadata metadata = collector.getMetadata(); + String help = metadata != null ? metadata.getHelp() : null; + Unit unit = metadata != null ? metadata.getUnit() : null; + + // Only perform validation if collector provides sufficient metadata. + // Collectors that don't implement getPrometheusName()/getMetricType() will skip validation. + if (prometheusName != null && metricType != null) { + final String name = prometheusName; + final MetricType type = metricType; + final Set names = normalizedLabels; + final String helpForValidation = help; + final Unit unitForValidation = unit; + registered.compute( + prometheusName, + (n, existingInfo) -> { + if (existingInfo == null) { + return RegistrationInfo.of(type, names, helpForValidation, unitForValidation); + } else { + if (existingInfo.getType() != type) { + throw new IllegalArgumentException( + name + + ": Conflicting metric types. Existing: " + + existingInfo.getType() + + ", new: " + + type); + } + existingInfo.validateMetadata(helpForValidation, unitForValidation); + if (!existingInfo.addLabelSet(names)) { + throw new IllegalArgumentException( + name + ": duplicate metric name with identical label schema " + names); + } + return existingInfo; } - return existingInfo; - } - }); + }); - collectorMetadata.put(collector, new CollectorRegistration(prometheusName, normalizedLabels)); - } + collectorMetadata.put( + collector, new CollectorRegistration(prometheusName, normalizedLabels)); + } - if (prometheusName != null) { - prometheusNames.add(prometheusName); + if (prometheusName != null) { + prometheusNames.add(prometheusName); + } + } catch (Exception e) { + collectors.remove(collector); + CollectorRegistration reg = collectorMetadata.remove(collector); + if (reg != null && reg.prometheusName != null) { + unregisterLabelSchema(reg.prometheusName, reg.labelNames); + } + throw e; } - - collectors.add(collector); } public void register(MultiCollector collector) { - if (multiCollectors.contains(collector)) { + if (!multiCollectors.add(collector)) { throw new IllegalArgumentException("MultiCollector instance is already registered"); } - List prometheusNamesList = collector.getPrometheusNames(); List registrations = new ArrayList<>(); Set namesOnlyInPrometheusNames = new HashSet<>(); @@ -264,8 +270,8 @@ public void register(MultiCollector collector) { } multiCollectorMetadata.put(collector, registrations); - multiCollectors.add(collector); } catch (Exception e) { + multiCollectors.remove(collector); for (MultiCollectorRegistration registration : registrations) { unregisterLabelSchema(registration.prometheusName, registration.labelNames); } diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java index 949c65f91..cdec0ddaf 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/MetricSnapshots.java @@ -29,7 +29,8 @@ public MetricSnapshots(MetricSnapshot... snapshots) { * * @param snapshots the constructor creates a sorted copy of snapshots. * @throws IllegalArgumentException if snapshots contain conflicting metric types (same name but - * different metric types like Counter vs Gauge). + * different metric types like Counter vs Gauge), or if two HistogramSnapshots share a name + * but differ in gauge histogram vs classic histogram. */ public MetricSnapshots(Collection snapshots) { List list = new ArrayList<>(snapshots); diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index b1d8e32bf..f8994200a 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -256,6 +256,68 @@ public Set getLabelNames() { .hasMessageContaining("duplicate metric name with identical label schema"); } + @Test + void register_duplicateLabelSchema_rollsBackCollectorOnFailure() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector counter1 = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("http_requests").build(); + } + + @Override + public String getPrometheusName() { + return "http_requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("path", "status")); + } + }; + + Collector counter2 = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("http_requests").build(); + } + + @Override + public String getPrometheusName() { + return "http_requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("path", "status")); + } + }; + + registry.register(counter1); + + // Second collector has same name and label schema - registration fails during metadata + // validation. The failed collector must be rolled back (not present in the registry). + assertThatThrownBy(() -> registry.register(counter2)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("duplicate metric name with identical label schema"); + + // Only the first collector should be in the registry; counter2 was removed on rollback. + assertThat(registry.scrape().size()).isEqualTo(1); + } + @Test void register_nullType_skipsValidation() { PrometheusRegistry registry = new PrometheusRegistry(); From e7aeef5d0b6452335602e4986a7e089d9c1409e8 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Tue, 3 Feb 2026 10:11:04 -0500 Subject: [PATCH 20/23] lint Signed-off-by: Jay DeLuca --- docs/content/getting-started/registry.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/getting-started/registry.md b/docs/content/getting-started/registry.md index 28857213b..7f561ecef 100644 --- a/docs/content/getting-started/registry.md +++ b/docs/content/getting-started/registry.md @@ -90,7 +90,7 @@ unavailable:** if `getPrometheusName()` or `getMetricType()` returns `null`, the validate that collector. If two such collectors produce the same metric name and same label set at scrape time, the exposition output may contain duplicate time series and be invalid for Prometheus. -When validation *is* performed (name and type are non-null), **null label names are treated as an +When validation _is_ performed (name and type are non-null), **null label names are treated as an empty label schema:** `getLabelNames()` returning `null` is normalized to `Collections.emptySet()` and full label-schema validation and duplicate detection still apply. A collector that returns a non-null type but leaves `getLabelNames()` as `null` is still validated, with its labels treated as From b17cd0f6545ff04600c4f9783292928a4c833331 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Tue, 3 Feb 2026 13:28:10 -0500 Subject: [PATCH 21/23] code review Signed-off-by: Jay DeLuca --- .../DuplicateNamesProtobufTest.java | 10 +++---- .../expositionformats/TextFormatUtil.java | 16 ++++++++++ .../DuplicateNamesExpositionTest.java | 12 ++++---- .../model/registry/PrometheusRegistry.java | 29 +++++-------------- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java b/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java index 00dd7c59f..860ef08ee 100644 --- a/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java +++ b/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java @@ -41,7 +41,7 @@ public MetricSnapshot collect() { @Override public String getPrometheusName() { - return "api_responses_total"; + return "api_responses"; } }); @@ -63,7 +63,7 @@ public MetricSnapshot collect() { @Override public String getPrometheusName() { - return "api_responses_total"; + return "api_responses"; } }); return registry; @@ -140,7 +140,7 @@ public MetricSnapshot collect() { @Override public String getPrometheusName() { - return "api_responses_total"; + return "api_responses"; } }); @@ -168,7 +168,7 @@ public MetricSnapshot collect() { @Override public String getPrometheusName() { - return "api_responses_total"; + return "api_responses"; } }); @@ -258,7 +258,7 @@ public MetricSnapshot collect() { @Override public String getPrometheusName() { - return "http_requests_total"; + return "http_requests"; } }); diff --git a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java index e5cbe8b40..2891d544c 100644 --- a/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java +++ b/prometheus-metrics-exposition-textformats/src/main/java/io/prometheus/metrics/expositionformats/TextFormatUtil.java @@ -21,6 +21,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import javax.annotation.Nullable; /** @@ -235,6 +236,21 @@ private static MetricSnapshot mergeSnapshots(List snapshots) { "Cannot merge histograms: gauge histogram and classic histogram"); } } + // Validate metadata consistency so we don't silently pick one help/unit when they differ. + if (!Objects.equals( + first.getMetadata().getPrometheusName(), snapshot.getMetadata().getPrometheusName())) { + throw new IllegalArgumentException("Cannot merge snapshots: inconsistent metric name"); + } + if (!Objects.equals(first.getMetadata().getHelp(), snapshot.getMetadata().getHelp())) { + throw new IllegalArgumentException( + "Cannot merge snapshots: conflicting help for metric " + + first.getMetadata().getPrometheusName()); + } + if (!Objects.equals(first.getMetadata().getUnit(), snapshot.getMetadata().getUnit())) { + throw new IllegalArgumentException( + "Cannot merge snapshots: conflicting unit for metric " + + first.getMetadata().getPrometheusName()); + } totalDataPoints += snapshot.getDataPoints().size(); } diff --git a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java index 31b029fa5..bdd21440f 100644 --- a/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java +++ b/prometheus-metrics-exposition-textformats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesExpositionTest.java @@ -35,7 +35,7 @@ public MetricSnapshot collect() { @Override public String getPrometheusName() { - return "api_responses_total"; + return "api_responses"; } }); @@ -57,7 +57,7 @@ public MetricSnapshot collect() { @Override public String getPrometheusName() { - return "api_responses_total"; + return "api_responses"; } }); return registry; @@ -110,7 +110,7 @@ public MetricSnapshot collect() { @Override public String getPrometheusName() { - return "api_responses_total"; + return "api_responses"; } }); @@ -138,7 +138,7 @@ public MetricSnapshot collect() { @Override public String getPrometheusName() { - return "api_responses_total"; + return "api_responses"; } }); @@ -205,7 +205,7 @@ public MetricSnapshot collect() { @Override public String getPrometheusName() { - return "api_responses_total"; + return "api_responses"; } }); @@ -228,7 +228,7 @@ public MetricSnapshot collect() { @Override public String getPrometheusName() { - return "api_responses_total"; + return "api_responses"; } }); diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index 73b2d8722..cc25eb150 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -18,7 +18,6 @@ public class PrometheusRegistry { public static final PrometheusRegistry defaultRegistry = new PrometheusRegistry(); - private final Set prometheusNames = ConcurrentHashMap.newKeySet(); private final Set collectors = ConcurrentHashMap.newKeySet(); private final Set multiCollectors = ConcurrentHashMap.newKeySet(); private final ConcurrentHashMap registered = new ConcurrentHashMap<>(); @@ -187,11 +186,12 @@ public void register(Collector collector) { + ", new: " + type); } - existingInfo.validateMetadata(helpForValidation, unitForValidation); + // Check label set first; only mutate help/unit after validation passes. if (!existingInfo.addLabelSet(names)) { throw new IllegalArgumentException( name + ": duplicate metric name with identical label schema " + names); } + existingInfo.validateMetadata(helpForValidation, unitForValidation); return existingInfo; } }); @@ -199,10 +199,6 @@ public void register(Collector collector) { collectorMetadata.put( collector, new CollectorRegistration(prometheusName, normalizedLabels)); } - - if (prometheusName != null) { - prometheusNames.add(prometheusName); - } } catch (Exception e) { collectors.remove(collector); CollectorRegistration reg = collectorMetadata.remove(collector); @@ -219,7 +215,6 @@ public void register(MultiCollector collector) { } List prometheusNamesList = collector.getPrometheusNames(); List registrations = new ArrayList<>(); - Set namesOnlyInPrometheusNames = new HashSet<>(); try { for (String prometheusName : prometheusNamesList) { @@ -249,24 +244,20 @@ public void register(MultiCollector collector) { + ", new: " + type); } - existingInfo.validateMetadata(helpForValidation, unitForValidation); + // Check label set first; only mutate help/unit after validation passes. if (!existingInfo.addLabelSet(labelNamesForValidation)) { throw new IllegalArgumentException( prometheusName + ": duplicate metric name with identical label schema " + labelNamesForValidation); } + existingInfo.validateMetadata(helpForValidation, unitForValidation); return existingInfo; } }); registrations.add(new MultiCollectorRegistration(prometheusName, normalizedLabels)); } - - boolean addedToPrometheusNames = prometheusNames.add(prometheusName); - if (addedToPrometheusNames && metricType == null) { - namesOnlyInPrometheusNames.add(prometheusName); - } } multiCollectorMetadata.put(collector, registrations); @@ -275,9 +266,6 @@ public void register(MultiCollector collector) { for (MultiCollectorRegistration registration : registrations) { unregisterLabelSchema(registration.prometheusName, registration.labelNames); } - for (String name : namesOnlyInPrometheusNames) { - prometheusNames.remove(name); - } throw e; } } @@ -303,8 +291,8 @@ public void unregister(MultiCollector collector) { } /** - * Decrements the reference count for a label schema and removes the metric name entirely if no - * schemas remain. + * Removes the label schema for the given metric name. If no label schemas remain for that name, + * removes the metric name entirely from the registry. */ private void unregisterLabelSchema(String prometheusName, Set labelNames) { registered.computeIfPresent( @@ -312,18 +300,15 @@ private void unregisterLabelSchema(String prometheusName, Set labelNames (name, info) -> { info.removeLabelSet(labelNames); if (info.isEmpty()) { - // No more label schemas for this name, remove it entirely - prometheusNames.remove(prometheusName); return null; // remove from registered map } - return info; // keep the RegistrationInfo, just with decremented count + return info; }); } public void clear() { collectors.clear(); multiCollectors.clear(); - prometheusNames.clear(); registered.clear(); collectorMetadata.clear(); multiCollectorMetadata.clear(); From 28a8514e0b4e176be309737afe51bc8c5110ae12 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Tue, 3 Feb 2026 15:16:33 -0500 Subject: [PATCH 22/23] proto update Signed-off-by: Jay DeLuca --- .../prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java | 2 +- .../metrics/expositionformats/DuplicateNamesProtobufTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java b/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java index 3979d7330..7530070ac 100644 --- a/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java +++ b/integration-tests/it-exporter/it-exporter-test/src/test/java/io/prometheus/metrics/it/exporter/test/DuplicateMetricsIT.java @@ -3,7 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import io.prometheus.client.it.common.ExporterTest; -import io.prometheus.metrics.expositionformats.generated.com_google_protobuf_4_33_4.Metrics; +import io.prometheus.metrics.expositionformats.generated.com_google_protobuf_4_33_5.Metrics; import java.io.IOException; import java.net.URISyntaxException; import java.util.List; diff --git a/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java b/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java index 860ef08ee..9b1caa8f6 100644 --- a/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java +++ b/prometheus-metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/DuplicateNamesProtobufTest.java @@ -3,7 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import io.prometheus.metrics.config.EscapingScheme; -import io.prometheus.metrics.expositionformats.generated.com_google_protobuf_4_33_4.Metrics; +import io.prometheus.metrics.expositionformats.generated.com_google_protobuf_4_33_5.Metrics; import io.prometheus.metrics.expositionformats.internal.PrometheusProtobufWriterImpl; import io.prometheus.metrics.model.registry.Collector; import io.prometheus.metrics.model.registry.PrometheusRegistry; From 9bf69c844a4e218c3bb48716a0116834598355a5 Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Wed, 4 Feb 2026 13:06:11 -0500 Subject: [PATCH 23/23] be consistent about help/unit validation, ensure deregister labels Signed-off-by: Jay DeLuca --- .../model/registry/PrometheusRegistry.java | 138 +++++------ .../registry/PrometheusRegistryTest.java | 222 ++++++++++-------- 2 files changed, 178 insertions(+), 182 deletions(-) diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java index cc25eb150..17c78f366 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/registry/PrometheusRegistry.java @@ -1,5 +1,7 @@ package io.prometheus.metrics.model.registry; +import static java.util.Collections.emptySet; + import io.prometheus.metrics.model.snapshots.MetricMetadata; import io.prometheus.metrics.model.snapshots.MetricSnapshot; import io.prometheus.metrics.model.snapshots.MetricSnapshots; @@ -53,8 +55,7 @@ private static class MultiCollectorRegistration { /** * Tracks registration information for each metric name to enable validation of type consistency, - * label schema uniqueness, and help/unit consistency. Stores metadata to enable O(1) - * unregistration without iterating through all collectors. + * label schema uniqueness, and help/unit consistency. */ private static class RegistrationInfo { private final MetricType type; @@ -80,31 +81,25 @@ static RegistrationInfo of( @Nullable Unit unit) { Set> labelSchemas = ConcurrentHashMap.newKeySet(); Set normalized = - (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; + (labelNames == null || labelNames.isEmpty()) ? emptySet() : labelNames; labelSchemas.add(normalized); return new RegistrationInfo(type, labelSchemas, help, unit); } /** - * Validates that the given help and unit are consistent with this registration. Throws if - * non-null values conflict. When stored help/unit is null and the new value is non-null, - * captures the first non-null so subsequent registrations are validated consistently. + * Validates that the given help and unit are exactly equal to this registration. Throws if + * values differ, including when one is null and the other is non-null. This ensures consistent + * metadata across all collectors sharing the same metric name. */ void validateMetadata(@Nullable String newHelp, @Nullable Unit newUnit) { - if (help != null && newHelp != null && !Objects.equals(help, newHelp)) { + if (!Objects.equals(help, newHelp)) { throw new IllegalArgumentException( "Conflicting help strings. Existing: \"" + help + "\", new: \"" + newHelp + "\""); } - if (unit != null && newUnit != null && !Objects.equals(unit, newUnit)) { + if (!Objects.equals(unit, newUnit)) { throw new IllegalArgumentException( "Conflicting unit. Existing: " + unit + ", new: " + newUnit); } - if (help == null && newHelp != null) { - this.help = newHelp; - } - if (unit == null && newUnit != null) { - this.unit = newUnit; - } } /** @@ -115,7 +110,7 @@ void validateMetadata(@Nullable String newHelp, @Nullable Unit newUnit) { */ boolean addLabelSet(@Nullable Set labelNames) { Set normalized = - (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; + (labelNames == null || labelNames.isEmpty()) ? emptySet() : labelNames; return labelSchemas.add(normalized); } @@ -126,7 +121,7 @@ boolean addLabelSet(@Nullable Set labelNames) { */ void removeLabelSet(@Nullable Set labelNames) { Set normalized = - (labelNames == null || labelNames.isEmpty()) ? Collections.emptySet() : labelNames; + (labelNames == null || labelNames.isEmpty()) ? emptySet() : labelNames; labelSchemas.remove(normalized); } @@ -147,11 +142,56 @@ MetricType getType() { */ private static Set immutableLabelNames(@Nullable Set labelNames) { if (labelNames == null || labelNames.isEmpty()) { - return Collections.emptySet(); + return emptySet(); } return Collections.unmodifiableSet(new HashSet<>(labelNames)); } + /** + * Validates the registration of a metric with the given parameters. Ensures type consistency, + * label schema uniqueness, and help/unit consistency. + */ + private void validateRegistration( + String prometheusName, + MetricType metricType, + Set normalizedLabels, + @Nullable String help, + @Nullable Unit unit) { + final MetricType type = metricType; + final Set names = normalizedLabels; + final String helpForValidation = help; + final Unit unitForValidation = unit; + registered.compute( + prometheusName, + (n, existingInfo) -> { + if (existingInfo == null) { + return RegistrationInfo.of(type, names, helpForValidation, unitForValidation); + } else { + if (existingInfo.getType() != type) { + throw new IllegalArgumentException( + prometheusName + + ": Conflicting metric types. Existing: " + + existingInfo.getType() + + ", new: " + + type); + } + // Check label set first; only mutate help/unit after validation passes. + if (!existingInfo.addLabelSet(names)) { + throw new IllegalArgumentException( + prometheusName + ": duplicate metric name with identical label schema " + names); + } + // Roll back label schema if metadata validation fails + try { + existingInfo.validateMetadata(helpForValidation, unitForValidation); + } catch (IllegalArgumentException e) { + existingInfo.removeLabelSet(names); + throw e; + } + return existingInfo; + } + }); + } + public void register(Collector collector) { if (!collectors.add(collector)) { throw new IllegalArgumentException("Collector instance is already registered"); @@ -167,35 +207,7 @@ public void register(Collector collector) { // Only perform validation if collector provides sufficient metadata. // Collectors that don't implement getPrometheusName()/getMetricType() will skip validation. if (prometheusName != null && metricType != null) { - final String name = prometheusName; - final MetricType type = metricType; - final Set names = normalizedLabels; - final String helpForValidation = help; - final Unit unitForValidation = unit; - registered.compute( - prometheusName, - (n, existingInfo) -> { - if (existingInfo == null) { - return RegistrationInfo.of(type, names, helpForValidation, unitForValidation); - } else { - if (existingInfo.getType() != type) { - throw new IllegalArgumentException( - name - + ": Conflicting metric types. Existing: " - + existingInfo.getType() - + ", new: " - + type); - } - // Check label set first; only mutate help/unit after validation passes. - if (!existingInfo.addLabelSet(names)) { - throw new IllegalArgumentException( - name + ": duplicate metric name with identical label schema " + names); - } - existingInfo.validateMetadata(helpForValidation, unitForValidation); - return existingInfo; - } - }); - + validateRegistration(prometheusName, metricType, normalizedLabels, help, unit); collectorMetadata.put( collector, new CollectorRegistration(prometheusName, normalizedLabels)); } @@ -225,37 +237,7 @@ public void register(MultiCollector collector) { Unit unit = metadata != null ? metadata.getUnit() : null; if (metricType != null) { - final MetricType type = metricType; - final Set labelNamesForValidation = normalizedLabels; - final String helpForValidation = help; - final Unit unitForValidation = unit; - registered.compute( - prometheusName, - (name, existingInfo) -> { - if (existingInfo == null) { - return RegistrationInfo.of( - type, labelNamesForValidation, helpForValidation, unitForValidation); - } else { - if (existingInfo.getType() != type) { - throw new IllegalArgumentException( - prometheusName - + ": Conflicting metric types. Existing: " - + existingInfo.getType() - + ", new: " - + type); - } - // Check label set first; only mutate help/unit after validation passes. - if (!existingInfo.addLabelSet(labelNamesForValidation)) { - throw new IllegalArgumentException( - prometheusName - + ": duplicate metric name with identical label schema " - + labelNamesForValidation); - } - existingInfo.validateMetadata(helpForValidation, unitForValidation); - return existingInfo; - } - }); - + validateRegistration(prometheusName, metricType, normalizedLabels, help, unit); registrations.add(new MultiCollectorRegistration(prometheusName, normalizedLabels)); } } @@ -300,7 +282,7 @@ private void unregisterLabelSchema(String prometheusName, Set labelNames (name, info) -> { info.removeLabelSet(labelNames); if (info.isEmpty()) { - return null; // remove from registered map + return null; } return info; }); diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java index f8994200a..90a04934e 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/registry/PrometheusRegistryTest.java @@ -86,7 +86,7 @@ public List getPrometheusNames() { }; @Test - void register_duplicateName_withoutTypeInfo_notAllowed() { + void register_sameInstanceTwice_notAllowed() { PrometheusRegistry registry = new PrometheusRegistry(); registry.register(noName); @@ -318,6 +318,110 @@ public Set getLabelNames() { assertThat(registry.scrape().size()).isEqualTo(1); } + @Test + void register_metadataValidationFailure_rollsBackLabelSchema() { + PrometheusRegistry registry = new PrometheusRegistry(); + + Collector counterWithHelpOne = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("path")); + } + + @Override + public MetricMetadata getMetadata() { + return new MetricMetadata("requests", "First help", null); + } + }; + + Collector counterWithHelpTwo = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("status")); + } + + @Override + public MetricMetadata getMetadata() { + return new MetricMetadata("requests", "Second help", null); + } + }; + + Collector counterWithCorrectHelp = + new Collector() { + @Override + public MetricSnapshot collect() { + return CounterSnapshot.builder().name("requests").build(); + } + + @Override + public String getPrometheusName() { + return "requests"; + } + + @Override + public MetricType getMetricType() { + return MetricType.COUNTER; + } + + @Override + public Set getLabelNames() { + return new HashSet<>(asList("status")); + } + + @Override + public MetricMetadata getMetadata() { + return new MetricMetadata("requests", "First help", null); + } + }; + + registry.register(counterWithHelpOne); + + // Second collector has conflicting help - should fail and roll back its label schema + assertThatThrownBy(() -> registry.register(counterWithHelpTwo)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Conflicting help strings"); + + // Third collector has same label schema as second (which failed), but correct help. + // If label schema wasn't rolled back, this would fail with "duplicate label schema". + // If rollback worked, this should succeed. + assertThatCode(() -> registry.register(counterWithCorrectHelp)).doesNotThrowAnyException(); + + // Verify both collectors are in the registry + assertThat(registry.scrape().size()).isEqualTo(2); + } + @Test void register_nullType_skipsValidation() { PrometheusRegistry registry = new PrometheusRegistry(); @@ -710,71 +814,7 @@ public MetricMetadata getMetadata() { } @Test - void register_sameName_sameHelpAndUnit_allowed() { - PrometheusRegistry registry = new PrometheusRegistry(); - - Collector withPath = - new Collector() { - @Override - public MetricSnapshot collect() { - return CounterSnapshot.builder().name("requests").help("Total requests").build(); - } - - @Override - public String getPrometheusName() { - return "requests"; - } - - @Override - public MetricType getMetricType() { - return MetricType.COUNTER; - } - - @Override - public Set getLabelNames() { - return new HashSet<>(asList("path")); - } - - @Override - public MetricMetadata getMetadata() { - return new MetricMetadata("requests", "Total requests", null); - } - }; - - Collector withStatus = - new Collector() { - @Override - public MetricSnapshot collect() { - return CounterSnapshot.builder().name("requests").help("Total requests").build(); - } - - @Override - public String getPrometheusName() { - return "requests"; - } - - @Override - public MetricType getMetricType() { - return MetricType.COUNTER; - } - - @Override - public Set getLabelNames() { - return new HashSet<>(asList("status")); - } - - @Override - public MetricMetadata getMetadata() { - return new MetricMetadata("requests", "Total requests", null); - } - }; - - registry.register(withPath); - assertThatCode(() -> registry.register(withStatus)).doesNotThrowAnyException(); - } - - @Test - void register_sameName_oneNullHelp_allowed() { + void register_sameName_nullVsNonNullHelp_notAllowed() { PrometheusRegistry registry = new PrometheusRegistry(); Collector withHelp = @@ -834,38 +874,16 @@ public MetricMetadata getMetadata() { }; registry.register(withHelp); - // One has help, one doesn't - should be allowed - assertThatCode(() -> registry.register(withoutHelp)).doesNotThrowAnyException(); + assertThatThrownBy(() -> registry.register(withoutHelp)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("Conflicting help strings"); } @Test - void register_firstOmitsHelp_secondProvidesHelp_thirdWithDifferentHelp_throws() { + void register_sameName_sameHelpAndUnit_allowed() { PrometheusRegistry registry = new PrometheusRegistry(); - Collector withoutHelp = - new Collector() { - @Override - public MetricSnapshot collect() { - return CounterSnapshot.builder().name("requests").build(); - } - - @Override - public String getPrometheusName() { - return "requests"; - } - - @Override - public MetricType getMetricType() { - return MetricType.COUNTER; - } - - @Override - public Set getLabelNames() { - return new HashSet<>(asList("path")); - } - }; - - Collector withHelpTotal = + Collector withPath = new Collector() { @Override public MetricSnapshot collect() { @@ -884,7 +902,7 @@ public MetricType getMetricType() { @Override public Set getLabelNames() { - return new HashSet<>(asList("status")); + return new HashSet<>(asList("path")); } @Override @@ -893,11 +911,11 @@ public MetricMetadata getMetadata() { } }; - Collector withHelpOther = + Collector withStatus = new Collector() { @Override public MetricSnapshot collect() { - return CounterSnapshot.builder().name("requests").help("Other help").build(); + return CounterSnapshot.builder().name("requests").help("Total requests").build(); } @Override @@ -912,21 +930,17 @@ public MetricType getMetricType() { @Override public Set getLabelNames() { - return new HashSet<>(asList("method")); + return new HashSet<>(asList("status")); } @Override public MetricMetadata getMetadata() { - return new MetricMetadata("requests", "Other help", null); + return new MetricMetadata("requests", "Total requests", null); } }; - registry.register(withoutHelp); - registry.register(withHelpTotal); - // First had no help, second provided "Total requests" (captured). Third conflicts. - assertThatThrownBy(() -> registry.register(withHelpOther)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessageContaining("Conflicting help strings"); + registry.register(withPath); + assertThatCode(() -> registry.register(withStatus)).doesNotThrowAnyException(); } @Test