Micrometer metric names

[flozano]

Hi !

I am looking at the metric names in MicrometerApnsClientMetricsListener and they look a little too generic:

    /**
     * The name of a {@link io.micrometer.core.instrument.Timer} that measures round-trip time when sending
     * notifications.
     */
    public static final String NOTIFICATION_TIMER_NAME = "notifications.sent.timer";

    /**
     * The name of a {@link io.micrometer.core.instrument.Counter} that measures the number of write failures when
     * sending notifications.
     */
    public static final String WRITE_FAILURES_COUNTER_NAME = "notifications.failed";

    /**
     * The name of a {@link io.micrometer.core.instrument.Counter} that measures the number of notifications sent
     * (regardless of whether they're accepted or rejected by the server).
     */
    public static final String SENT_NOTIFICATIONS_COUNTER_NAME = "notifications.sent";

    /**
     * The name of a {@link io.micrometer.core.instrument.Counter} that measures the number of notifications accepted by
     * the APNs server.
     */
    public static final String ACCEPTED_NOTIFICATIONS_COUNTER_NAME = "notifications.accepted";

    /**
     * The name of a {@link io.micrometer.core.instrument.Counter} that measures the number of notifications rejected by
     * the APNs server.
     */
    public static final String REJECTED_NOTIFICATIONS_COUNTER_NAME = "notifications.rejected";

    /**
     * The name of a {@link io.micrometer.core.instrument.Gauge} that measures the number of open connections in an APNs
     * client's internal connection pool.
     */
    public static final String OPEN_CONNECTIONS_GAUGE_NAME = "connections.open";

    /**
     * The name of a {@link io.micrometer.core.instrument.Counter} that measures the number of a client's failed
     * connection attempts.
     */
    public static final String CONNECTION_FAILURES_COUNTER_NAME = "connections.failed";

connections.failed or notifications.accepted doesn't look very APNS-specific at all. Tags is not really an option as the metric is very different from other kind of "connections" or "notifications" I may have in my app (maybe push, maybe other kind of stuff).

it'd be nice to be able to add a prefix to the metric names so that they can be "namespaced" in "apns". Is there an easy way to achieve it? I'm thinking in wrapping the MeterRegistry I pass to pushy, but it'd be better probably if the library allowed it directly.

[jchambers]

I think this is largely covered by #666 (which hasn't been resolved yet). You might also be interested in #679.

In short, the existing Micrometer metrics reporter is pretty terrible. I wrote it before I had any significant exposure to Micrometer and really understood what it was doing with tags or naming and made some bad assumptions about names plugging into a deeper hierarchy (which is the case with Dropwizard metrics).

I've been mostly deferring work on this until #540 is done because I think that will drastically change the "shape" of metrics reporting (for the better!). I promise I'm still chipping away at that, even though world events have slowed me down a bit lately.

I recognize that's not a super satisfying answer, but I hope it sheds some light on the situation! I think the criticism of the existing implementation is completely valid, and I agree that it needs to change.

[flozano]

No problem! I may address my current problem with some copy and paste, and defer the proper solution until it's ready 👍👍