Skip to content

Commit

Permalink
Do not set explicit unit on Prometheus Metadata
Browse files Browse the repository at this point in the history
We should not set Unit in the Metadata since if it is set, it must be a
suffix of the MetricFamily name separated by an underscore.
See OpenMetrics spec:
https://github.com/OpenObservability/OpenMetrics/blob/1386544931307dff279688f332890c31b6c5de36/specification/OpenMetrics.md#unit

This can be problematic since Micrometer adds the unit to the name
but not necessarily as a suffix. For example, Micrometer does this:
test_seconds_max
but if Unit is present the name should be
test_max_seconds

The difference is not having the invalid unit definition on the output:
# UNIT test_seconds_max seconds
which would cause scraping errors on the server side
(the Prometheus 0.x registry also does this).

Closes micrometer-metricsgh-5038
  • Loading branch information
jonatan-ivanov committed May 3, 2024
1 parent c63b93a commit 3512349
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -556,8 +556,9 @@ private MetricMetadata getMetadata(Meter.Id 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(" ") : " ";
Unit unit = id.getBaseUnit() != null ? new Unit(id.getBaseUnit()) : null;
return new MetricMetadata(name, help, unit);
// Unit is intentionally not set, see:
// https://github.com/OpenObservability/OpenMetrics/blob/1386544931307dff279688f332890c31b6c5de36/specification/OpenMetrics.md#unit
return new MetricMetadata(name, help, null);
}

private void applyToCollector(Meter.Id id, Consumer<MicrometerCollector> consumer) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -844,13 +844,14 @@ void meterTriggeringAnotherMeterWhenCollectingValue() {
void openMetricsScrape() {
Counter.builder("my.counter").baseUnit("bytes").register(registry);
Timer.builder("my.timer").register(registry);
assertThat(registry.scrape("application/openmetrics-text; version=1.0.0; charset=utf-8"))
.contains("# TYPE my_counter_bytes counter\n" + "# UNIT my_counter_bytes bytes\n"
+ "# HELP my_counter_bytes \n" + "my_counter_bytes_total 0.0\n")
.contains("# TYPE my_timer_seconds_max gauge\n" + "# UNIT my_timer_seconds_max seconds\n"
+ "# HELP my_timer_seconds_max \n" + "my_timer_seconds_max 0.0\n")
.contains("# TYPE my_timer_seconds summary\n" + "# UNIT my_timer_seconds seconds\n"
+ "# HELP my_timer_seconds \n" + "my_timer_seconds_count 0\n" + "my_timer_seconds_sum 0.0\n")
String result = registry.scrape("application/openmetrics-text; version=1.0.0; charset=utf-8");
assertThat(result).doesNotContain("# UNIT")
.contains("# TYPE my_counter_bytes counter\n" + "# HELP my_counter_bytes \n"
+ "my_counter_bytes_total 0.0\n")
.contains("# TYPE my_timer_seconds_max gauge\n" + "# HELP my_timer_seconds_max \n"
+ "my_timer_seconds_max 0.0\n")
.contains("# TYPE my_timer_seconds summary\n" + "# HELP my_timer_seconds \n" + "my_timer_seconds_count 0\n"
+ "my_timer_seconds_sum 0.0\n")
.endsWith("# EOF\n");
}

Expand Down

0 comments on commit 3512349

Please sign in to comment.