Is it a good practice to define constant strings to a listener?

[itisbugra]

In file MicrometerApnsClientMetricsListener.java, the names for the metrics are defined with constant strings. Should we follow up with a cleaner approach, perhaps using messages from a ResourceBundle, or at least providing a way to override those fields with a Map<String, String> , given in constructor (or anywhere)?

I would like to organize metrics like Spring Boot does, such as jvm.memory... or tomcat.threads.... It is not easy to infer the context from notifications.failed.

I can shoot a pull request, maybe try it on my way. Any thoughts?

[jchambers]

I'll confess Micrometer isn't my first choice for a metrics library and am not as familiar with some of the details as I'd like to be. That said, in other similar systems, the approach would be to register all of these metrics within some other category. Using a completely made-up API:

final MetricRegistry pushNotificationMetrics =
    new MetricRegistry("push.notification.custom.prefix.you.get.the.idea");

pushNotificationMetrics.registerAll(metricsFromPushy);

…then you'd wind up with metrics with names like push.notification.custom.prefix.you.get.the.idea.notifications.failed. Let me read up a bit to see if that's something Micrometer knows how to do (I assumed it was). I'd much prefer that approach over custom names for all of the metrics.

[itisbugra]

You're right about constructing a new MetricRegistry to prefix all of the instrument labels, however for people using Spring Boot Actuator, the MeterRegistry (which is different from MetricRegistry) object will be provided in the DI context, and you might not be available for instantiate a new one.

That may seem like a duplication of code for some reason, though I could not find such way to do that, alternatively.

[jchambers]

Could you get away with a MeterFilter? As a last resort, we could think about adding a prefix argument to the MicrometerApnsClientMetricsListener constructor. Both of those would seem a lot simpler (and safer) than full customization.

[itisbugra]

I could not assert it is safer, but it is simpler indeed. In my case, a prefix would be more than okay. Should you we implement this in #667? This looks like a v14.0 feature.

[itisbugra]

Could you get away with a MeterFilter? As a last resort, we could think about adding a prefix argument to the MicrometerApnsClientMetricsListener constructor. Both of those would seem a lot simpler (and safer) than full customization.

You meant this API?

[jchambers]

I was actually thinking MeterFilter#map(Meter.Id).

[filiphr]

Using the MeterFilter is doable. However, when using Spring Boot the MeterRegistry is already preconfigured. You can still use the MeterFilter, but you would need to be careful not to change other metrics.

If possible I would go with the approach to pass a prefix to the MicrometerApnsClientMetricsListener as what I want now is just to have pushy. in front of the metrics. I would even go so far and say the they should already be prefixed with pushy..

Have a look at this for example:

connections.failed
connections.open
hikaricp.connections
hikaricp.connections.acquire
hikaricp.connections.active
hikaricp.connections.creation
...
notifications.accepted
notifications.failed
notifications.rejected
notifications.sent
notifications.sent.timer
...
tomcat.sessions.alive.max
tomcat.sessions.created
tomcat.sessions.expired
tomcat.sessions.rejected
...

There are more examples from Spring Boot, but my goal is to show that only the ones from Pushy are not that descriptive. If there was pushy. in the name it would be immediately clear where they are coming from.

[jchambers]

I've added a pushy. prefix in #1046; with apologies for the delay, I think that (in combination with filters) should get the job done.