Custom ConnectionProvider.MeterRegistrar cannot collect pending.connections.time

[ejhnsn]

Motivation

When a custom ConnectionProvider.MeterRegistrar is supplied via metrics(true, registrarSupplier), the reactor.netty.connection.provider.pending.connections.time timer is never recorded. A custom registrar can reproduce the six pool gauges (total/active/idle/pending/max/max.pending.connections) from the ConnectionPoolMetrics handle it's given, but it has no way to obtain the pending-acquire latency samples, so that one metric silently disappears for anyone using a custom registrar.

This is surprising because the built-in registrar exposes the timer, and nothing in the reference docs or the metrics(boolean, Supplier) Javadoc (which says "All generated metrics are provided to the specified registrar") hints that supplying your own registrar drops it.

Why it happens (verified against main, 1.3.6-SNAPSHOT):

The built-in and custom paths are mutually exclusive. In PooledConnectionProvider#newPool the factory path is chosen by whether a custom registrar is present:

// PooledConnectionProvider.java
InstrumentedPool<T> newPool = metricsEnabled && poolFactory.registrar == null && Metrics.isMicrometerAvailable() ?
        createPool(id, config, poolFactory, remoteAddress, resolverGroup) :   // built-in path (id != null)
        createPool(config, poolFactory, remoteAddress, resolverGroup);         // custom-registrar path

Only the built-in path (id != null) installs the recorder, in DefaultPooledConnectionProvider.PooledConnectionAllocator:

// DefaultPooledConnectionProvider.java
this.pool = id == null ?
        provider.newPool(connectChannel(), null, DEFAULT_DESTROY_HANDLER, DEFAULT_EVICTION_PREDICATE) :          // no PoolMetricsRecorder
        provider.newPool(connectChannel(), DEFAULT_DESTROY_HANDLER, DEFAULT_EVICTION_PREDICATE,
                new MicrometerPoolMetricsRecorder(id, name, remoteAddress));                                     // recorder installed

MicrometerPoolMetricsRecorder is the only thing that turns reactor-pool's recordPendingSuccessAndLatency / recordPendingFailureAndLatency push callbacks into the pending.connections.time Timer. With a custom registrar, no PoolMetricsRecorder is installed, so those callbacks go nowhere. Meanwhile the custom registrar's only handle — ConnectionPoolMetrics — is purely instantaneous gauges (acquiredSize / allocatedSize / idleSize / pendingAcquireSize / maxAllocatedSize / maxPendingAcquireSize), with no pending-acquire-time accessor and no callback to receive samples.

pending.connections.time was added later (#2946 / #2980, reactor-pool #178) and wired only into MicrometerPoolMetricsRecorder. The custom-registrar SPI (MeterRegistrar + ConnectionPoolMetrics) predates it and was never widened to surface it. Conceptually the registrar model is pull-based (poll a gauge snapshot) while pending-acquire-time is push-based (Timer samples), so it can't be expressed as a ConnectionPoolMetrics getter — the custom path needs access to the push samples.

Desired solution

Give a custom MeterRegistrar access to the pending-acquire-time samples in a backwards-compatible way:

• Add default (no-op) methods to ConnectionProvider.MeterRegistrar, e.g. recordPendingAcquireSuccess(String poolName, String id, SocketAddress remoteAddress, long latencyMs) and recordPendingAcquireFailure(...). Defaults keep every existing implementation source- and behavior-compatible.
• On the custom-registrar branch, install a thin PoolMetricsRecorder that forwards recordPendingSuccessAndLatency / recordPendingFailureAndLatency to those new registrar methods, so a custom registrar can build its own Timer (with its own tags).

This mirrors precedent already in the codebase: the HTTP-level public extension point (HttpClientMetricsRecorder) is push-based with record* methods; the connection-pool registrar simply never got the push half.

I'd be glad to open a PR along these lines if the direction sounds reasonable.

Considered alternatives

• Widen ConnectionPoolMetrics with a pendingAcquireTime() getter — doesn't fit; pending-acquire-time is a distribution of per-acquire latencies (a Timer), not an instantaneous value you can poll.
• A separate opt-in sub-interface (e.g. MeterRegistrar implements an additional PendingTimeAware interface) instead of default methods on MeterRegistrar — happy to go this route instead if you'd prefer to keep MeterRegistrar minimal.

Additional context

• Versions: reactor-netty-core main (1.3.6-SNAPSHOT); behavior is identical on the 1.3.x line. reactor-pool 1.2.x.
• Reproduction:
  1. Build a ConnectionProvider with .metrics(true, () -> myCustomRegistrar).
  2. Drive enough concurrent load (small maxConnections, short pendingAcquireTimeout) to force pending acquisitions.
  3. Observe: reactor.netty.connection.provider.{total,active,idle,pending,max,max.pending}.connections are present, but reactor.netty.connection.provider.pending.connections.time is absent.
  4. Switch to .metrics(true) (built-in registrar) and the timer reappears.
• Our use case (for color): we maintain a custom registrar purely to normalize the remote.address tag (our hosts sit behind shared edges; we align remote.address with the bare-host value space used elsewhere in our metrics). Adopting a custom registrar silently dropped pending.connections.time, which is one of the more useful pool-saturation signals — hence this report.

[violetagg]

@ejhnsn Would you like to provide a PR?

[ejhnsn]

@ejhnsn Would you like to provide a PR?

Yes, I will.