Skip to content

Commit 083f12e

Browse files
shakuzenshakuzen
authored
Avoid calling naming convention on scrape (#5288)
The naming convention was being called on each scrape for each metric to create the MetricMetadata. This is unnecessary because we already have the computed convention name specifically to avoid needing to call the convention again. See gh-5229
1 parent faeebde commit 083f12e

File tree

3 files changed

+133
-39
lines changed

3 files changed

+133
-39
lines changed

‎implementations/micrometer-registry-prometheus-simpleclient/src/test/java/io/micrometer/prometheus/PrometheusMeterRegistryTest.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package io.micrometer.prometheus;
1717

18+
import io.micrometer.common.lang.Nullable;
1819
import io.micrometer.core.Issue;
1920
import io.micrometer.core.instrument.Timer;
2021
import io.micrometer.core.instrument.*;
@@ -717,6 +718,50 @@ void noExemplarsIfNoSampler() {
717718
assertThat(scraped).endsWith("# EOF\n");
718719
}
719720

721+
@Test
722+
@Issue("#5229")
723+
void doesNotCallConventionOnScrape() {
724+
CountingPrometheusNamingConvention convention = new CountingPrometheusNamingConvention();
725+
registry.config().namingConvention(convention);
726+
727+
Timer.builder("timer").tag("k1", "v1").description("my timer").register(registry);
728+
Counter.builder("counter").tag("k1", "v1").description("my counter").register(registry);
729+
DistributionSummary.builder("summary").tag("k1", "v1").description("my summary").register(registry);
730+
Gauge.builder("gauge", new AtomicInteger(), AtomicInteger::doubleValue)
731+
.tag("k1", "v1")
732+
.description("my gauge")
733+
.register(registry);
734+
LongTaskTimer.builder("long.task.timer").tag("k1", "v1").description("my long task timer").register(registry);
735+
736+
int expectedNameCount = convention.nameCount.get();
737+
int expectedTagKeyCount = convention.tagKeyCount.get();
738+
739+
registry.scrape();
740+
741+
assertThat(convention.nameCount.get()).isEqualTo(expectedNameCount);
742+
assertThat(convention.tagKeyCount.get()).isEqualTo(expectedTagKeyCount);
743+
}
744+
745+
private static class CountingPrometheusNamingConvention extends PrometheusNamingConvention {
746+
747+
AtomicInteger nameCount = new AtomicInteger();
748+
749+
AtomicInteger tagKeyCount = new AtomicInteger();
750+
751+
@Override
752+
public String name(String name, Meter.Type type, @Nullable String baseUnit) {
753+
nameCount.incrementAndGet();
754+
return super.name(name, type, baseUnit);
755+
}
756+
757+
@Override
758+
public String tagKey(String key) {
759+
tagKeyCount.incrementAndGet();
760+
return super.tagKey(key);
761+
}
762+
763+
}
764+
720765
static class TestSpanContextSupplier implements SpanContextSupplier {
721766

722767
private final AtomicLong count = new AtomicLong();

‎implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistry.java

Lines changed: 43 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -213,8 +213,8 @@ public Counter newCounter(Meter.Id id) {
213213
(conventionName,
214214
tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName,
215215
family -> new CounterSnapshot(family.metadata, family.dataPointSnapshots),
216-
getMetadata(id), new CounterDataPointSnapshot(counter.count(),
217-
Labels.of(tagKeys, tagValues), counter.exemplar(), 0))));
216+
getMetadata(conventionName, id.getDescription()), new CounterDataPointSnapshot(
217+
counter.count(), Labels.of(tagKeys, tagValues), counter.exemplar(), 0))));
218218
});
219219
return counter;
220220
}
@@ -246,9 +246,9 @@ public DistributionSummary newDistributionSummary(Meter.Id id,
246246

247247
Exemplars exemplars = summary.exemplars();
248248
families.add(new MicrometerCollector.Family<>(conventionName,
249-
family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id),
250-
new SummaryDataPointSnapshot(count, sum, quantiles, Labels.of(tagKeys, tagValues),
251-
exemplars, 0)));
249+
family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots),
250+
getMetadata(conventionName, id.getDescription()), new SummaryDataPointSnapshot(count, sum,
251+
quantiles, Labels.of(tagKeys, tagValues), exemplars, 0)));
252252
}
253253
else {
254254
List<Double> buckets = new ArrayList<>();
@@ -277,8 +277,9 @@ public DistributionSummary newDistributionSummary(Meter.Id id,
277277
families.add(new MicrometerCollector.Family<>(conventionName,
278278
family -> new io.prometheus.metrics.model.snapshots.HistogramSnapshot(family.metadata,
279279
family.dataPointSnapshots),
280-
getMetadata(id), new HistogramDataPointSnapshot(ClassicHistogramBuckets.of(buckets, counts),
281-
sum, Labels.of(tagKeys, tagValues), exemplars, 0)));
280+
getMetadata(conventionName, id.getDescription()),
281+
new HistogramDataPointSnapshot(ClassicHistogramBuckets.of(buckets, counts), sum,
282+
Labels.of(tagKeys, tagValues), exemplars, 0)));
282283

283284
// TODO: Add support back for VictoriaMetrics
284285
// Previously we had low-level control so a histogram was just
@@ -292,7 +293,7 @@ public DistributionSummary newDistributionSummary(Meter.Id id,
292293

293294
families.add(new MicrometerCollector.Family<>(conventionName + "_max",
294295
family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots),
295-
getMetadata(id, "_max"),
296+
getMetadata(conventionName + "_max", id.getDescription()),
296297
new GaugeDataPointSnapshot(summary.max(), Labels.of(tagKeys, tagValues), null)));
297298

298299
return families.build();
@@ -319,17 +320,18 @@ protected <T> io.micrometer.core.instrument.Gauge newGauge(Meter.Id id, @Nullabl
319320
applyToCollector(id, (collector) -> {
320321
List<String> tagValues = tagValues(id);
321322
if (id.getName().endsWith(".info")) {
322-
collector
323-
.add(tagValues,
324-
(conventionName, tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName,
325-
family -> new InfoSnapshot(family.metadata, family.dataPointSnapshots),
326-
getMetadata(id), new InfoDataPointSnapshot(Labels.of(tagKeys, tagValues)))));
323+
collector.add(tagValues,
324+
(conventionName,
325+
tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName,
326+
family -> new InfoSnapshot(family.metadata, family.dataPointSnapshots),
327+
getMetadata(conventionName, id.getDescription()),
328+
new InfoDataPointSnapshot(Labels.of(tagKeys, tagValues)))));
327329
}
328330
else {
329331
collector.add(tagValues,
330332
(conventionName, tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName,
331333
family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots),
332-
getMetadata(id),
334+
getMetadata(conventionName, id.getDescription()),
333335
new GaugeDataPointSnapshot(gauge.value(), Labels.of(tagKeys, tagValues), null))));
334336
}
335337
});
@@ -352,10 +354,12 @@ protected <T> FunctionTimer newFunctionTimer(Meter.Id id, T obj, ToLongFunction<
352354
applyToCollector(id, (collector) -> {
353355
List<String> tagValues = tagValues(id);
354356
collector.add(tagValues,
355-
(conventionName, tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName,
356-
family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id),
357-
new SummaryDataPointSnapshot((long) ft.count(), ft.totalTime(getBaseTimeUnit()),
358-
Quantiles.EMPTY, Labels.of(tagKeys, tagValues), null, 0))));
357+
(conventionName,
358+
tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName,
359+
family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots),
360+
getMetadata(conventionName, id.getDescription()),
361+
new SummaryDataPointSnapshot((long) ft.count(), ft.totalTime(getBaseTimeUnit()),
362+
Quantiles.EMPTY, Labels.of(tagKeys, tagValues), null, 0))));
359363
});
360364
return ft;
361365
}
@@ -365,10 +369,12 @@ protected <T> FunctionCounter newFunctionCounter(Meter.Id id, T obj, ToDoubleFun
365369
FunctionCounter fc = new CumulativeFunctionCounter<>(id, obj, countFunction);
366370
applyToCollector(id, (collector) -> {
367371
List<String> tagValues = tagValues(id);
368-
collector.add(tagValues,
369-
(conventionName, tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName,
370-
family -> new CounterSnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id),
371-
new CounterDataPointSnapshot(fc.count(), Labels.of(tagKeys, tagValues), null, 0))));
372+
collector
373+
.add(tagValues,
374+
(conventionName, tagKeys) -> Stream.of(new MicrometerCollector.Family<>(conventionName,
375+
family -> new CounterSnapshot(family.metadata, family.dataPointSnapshots),
376+
getMetadata(conventionName, id.getDescription()),
377+
new CounterDataPointSnapshot(fc.count(), Labels.of(tagKeys, tagValues), null, 0))));
372378
});
373379
return fc;
374380
}
@@ -423,14 +429,16 @@ protected Meter newMeter(Meter.Id id, Meter.Type type, Iterable<Measurement> mea
423429
private MicrometerCollector.Family<CounterDataPointSnapshot> customCounterFamily(Meter.Id id, String conventionName,
424430
String suffix, Labels labels, double value) {
425431
return new MicrometerCollector.Family<>(conventionName + suffix,
426-
family -> new CounterSnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id, suffix),
432+
family -> new CounterSnapshot(family.metadata, family.dataPointSnapshots),
433+
getMetadata(conventionName + suffix, id.getDescription()),
427434
new CounterDataPointSnapshot(value, labels, null, 0));
428435
}
429436

430437
private MicrometerCollector.Family<GaugeDataPointSnapshot> customGaugeFamily(Meter.Id id, String conventionName,
431438
String suffix, Labels labels, double value) {
432439
return new MicrometerCollector.Family<>(conventionName + suffix,
433-
family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id, suffix),
440+
family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots),
441+
getMetadata(conventionName + suffix, id.getDescription()),
434442
new GaugeDataPointSnapshot(value, labels, null));
435443
}
436444

@@ -470,9 +478,9 @@ private void addDistributionStatisticSamples(Meter.Id id, MicrometerCollector co
470478

471479
Exemplars exemplars = createExemplarsWithScaledValues(exemplarsSupplier.get());
472480
families.add(new MicrometerCollector.Family<>(conventionName,
473-
family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id),
474-
new SummaryDataPointSnapshot(count, sum, quantiles, Labels.of(tagKeys, tagValues), exemplars,
475-
0)));
481+
family -> new SummarySnapshot(family.metadata, family.dataPointSnapshots),
482+
getMetadata(conventionName, id.getDescription()), new SummaryDataPointSnapshot(count, sum,
483+
quantiles, Labels.of(tagKeys, tagValues), exemplars, 0)));
476484
}
477485
else {
478486
List<Double> buckets = new ArrayList<>();
@@ -501,8 +509,9 @@ private void addDistributionStatisticSamples(Meter.Id id, MicrometerCollector co
501509
families.add(new MicrometerCollector.Family<>(conventionName,
502510
family -> new io.prometheus.metrics.model.snapshots.HistogramSnapshot(forLongTaskTimer,
503511
family.metadata, family.dataPointSnapshots),
504-
getMetadata(id), new HistogramDataPointSnapshot(ClassicHistogramBuckets.of(buckets, counts),
505-
sum, Labels.of(tagKeys, tagValues), exemplars, 0)));
512+
getMetadata(conventionName, id.getDescription()),
513+
new HistogramDataPointSnapshot(ClassicHistogramBuckets.of(buckets, counts), sum,
514+
Labels.of(tagKeys, tagValues), exemplars, 0)));
506515

507516
// TODO: Add support back for VictoriaMetrics
508517
// Previously we had low-level control so a histogram was just
@@ -515,9 +524,9 @@ private void addDistributionStatisticSamples(Meter.Id id, MicrometerCollector co
515524
}
516525

517526
families.add(new MicrometerCollector.Family<>(conventionName + "_max",
518-
family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots), getMetadata(id, "_max"),
519-
new GaugeDataPointSnapshot(histogramSnapshot.max(getBaseTimeUnit()), Labels.of(tagKeys, tagValues),
520-
null)));
527+
family -> new GaugeSnapshot(family.metadata, family.dataPointSnapshots),
528+
getMetadata(conventionName + "_max", id.getDescription()), new GaugeDataPointSnapshot(
529+
histogramSnapshot.max(getBaseTimeUnit()), Labels.of(tagKeys, tagValues), null)));
521530

522531
return families.build();
523532
});
@@ -549,13 +558,8 @@ private void onMeterRemoved(Meter meter) {
549558
}
550559
}
551560

552-
private MetricMetadata getMetadata(Meter.Id id) {
553-
return getMetadata(id, "");
554-
}
555-
556-
private MetricMetadata getMetadata(Meter.Id id, String suffix) {
557-
String name = config().namingConvention().name(id.getName(), id.getType(), id.getBaseUnit()) + suffix;
558-
String help = prometheusConfig.descriptions() ? Optional.ofNullable(id.getDescription()).orElse(" ") : " ";
561+
private MetricMetadata getMetadata(String name, @Nullable String description) {
562+
String help = prometheusConfig.descriptions() ? Optional.ofNullable(description).orElse(" ") : " ";
559563
// Unit is intentionally not set, see:
560564
// https://github.com/OpenObservability/OpenMetrics/blob/1386544931307dff279688f332890c31b6c5de36/specification/OpenMetrics.md#unit
561565
return new MetricMetadata(name, help, null);

‎implementations/micrometer-registry-prometheus/src/test/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistryTest.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package io.micrometer.prometheusmetrics;
1717

18+
import io.micrometer.common.lang.Nullable;
1819
import io.micrometer.core.Issue;
1920
import io.micrometer.core.instrument.LongTaskTimer.Sample;
2021
import io.micrometer.core.instrument.Timer;
@@ -991,6 +992,50 @@ void noExemplarsIfNoSampler() {
991992
assertThat(scraped).endsWith("# EOF\n");
992993
}
993994

995+
@Test
996+
@Issue("#5229")
997+
void doesNotCallConventionOnScrape() {
998+
CountingPrometheusNamingConvention convention = new CountingPrometheusNamingConvention();
999+
registry.config().namingConvention(convention);
1000+
1001+
Timer.builder("timer").tag("k1", "v1").description("my timer").register(registry);
1002+
Counter.builder("counter").tag("k1", "v1").description("my counter").register(registry);
1003+
DistributionSummary.builder("summary").tag("k1", "v1").description("my summary").register(registry);
1004+
Gauge.builder("gauge", new AtomicInteger(), AtomicInteger::doubleValue)
1005+
.tag("k1", "v1")
1006+
.description("my gauge")
1007+
.register(registry);
1008+
LongTaskTimer.builder("long.task.timer").tag("k1", "v1").description("my long task timer").register(registry);
1009+
1010+
int expectedNameCount = convention.nameCount.get();
1011+
int expectedTagKeyCount = convention.tagKeyCount.get();
1012+
1013+
registry.scrape();
1014+
1015+
assertThat(convention.nameCount.get()).isEqualTo(expectedNameCount);
1016+
assertThat(convention.tagKeyCount.get()).isEqualTo(expectedTagKeyCount);
1017+
}
1018+
1019+
private static class CountingPrometheusNamingConvention extends PrometheusNamingConvention {
1020+
1021+
AtomicInteger nameCount = new AtomicInteger();
1022+
1023+
AtomicInteger tagKeyCount = new AtomicInteger();
1024+
1025+
@Override
1026+
public String name(String name, Meter.Type type, @Nullable String baseUnit) {
1027+
nameCount.incrementAndGet();
1028+
return super.name(name, type, baseUnit);
1029+
}
1030+
1031+
@Override
1032+
public String tagKey(String key) {
1033+
tagKeyCount.incrementAndGet();
1034+
return super.tagKey(key);
1035+
}
1036+
1037+
}
1038+
9941039
private PrometheusMeterRegistry createPrometheusMeterRegistryWithProperties(Properties properties) {
9951040
PrometheusConfig prometheusConfig = new PrometheusConfig() {
9961041
@Override

0 commit comments

Comments
 (0)