From a85e35aa1e5460d6a474c0e27345fecfc17daacb Mon Sep 17 00:00:00 2001 From: Jay DeLuca Date: Tue, 20 Jan 2026 06:41:22 -0500 Subject: [PATCH 01/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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/18] 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)