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

Remove _total suffix in micrometer using NamingConvention not working in 2.5.0 #21758

Closed
CarlosPanarello opened this issue Nov 28, 2021 · 6 comments
Labels
area/metrics kind/bug Something isn't working

Comments

@CarlosPanarello
Copy link
Contributor

Describe the bug

I like to remove some suffix in some name metrics, i made it using NamingConvention from micrometer and set it in PrometheusMeterRegistry.
This was working just fine in quarkus 1.13.7, but when i change it to quarkus 2.5.0 doesn't.

Maybe this is a change in micrometer lib or in quarkus?

Thanks

Expected behavior

In quarkus 1.13.7 using micrometer-quickstart example with my namingConvention to remove _total suffix from counter metrics works just fine.

Actual behavior

In quarkus 2.5.0 using micrometer-quickstart example with my namingConvention to remove _total suffix from counter metrics doesn't remove suffix like quarkus 1.13.7.

How to Reproduce?

Reproducer:

Steps to reproduce the behavior:
1. Clone my [project](https://github.com/quarkusio/quarkus-quickstarts/tree/main/micrometer-quickstart) from micrometer-quickstart
1. Remove the comments to change quarkus version (test with quarkus 1.13.7 and quarkus 2.5.0)
2. Call endpoint http://localhost:8080/example/prime/31 or http://localhost:8080/example/prime/70 
3. Call endpoint http://localhost:8080/q/metrics search for example_prime_number metric 

Output of uname -a or ver

Darwin MacBook-Pro.local 20.6.0 Darwin Kernel Version 20.6.0: Mon Aug 30 06:12:21 PDT 2021; root:xnu-7195.141.6~3/RELEASE_X86_64 x86_64

Output of java -version

openjdk version "11.0.2" 2019-01-15

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.5.0

Build tool (ie. output of mvnw --version or gradlew --version)

apache-maven-3.8.1

Additional information

No response

@CarlosPanarello CarlosPanarello added the kind/bug Something isn't working label Nov 28, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Nov 28, 2021

/cc @ebullient

@ebullient
Copy link
Contributor

ebullient commented Nov 28, 2021

The PrometheusNamingConvention doesn't have a lot of flexibility over appending _total:

https://github.com/micrometer-metrics/micrometer/blob/8e7f66670cbf33189f9aa3af99943444c5f2643a/implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheus/PrometheusNamingConvention.java#L58

The default construction of the PrometheusMeterRegistry sets the NamingConvention directly (making it hard for you to override via config:

https://github.com/micrometer-metrics/micrometer/blob/8e7f66670cbf33189f9aa3af99943444c5f2643a/implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheus/PrometheusMeterRegistry.java#L66

You could open an issue against micrometer itself. I would suggesta test to see if a NamingConvention has been set on the configuration before setting that one.


To address this now, I think you will need to construct your own PrometheusMeterRegistry that sets the naming convention that you want (that does not add the conventional _total suffix).

This is how the micrometer extension produces the PrometheusMeterRegistry (you'll want to disable this default instance):

public PrometheusMeterRegistry registry(PrometheusConfig config, CollectorRegistry collectorRegistry, Clock clock) {

@CarlosPanarello
Copy link
Contributor Author

CarlosPanarello commented Nov 29, 2021

The default construction of the PrometheusMeterRegistry sets the NamingConvention directly (making it hard for you to override via config:

I created my own NamingConvention called NamigMetric, and set it in PrometheusRegistry when the app started and replacing the namingConvenction in PromethesMeterRegistry.

I tried with new Provider, but the result was the same.

You could open an issue against micrometer itself. I would suggesta test to see if a NamingConvention has been set on the configuration before setting that one.

Yes, it has been defined and executed.

I debuged and the only namingConvention that is running is my, even so the suffix _total is added.

I'm looking where else it is adding the suffix _total besides PrometheusNamingConvention, any suggestion ?

Another diference, is that , in quarkus 1, the version of micrometer-core was 1.6.5 in quarkus 2 is 1.7.5.

@CarlosPanarello
Copy link
Contributor Author

CarlosPanarello commented Nov 29, 2021

@ebullient I make some tests and make a downgrade to version 1.6.5 of the micrometer-core and micrometer-prometheus registry, that is used in quarkus 1 and worked just fine in quarkus 2.

I will open an issue in micrometer about it.

i made the replace in pom like this in my project

<dependencies>
        <dependency>
            <artifactId>quarkus-micrometer</artifactId>
            <groupId>io.quarkus</groupId>
            <exclusions>
                <exclusion>
                    <groupId>io.micrometer</groupId>
                    <artifactId>micrometer-core</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>io.micrometer</groupId>
                    <artifactId>micrometer-registry-prometheus</artifactId>
                </exclusion>
            </exclusions>
        </dependency>
    <dependency>
        <groupId>io.micrometer</groupId>
        <artifactId>micrometer-core</artifactId>
        <version>1.6.5</version>
    </dependency>
        <dependency>
            <groupId>io.micrometer</groupId>
            <artifactId>micrometer-registry-prometheus</artifactId>
            <version>1.6.5</version>
        </dependency>
        <dependency>
            <groupId>io.quarkus</groupId>
            <artifactId>quarkus-micrometer-registry-prometheus</artifactId>
            <exclusions>
                <exclusion>
                    <groupId>io.micrometer</groupId>
                    <artifactId>micrometer-core</artifactId>
                </exclusion>
                <exclusion>
                    <groupId>io.micrometer</groupId>
                    <artifactId>micrometer-registry-prometheus</artifactId>
                </exclusion>
            </exclusions>
        </dependency>

@CarlosPanarello
Copy link
Contributor Author

I find the reason, in micrometer version 1.7.0 and above, they are using prometheus-simple-client 0.10.0, and in that version they made change to add total in counter.

But looking at some issues about it the solution isn't good either, they suggest to add a scrape config in prometheus, for me this is not a good solution because we have many prometheus so we have to add this config to keep working grafana's dashboards if we want to change to any version in quarkus 2.

@ebullient do you think it is a good idea to add a filter for scrap, or this will be heavy in run time?

@ebullient
Copy link
Contributor

Using a filter with the scrape will be fine, as that is running elsewhere (in Prometheus-land, not in your app). This kind of scrape-config adjustment is pretty common.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics kind/bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants