dropwizard5 MapperConfig doesn't rename the metric?

[andycoates]

I'm not a java dev so bear with me.. but I think there's an issue with the dropwizard5 MapperConfig use:

https://github.com/prometheus/client_java/blob/main/prometheus-metrics-instrumentation-dropwizard5/src/main/java/io/prometheus/metrics/instrumentation/dropwizard5/labels/MapperConfig.java#L44

This says that you can specify a name to rename the resulting metric (e.g. after matching with wildcards like in the example) - but this doesn't seem to actually do anything?

You can see this in the tests too, e.g.:

https://github.com/prometheus/client_java/blob/main/prometheus-metrics-instrumentation-dropwizard5/src/test/java/io/prometheus/metrics/instrumentation/dropwizard5/labels/CustomLabelMapperTest.java#L107

        final MapperConfig mapperConfig = new MapperConfig(
                "app.okhttpclient.client.HttpClient.*.*",
                "app.okhttpclient.client.HttpClient",
                labels
        );

This should name the metric app.okhttpclient.client.HttpClient according to the code comments, but in the tests you can see it tests that it's equal to its original name and not the set name:

        String expected = "# TYPE app_okhttpclient_client_HttpClient_greatService_400 counter\n" +

                                                                     ^^^^^^^^^^^^^^^^  this wouldn't be here if it was renamed

I also see this in practice when writing my own code too - just the test makes it easy to verify. The labels work, just the name doesn't seem to ever get referenced and actually used as a rename.

Is this a known issue or limitation?

Thanks!

[fstab]

Thanks for creating the issue!

@kingster do you want to have a look?

[kingster]

Hmm, thanks for pointing this out. This looks like a bug which happened during the migration, let me raise a pr to fix this.

[kingster]

Hi @andycoates Have attempted the fix in #949 Can you please checkout the test-cases and verify if that is what is expected?

[andycoates]

Hey @kingster thanks for doing this. In the example test I referenced (and the intent of the code comment for setting name) that one is what you'd expect now - but looking at the other test cases I don't think they're doing what is expected. Will review and test more locally to verify.