Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MeterRegistry: same meter name with different tags is not reflected in /actuator/prometheus #42830

Closed
bgalek opened this issue Oct 22, 2024 · 7 comments
Assignees
Labels
for: external-project For an external project and not something we can fix status: invalid An issue that we don't feel is valid

Comments

@bgalek
Copy link

bgalek commented Oct 22, 2024

Setup

Let's setup a simple spring-boot app:

build.gradle.kts

plugins {
    java
    id("org.springframework.boot") version "3.3.4"
    id("io.spring.dependency-management") version "1.1.6"
}

java {
    toolchain {
        languageVersion = JavaLanguageVersion.of(23)
    }
}

repositories {
    mavenCentral()
}

dependencies {
    implementation("org.springframework.boot:spring-boot-starter-actuator")
    implementation("org.springframework.boot:spring-boot-starter-web")
    runtimeOnly("io.micrometer:micrometer-registry-prometheus")
}

App.java

@SpringBootApplication
public class TestPrometheusMetricsApplication {

    public static void main(String[] args) {
        SpringApplication.run(TestPrometheusMetricsApplication.class, args);
    }

    @Configuration
    static class TestConfiguration {

        @Bean
        public String test(MeterRegistry meterRegistry) {
            meterRegistry.counter("test_counter", "tag1", "value1");
            meterRegistry.counter("test_counter", "tag1", "valueA", "tag2", "valueB");
            return "ok";
        }
    }
}

application.properties

spring.application.name=test-prometheus-metrics
management.endpoints.web.exposure.include=*

The problem

When visiting http://localhost:8080/actuator/metrics/test_counter, we can see
that availableTags for test_counterwas merged (tag1 and tag2). That seems a sensible thing to do in our example.

{
    "name": "test_counter",
    "measurements": [
        {
            "statistic": "COUNT",
            "value": 0
        }
    ],
    "availableTags": [
        {
            "tag": "tag1",
            "values": [
                "value1",
                "valueA"
            ]
        },
        {
            "tag": "tag2",
            "values": [
                "valueB"
            ]
        }
    ]
}

But Prometheus scrape endpoint http://localhost:8080/actuator/prometheus
has only one first counter registered!

# HELP test_counter_total  
# TYPE test_counter_total counter
test_counter_total{tag1="value1"} 1.0

Expected behavior

This behavior seems unintuitive since /actuator/prometheus returns different data than /actuator/metrics/test_counter.
There is no log (on the debug level) that the second meter was ignored.

We feel that /actuator/prometheus should behave the same as /actuator/metrics/test_counter and include merged tags for each registered meter.

If it's a limitation of Prometheus itself, we would expect some kind of feedback for spring boot users—an exception that it's not allowed or a log saying that the meter was overridden/ignored.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 22, 2024
@wilkinsona wilkinsona self-assigned this Oct 22, 2024
@wilkinsona
Copy link
Member

wilkinsona commented Oct 22, 2024

Spring Boot itself knows very little about Prometheus and any limitations of its scrape format as it is handled by Micrometer and the underlying Prometheus library.

You can observe the behavior that you've described by using Micrometer's PrometheusMeterRegistry directly:

PrometheusMeterRegistry meterRegistry = new PrometheusMeterRegistry(PrometheusConfig.DEFAULT);
		meterRegistry.counter("test_counter", "tag1", "value1");
meterRegistry.counter("test_counter", "tag1", "valueA", "tag2", "valueB");
        
System.out.println(meterRegistry.scrape());
        
meterRegistry.getMeters().forEach((meter) -> System.out.println(meter.getId()));

The above will produce the following output:

# HELP test_counter_total  
# TYPE test_counter_total counter
test_counter_total{tag1="value1"} 0.0

MeterId{name='test_counter', tags=[tag(tag1=value1)]}
MeterId{name='test_counter', tags=[tag(tag1=valueA),tag(tag2=valueB)]}

As you can see, the scrape result includes test_counter only once whereas looking at the meters individual, as Boot's MetricsEndpoint does, shows both.

If you believe that some improvements could be made in this area, please open a Micrometer issue.

@wilkinsona wilkinsona closed this as not planned Won't fix, can't repro, duplicate, stale Oct 22, 2024
@wilkinsona wilkinsona added status: invalid An issue that we don't feel is valid for: external-project For an external project and not something we can fix and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 22, 2024
@jonatan-ivanov
Copy link
Member

fyi: micrometer-metrics/micrometer#877
In your case you can do this instead:

meterRegistry.counter("test_counter", "tag1", "value1", "tag2", "none");
meterRegistry.counter("test_counter", "tag1", "valueA", "tag2", "valueB");

Or you can use na, null, etc. to indicate that the value does not exists.

@bgalek
Copy link
Author

bgalek commented Oct 24, 2024

@jonatan-ivanov @wilkinsona
Thank you for the context.

Still, it could make sense to inform the user of spring-boot what happened.
There could be an autoconfiguration could add onMeterRegistrationFailed handler to throw an error or at least log when

public class PrometheusReporterConfiguration {
private static final Logger logger = LoggerFactory.getLogger(PrometheusReporterConfiguration.class);
...
private void configureLoggerMetrics(PrometheusMeterRegistry prometheusMeterRegistry) {
        prometheusMeterRegistry.config()
            .onMeterRegistrationFailed((id, msg) -> logger.error("Failed to register meter: {}, message: {}", id, msg));
        ...
    }
}

WDYT?

@wilkinsona
Copy link
Member

I don't think such functionality belongs in Boot as the behaviour isn't specific to Boot. As demonstrated above, the behavior is reproducible with only Micrometer. If some logging is to be done anywhere, IMO, it should be done in Micrometer.

@bgalek
Copy link
Author

bgalek commented Oct 24, 2024

@wilkinsona ok, thx! I'll go there :)

@jonatan-ivanov
Copy link
Member

Sorry for increasing the noise here but what you need might be already done in Micrometer, see: micrometer-metrics/micrometer#5228
You can try Micrometer 1.14.0-RC1 (Boot 3.4.0-RC1) or wait till 1.14.0 (around 11th November) and try that:

@bgalek
Copy link
Author

bgalek commented Oct 25, 2024

omg @jonatan-ivanov that's perfect, thank you for that link :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: external-project For an external project and not something we can fix status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

4 participants