diff --git a/implementations/micrometer-registry-prometheus-simpleclient/src/test/java/io/micrometer/prometheus/PrometheusMeterRegistryTest.java b/implementations/micrometer-registry-prometheus-simpleclient/src/test/java/io/micrometer/prometheus/PrometheusMeterRegistryTest.java index 0bc53ffcec..cf4c0da227 100644 --- a/implementations/micrometer-registry-prometheus-simpleclient/src/test/java/io/micrometer/prometheus/PrometheusMeterRegistryTest.java +++ b/implementations/micrometer-registry-prometheus-simpleclient/src/test/java/io/micrometer/prometheus/PrometheusMeterRegistryTest.java @@ -15,6 +15,7 @@ */ package io.micrometer.prometheus; +import io.micrometer.common.lang.Nullable; import io.micrometer.core.Issue; import io.micrometer.core.instrument.Timer; import io.micrometer.core.instrument.*; @@ -717,6 +718,50 @@ void noExemplarsIfNoSampler() { assertThat(scraped).endsWith("# EOF\n"); } + @Test + @Issue("#5229") + void doesNotCallConventionOnScrape() { + CountingPrometheusNamingConvention convention = new CountingPrometheusNamingConvention(); + registry.config().namingConvention(convention); + + Timer.builder("timer").tag("k1", "v1").description("my timer").register(registry); + Counter.builder("counter").tag("k1", "v1").description("my counter").register(registry); + DistributionSummary.builder("summary").tag("k1", "v1").description("my summary").register(registry); + Gauge.builder("gauge", new AtomicInteger(), AtomicInteger::doubleValue) + .tag("k1", "v1") + .description("my gauge") + .register(registry); + LongTaskTimer.builder("long.task.timer").tag("k1", "v1").description("my long task timer").register(registry); + + int expectedNameCount = convention.nameCount.get(); + int expectedTagKeyCount = convention.tagKeyCount.get(); + + registry.scrape(); + + assertThat(convention.nameCount.get()).isEqualTo(expectedNameCount); + assertThat(convention.tagKeyCount.get()).isEqualTo(expectedTagKeyCount); + } + + private static class CountingPrometheusNamingConvention extends PrometheusNamingConvention { + + AtomicInteger nameCount = new AtomicInteger(); + + AtomicInteger tagKeyCount = new AtomicInteger(); + + @Override + public String name(String name, Meter.Type type, @Nullable String baseUnit) { + nameCount.incrementAndGet(); + return super.name(name, type, baseUnit); + } + + @Override + public String tagKey(String key) { + tagKeyCount.incrementAndGet(); + return super.tagKey(key); + } + + } + static class TestSpanContextSupplier implements SpanContextSupplier { private final AtomicLong count = new AtomicLong(); diff --git a/implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistry.java b/implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistry.java index 641acb4fb4..748efa48df 100644 --- a/implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistry.java +++ b/implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistry.java @@ -213,8 +213,8 @@ public Counter newCounter(Meter.Id id) { (conventionName, tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName, family -> new CounterSnapshot(family.metadata, family.dataPointSnapshots), - getMetadata(id), new CounterDataPointSnapshot(counter.count(), - Labels.of(tagKeys, tagValues), counter.exemplar(), 0)))); + getMetadata(conventionName, id.getDescription()), new CounterDataPointSnapshot( + counter.count(), Labels.of(tagKeys, tagValues), counter.exemplar(), 0)))); }); return counter; } @@ -246,9 +246,9 @@ public DistributionSummary newDistributionSummary(Meter.Id id, Exemplars exemplars = summary.exemplars(); families.add(new MicrometerCollector.Family<>(conventionName, - family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id), - new SummaryDataPointSnapshot(count, sum, quantiles, Labels.of(tagKeys, tagValues), - exemplars, 0))); + family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots), + getMetadata(conventionName, id.getDescription()), new SummaryDataPointSnapshot(count, sum, + quantiles, Labels.of(tagKeys, tagValues), exemplars, 0))); } else { List buckets = new ArrayList<>(); @@ -277,8 +277,9 @@ public DistributionSummary newDistributionSummary(Meter.Id id, families.add(new MicrometerCollector.Family<>(conventionName, family -> new io.prometheus.metrics.model.snapshots.HistogramSnapshot(family.metadata, family.dataPointSnapshots), - getMetadata(id), new HistogramDataPointSnapshot(ClassicHistogramBuckets.of(buckets, counts), - sum, Labels.of(tagKeys, tagValues), exemplars, 0))); + getMetadata(conventionName, id.getDescription()), + new HistogramDataPointSnapshot(ClassicHistogramBuckets.of(buckets, counts), sum, + Labels.of(tagKeys, tagValues), exemplars, 0))); // TODO: Add support back for VictoriaMetrics // Previously we had low-level control so a histogram was just @@ -292,7 +293,7 @@ public DistributionSummary newDistributionSummary(Meter.Id id, families.add(new MicrometerCollector.Family<>(conventionName + "_max", family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots), - getMetadata(id, "_max"), + getMetadata(conventionName + "_max", id.getDescription()), new GaugeDataPointSnapshot(summary.max(), Labels.of(tagKeys, tagValues), null))); return families.build(); @@ -319,17 +320,18 @@ protected io.micrometer.core.instrument.Gauge newGauge(Meter.Id id, @Nullabl applyToCollector(id, (collector) -> { List tagValues = tagValues(id); if (id.getName().endsWith(".info")) { - collector - .add(tagValues, - (conventionName, tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName, - family -> new InfoSnapshot(family.metadata, family.dataPointSnapshots), - getMetadata(id), new InfoDataPointSnapshot(Labels.of(tagKeys, tagValues))))); + collector.add(tagValues, + (conventionName, + tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName, + family -> new InfoSnapshot(family.metadata, family.dataPointSnapshots), + getMetadata(conventionName, id.getDescription()), + new InfoDataPointSnapshot(Labels.of(tagKeys, tagValues))))); } else { collector.add(tagValues, (conventionName, tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName, family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots), - getMetadata(id), + getMetadata(conventionName, id.getDescription()), new GaugeDataPointSnapshot(gauge.value(), Labels.of(tagKeys, tagValues), null)))); } }); @@ -352,10 +354,12 @@ protected FunctionTimer newFunctionTimer(Meter.Id id, T obj, ToLongFunction< applyToCollector(id, (collector) -> { List tagValues = tagValues(id); collector.add(tagValues, - (conventionName, tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName, - family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id), - new SummaryDataPointSnapshot((long) ft.count(), ft.totalTime(getBaseTimeUnit()), - Quantiles.EMPTY, Labels.of(tagKeys, tagValues), null, 0)))); + (conventionName, + tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName, + family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots), + getMetadata(conventionName, id.getDescription()), + new SummaryDataPointSnapshot((long) ft.count(), ft.totalTime(getBaseTimeUnit()), + Quantiles.EMPTY, Labels.of(tagKeys, tagValues), null, 0)))); }); return ft; } @@ -365,10 +369,12 @@ protected FunctionCounter newFunctionCounter(Meter.Id id, T obj, ToDoubleFun FunctionCounter fc = new CumulativeFunctionCounter<>(id, obj, countFunction); applyToCollector(id, (collector) -> { List tagValues = tagValues(id); - collector.add(tagValues, - (conventionName, tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName, - family -> new CounterSnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id), - new CounterDataPointSnapshot(fc.count(), Labels.of(tagKeys, tagValues), null, 0)))); + collector + .add(tagValues, + (conventionName, tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName, + family -> new CounterSnapshot(family.metadata, family.dataPointSnapshots), + getMetadata(conventionName, id.getDescription()), + new CounterDataPointSnapshot(fc.count(), Labels.of(tagKeys, tagValues), null, 0)))); }); return fc; } @@ -423,14 +429,16 @@ protected Meter newMeter(Meter.Id id, Meter.Type type, Iterable mea private MicrometerCollector.Family customCounterFamily(Meter.Id id, String conventionName, String suffix, Labels labels, double value) { return new MicrometerCollector.Family<>(conventionName + suffix, - family -> new CounterSnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id, suffix), + family -> new CounterSnapshot(family.metadata, family.dataPointSnapshots), + getMetadata(conventionName + suffix, id.getDescription()), new CounterDataPointSnapshot(value, labels, null, 0)); } private MicrometerCollector.Family customGaugeFamily(Meter.Id id, String conventionName, String suffix, Labels labels, double value) { return new MicrometerCollector.Family<>(conventionName + suffix, - family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id, suffix), + family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots), + getMetadata(conventionName + suffix, id.getDescription()), new GaugeDataPointSnapshot(value, labels, null)); } @@ -470,9 +478,9 @@ private void addDistributionStatisticSamples(Meter.Id id, MicrometerCollector co Exemplars exemplars = createExemplarsWithScaledValues(exemplarsSupplier.get()); families.add(new MicrometerCollector.Family<>(conventionName, - family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id), - new SummaryDataPointSnapshot(count, sum, quantiles, Labels.of(tagKeys, tagValues), exemplars, - 0))); + family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots), + getMetadata(conventionName, id.getDescription()), new SummaryDataPointSnapshot(count, sum, + quantiles, Labels.of(tagKeys, tagValues), exemplars, 0))); } else { List buckets = new ArrayList<>(); @@ -501,8 +509,9 @@ private void addDistributionStatisticSamples(Meter.Id id, MicrometerCollector co families.add(new MicrometerCollector.Family<>(conventionName, family -> new io.prometheus.metrics.model.snapshots.HistogramSnapshot(forLongTaskTimer, family.metadata, family.dataPointSnapshots), - getMetadata(id), new HistogramDataPointSnapshot(ClassicHistogramBuckets.of(buckets, counts), - sum, Labels.of(tagKeys, tagValues), exemplars, 0))); + getMetadata(conventionName, id.getDescription()), + new HistogramDataPointSnapshot(ClassicHistogramBuckets.of(buckets, counts), sum, + Labels.of(tagKeys, tagValues), exemplars, 0))); // TODO: Add support back for VictoriaMetrics // Previously we had low-level control so a histogram was just @@ -515,9 +524,9 @@ private void addDistributionStatisticSamples(Meter.Id id, MicrometerCollector co } families.add(new MicrometerCollector.Family<>(conventionName + "_max", - family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id, "_max"), - new GaugeDataPointSnapshot(histogramSnapshot.max(getBaseTimeUnit()), Labels.of(tagKeys, tagValues), - null))); + family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots), + getMetadata(conventionName + "_max", id.getDescription()), new GaugeDataPointSnapshot( + histogramSnapshot.max(getBaseTimeUnit()), Labels.of(tagKeys, tagValues), null))); return families.build(); }); @@ -549,13 +558,8 @@ private void onMeterRemoved(Meter meter) { } } - private MetricMetadata getMetadata(Meter.Id id) { - return getMetadata(id, ""); - } - - private MetricMetadata getMetadata(Meter.Id id, String suffix) { - String name = config().namingConvention().name(id.getName(), id.getType(), id.getBaseUnit()) + suffix; - String help = prometheusConfig.descriptions() ? Optional.ofNullable(id.getDescription()).orElse(" ") : " "; + private MetricMetadata getMetadata(String name, @Nullable String description) { + String help = prometheusConfig.descriptions() ? Optional.ofNullable(description).orElse(" ") : " "; // Unit is intentionally not set, see: // https://github.com/OpenObservability/OpenMetrics/blob/1386544931307dff279688f332890c31b6c5de36/specification/OpenMetrics.md#unit return new MetricMetadata(name, help, null); diff --git a/implementations/micrometer-registry-prometheus/src/test/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistryTest.java b/implementations/micrometer-registry-prometheus/src/test/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistryTest.java index 40e0d1be33..5d41d34302 100644 --- a/implementations/micrometer-registry-prometheus/src/test/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistryTest.java +++ b/implementations/micrometer-registry-prometheus/src/test/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistryTest.java @@ -15,6 +15,7 @@ */ package io.micrometer.prometheusmetrics; +import io.micrometer.common.lang.Nullable; import io.micrometer.core.Issue; import io.micrometer.core.instrument.LongTaskTimer.Sample; import io.micrometer.core.instrument.Timer; @@ -991,6 +992,50 @@ void noExemplarsIfNoSampler() { assertThat(scraped).endsWith("# EOF\n"); } + @Test + @Issue("#5229") + void doesNotCallConventionOnScrape() { + CountingPrometheusNamingConvention convention = new CountingPrometheusNamingConvention(); + registry.config().namingConvention(convention); + + Timer.builder("timer").tag("k1", "v1").description("my timer").register(registry); + Counter.builder("counter").tag("k1", "v1").description("my counter").register(registry); + DistributionSummary.builder("summary").tag("k1", "v1").description("my summary").register(registry); + Gauge.builder("gauge", new AtomicInteger(), AtomicInteger::doubleValue) + .tag("k1", "v1") + .description("my gauge") + .register(registry); + LongTaskTimer.builder("long.task.timer").tag("k1", "v1").description("my long task timer").register(registry); + + int expectedNameCount = convention.nameCount.get(); + int expectedTagKeyCount = convention.tagKeyCount.get(); + + registry.scrape(); + + assertThat(convention.nameCount.get()).isEqualTo(expectedNameCount); + assertThat(convention.tagKeyCount.get()).isEqualTo(expectedTagKeyCount); + } + + private static class CountingPrometheusNamingConvention extends PrometheusNamingConvention { + + AtomicInteger nameCount = new AtomicInteger(); + + AtomicInteger tagKeyCount = new AtomicInteger(); + + @Override + public String name(String name, Meter.Type type, @Nullable String baseUnit) { + nameCount.incrementAndGet(); + return super.name(name, type, baseUnit); + } + + @Override + public String tagKey(String key) { + tagKeyCount.incrementAndGet(); + return super.tagKey(key); + } + + } + private PrometheusMeterRegistry createPrometheusMeterRegistryWithProperties(Properties properties) { PrometheusConfig prometheusConfig = new PrometheusConfig() { @Override