Include uniqueName in the metric names

[coryjamesfisher]

Hello and thank you guys for your great work. Flexypool is amazingly helpful. My problem is that I have multiple connection pools(different datasources) and they all want to use the same metric name since they aren't factoring the uniqueName into it. I was able to figure out how to do this on my own, it just felt a bit more difficult than I'd like and it seems like including the uniqueName in the metric name might be good default behavior. Perhaps I am missing an easier way to do it?

For reference, here's how I got around it:

 configurationBuilder.setMetricsFactory(new MetricsFactory() {
  public Metrics newInstance(com.vladmihalcea.flexypool.common.ConfigurationProperties configurationProperties) {

   CompositeMeterRegistry globalRegistry = io.micrometer.core.instrument.Metrics.globalRegistry;

   return new MicrometerMetrics(configurationProperties, globalRegistry){
     @Override
     public Histogram histogram(String name) {
       return super.histogram(poolName + StringUtils.capitalize(name));
     }

     @Override
     public Timer timer(String name) {
       return super.timer(poolName + StringUtils.capitalize(name));
     }
   };
  }
});

[vladmihalcea]

Thanks for the tip. You should send me a Pull Request with a proposal for this fix.

[coryjamesfisher]

Thanks for the tip. You should send me a Pull Request with a proposal for this fix.

I'll see what I can come up with... my solution above was just a quick get it working example.

[coryjamesfisher]

@vladmihalcea Quick Question on implementation. Do you think it would be better to: a. introduce a MetricNamingStrategy construct here (Along with a MetricNamingStrategyDatasourcePrefix/None)

b. add a boolean flag to turn on/off datasource prefixing

c. add a string parameter for the prefix?

d. extend MicrometerMetrics or AbstractMetrics to include the overriden methods

e. some other solution you can think of?

Since I'm not familiar with the goals of the project I don't want to make any assumptions. Thanks!

[vladmihalcea]

An MBeanNamingStrategy is better. Instead of renaming each metric, it's better to provide a unique name for the JMX MBean.

[vladmihalcea]

I tested it with Dropwizard Metrics, and everything works fine in that case.

The uniqueName provided when configuring FlexyPool is used for the JMX MBean:

jmxReporter = JmxReporter
                .forRegistry(metricRegistry)
                .inDomain(MetricRegistry.name(getClass(), configurationProperties.getUniqueName()))
                .build();

The same thing should be done for Micrometer.

If you need to have them in the logs, then you can add a MetricNamingStrategy which takes the name and the Configuration as parameters and returns a String. The MetricNamingStrategy could be set also via the Configuration. A UniqueMetricNamingStrategy implementation should just add the uniqueName prefix before the default metric name.

[vladmihalcea]

Fixed and released in 2.2.2 version.