The R2DBC connection pool metrics exposed by Actuator are currently very limited, we only get a few gauges about the current pool status as exposed by io.r2dbc.pool.PoolMetrics. Other important metrics like number and latency of connection acquire/allocation operations are missing.
I looked through the R2DBC ConnectionPool source code and found that the ConnectionFactory.Builder has a method to accept a reactor.pool.PoolMetricsRecorder, which can be used to track those metrics.
Would you consider integrating this with Actuator? It looks like it should be pretty straight-forward to implement a PoolMetricsRecorder that uses Micrometer's MeterRegistry to track those metrics.
Comment From: wilkinsona
Thanks for the suggestion, @kzander91.
Given that Reactor depends on Micrometer, it may make sense for Reactor to provide a Micrometer-based implementation of PoolMetricsRecorder that we could reuse with R2DBC. WDYT, @simonbasle, @mp911de?
Comment From: mp911de
Likely, an implementation within Reactor Pool would be beneficial to all users of the Reactor Pool library in contrast to a Micrometer integration in the R2DBC Pool.
Comment From: wilkinsona
Thanks, @mp911de. That's what I was trying to describe in my comment above. I don't think there's anything that's R2DBC-specific here so a general integration in Reactor Pool with some configuration options for the meter name prefix, tags, and so on seems to make the must sense to me.
Comment From: simonbasle
@wilkinsona reactor-pool doesn't explicitly depend on Micrometer, nor does reactor-core (optional dependency).
Since we have an intermediary abstraction PoolMetricsRecorder in the reactor-pool, this is probably feasible for an opinionated simple version (no input parameters).
But we've been bitten by that sort of choice before. Notably, not providing a MeterRegistry and relying on Metrics.globalRegistry()... And we don't want Micrometer APIs to leak into the main pool API, so it becomes tricky as we want to add support for more stuff like tags, meter filtering, custom MeterRegistry, etc...
@mp911de side question: does the R2DBC pool expose configuration elements from reactor-pool? ie. could it let users provide a PoolMetricsRecorder?
Comment From: mp911de
ie. could it let users provide a PoolMetricsRecorder?
Yes.
Comment From: philwebb
I've raised https://github.com/reactor/reactor-pool/issues/161. We'll leave this one open but blocked to see if we can provide integration if the issue is accepted.
Comment From: simonbasle
pinging people here as well as in the PR: I've created a PR that adds a reactor-pool-micrometer module for the upcoming major release that will go alongside Spring Framework 6 and Spring Boot 3.
https://github.com/reactor/reactor-pool/pull/164
I'm after feedback on the choices around meter names, tags and types. see notably this enum and this test
Comment From: wilkinsona
Thanks, @simonbasle. I think it would be good for @shakuzen, @jonatan-ivanov, @marcingrzejszczak to cast an expert eye over this.
Comment From: simonbasle
this has been merged and released today in a new submodule of reactor/reactor-pool io.projectreactor.addons:reactor-pool-micrometer:0.1.0-RC1
Comment From: Riyon
Hi @wilkinsona do you know what's the status of this improvement?
Comment From: wilkinsona
All of the status is already here: - the issue is open, indicating that it's still of interest - the issue is in the general backlog, indicating that we have no plans at this time to implement it
If you’d like to see something sooner rather than later (I assume that’s why you are asking), your best path forward is to independently research what needs to be done and to submit a pull request.