Finish consolidating connection pool metrics (#1992)

When I first refactored connection pool metrics, I left some
`connectionPool*` metrics in `ConnectionPoolHandler` for simplicity. In
this PR I moved those metrics into `ConnectionPoolMetrics`

Author: jguerra

zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/ConnectionPoolHandler.java

@@ -19,9 +19,8 @@
 import static com.netflix.netty.common.HttpLifecycleChannelHandler.CompleteEvent;
 import static com.netflix.netty.common.HttpLifecycleChannelHandler.CompleteReason;
 
-import com.netflix.spectator.api.Counter;
+import com.netflix.spectator.api.Spectator;
 import com.netflix.zuul.netty.ChannelUtils;
-import com.netflix.zuul.netty.SpectatorUtils;
 import com.netflix.zuul.origins.OriginName;
 import io.netty.channel.ChannelDuplexHandler;
 import io.netty.channel.ChannelHandler;
@@ -30,6 +29,7 @@
 import io.netty.handler.codec.http.HttpResponse;
 import io.netty.handler.ssl.SslCloseCompletionEvent;
 import io.netty.handler.timeout.IdleStateEvent;
+import java.util.Objects;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -42,26 +42,18 @@
 public class ConnectionPoolHandler extends ChannelDuplexHandler {
     private static final Logger LOG = LoggerFactory.getLogger(ConnectionPoolHandler.class);
 
-    public static final String METRIC_PREFIX = "connectionpool";
-
+    private final ConnectionPoolMetrics metrics;
     private final OriginName originName;
-    private final Counter idleCounter;
-    private final Counter inactiveCounter;
-    private final Counter errorCounter;
-    private final Counter headerCloseCounter;
-    private final Counter sslCloseCompletionCounter;
 
+
+    @Deprecated
     public ConnectionPoolHandler(OriginName originName) {
-        if (originName == null) {
-            throw new IllegalArgumentException("Null originName passed to constructor!");
-        }
-        this.originName = originName;
-        this.idleCounter = SpectatorUtils.newCounter(METRIC_PREFIX + "_idle", originName.getMetricId());
-        this.inactiveCounter = SpectatorUtils.newCounter(METRIC_PREFIX + "_inactive", originName.getMetricId());
-        this.errorCounter = SpectatorUtils.newCounter(METRIC_PREFIX + "_error", originName.getMetricId());
-        this.headerCloseCounter = SpectatorUtils.newCounter(METRIC_PREFIX + "_headerClose", originName.getMetricId());
-        this.sslCloseCompletionCounter =
-                SpectatorUtils.newCounter(METRIC_PREFIX + "_sslClose", originName.getMetricId());
+        this(ConnectionPoolMetrics.create(Objects.requireNonNull(originName), Spectator.globalRegistry()));
+    }
+
+    public ConnectionPoolHandler(ConnectionPoolMetrics metrics) {
+        this.originName = metrics.originName();
+        this.metrics = metrics;
     }
 
     @Override
@@ -71,7 +63,7 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc
 
         if (evt instanceof IdleStateEvent) {
             // Log some info about this.
-            idleCounter.increment();
+            metrics.idleCounter().increment();
             String msg = "Origin channel for origin - " + originName + " - idle timeout has fired. "
                     + ChannelUtils.channelInfoForLogging(ctx.channel());
             closeConnection(ctx, msg);
@@ -88,7 +80,7 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc
                         String msg = "Origin channel for origin - " + originName
                                 + " - completed because of expired keep-alive. "
                                 + ChannelUtils.channelInfoForLogging(ctx.channel());
-                        headerCloseCounter.increment();
+                        metrics.headerCloseCounter().increment();
                         closeConnection(ctx, msg);
                     } else {
                         conn.setConnectionState(PooledConnection.ConnectionState.WRITE_READY);
@@ -101,7 +93,7 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc
                 closeConnection(ctx, msg);
             }
         } else if (evt instanceof SslCloseCompletionEvent event) {
-            sslCloseCompletionCounter.increment();
+            metrics.sslCloseCompletionCounter().increment();
             String msg = "Origin channel for origin - " + originName + " - received SslCloseCompletionEvent " + event
                     + ". " + ChannelUtils.channelInfoForLogging(ctx.channel());
             closeConnection(ctx, msg);
@@ -111,7 +103,7 @@ public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exc
     @Override
     public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
         // super.exceptionCaught(ctx, cause);
-        errorCounter.increment();
+        metrics.errorCounter().increment();
         String mesg = "Exception on Origin channel for origin - " + originName + ". "
                 + ChannelUtils.channelInfoForLogging(ctx.channel()) + " - "
                 + cause.getClass().getCanonicalName()
@@ -126,7 +118,7 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws E
     @Override
     public void channelInactive(ChannelHandlerContext ctx) throws Exception {
         // super.channelInactive(ctx);
-        inactiveCounter.increment();
+        metrics.inactiveCounter().increment();
         String msg = "Client channel for origin - " + originName + " - inactive event has fired. "
                 + ChannelUtils.channelInfoForLogging(ctx.channel());
         closeConnection(ctx, msg);

zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/ConnectionPoolMetrics.java

@@ -28,6 +28,7 @@
  * @since 2/26/25
  */
 public record ConnectionPoolMetrics(
+        OriginName originName,
         Counter createNewConnCounter,
         Counter createConnSucceededCounter,
         Counter createConnFailedCounter,
@@ -44,7 +45,12 @@ public record ConnectionPoolMetrics(
         Counter circuitBreakerClose,
         PercentileTimer connEstablishTimer,
         AtomicInteger connsInPool,
-        AtomicInteger connsInUse) {
+        AtomicInteger connsInUse,
+        Counter idleCounter,
+        Counter inactiveCounter,
+        Counter errorCounter,
+        Counter headerCloseCounter,
+        Counter sslCloseCompletionCounter) {
 
     public static ConnectionPoolMetrics create(OriginName originName, Registry registry) {
         Counter createNewConnCounter = newCounter("connectionpool_create", originName, registry);
@@ -66,13 +72,20 @@ public static ConnectionPoolMetrics create(OriginName originName, Registry regis
         Counter closeWrtBusyConnCounter = newCounter("connectionpool_closeWrtBusyConnCounter", originName, registry);
         Counter circuitBreakerClose = newCounter("connectionpool_closeCircuitBreaker", originName, registry);
 
+        Counter idleCounter = newCounter("connectionpool_idle", originName, registry);
+        Counter inactiveCounter = newCounter("connectionpool_inactive", originName, registry);
+        Counter errorCounter = newCounter("connectionpool_error", originName, registry);
+        Counter headerCloseCounter = newCounter("connectionpool_headerClose", originName, registry);
+        Counter sslCloseCompletionCounter = newCounter("connectionpool_sslClose", originName, registry);
+
         PercentileTimer connEstablishTimer = PercentileTimer.get(
                 registry, registry.createId("connectionpool_createTiming", "id", originName.getMetricId()));
 
         AtomicInteger connsInPool = newGauge("connectionpool_inPool", originName, registry);
         AtomicInteger connsInUse = newGauge("connectionpool_inUse", originName, registry);
 
         return new ConnectionPoolMetrics(
+                originName,
                 createNewConnCounter,
                 createConnSucceededCounter,
                 createConnFailedCounter,
@@ -89,7 +102,12 @@ public static ConnectionPoolMetrics create(OriginName originName, Registry regis
                 circuitBreakerClose,
                 connEstablishTimer,
                 connsInPool,
-                connsInUse);
+                connsInUse,
+                idleCounter,
+                inactiveCounter,
+                errorCounter,
+                headerCloseCounter,
+                sslCloseCompletionCounter);
     }
 
     private static Counter newCounter(String metricName, OriginName originName, Registry registry) {

zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/DefaultOriginChannelInitializer.java

@@ -49,7 +49,8 @@ public class DefaultOriginChannelInitializer extends OriginChannelInitializer {
     public DefaultOriginChannelInitializer(ConnectionPoolConfig connPoolConfig, Registry spectatorRegistry) {
         this.connectionPoolConfig = connPoolConfig;
         String niwsClientName = connectionPoolConfig.getOriginName().getNiwsClientName();
-        this.connectionPoolHandler = new ConnectionPoolHandler(connectionPoolConfig.getOriginName());
+        this.connectionPoolHandler = new ConnectionPoolHandler(
+                ConnectionPoolMetrics.create(connPoolConfig.getOriginName(), spectatorRegistry));
         this.httpMetricsHandler = new HttpMetricsChannelHandler(spectatorRegistry, "client", niwsClientName);
         this.nettyLogger = new LoggingHandler("zuul.origin.nettylog." + niwsClientName, LogLevel.INFO);
         this.sslContext = getClientSslContext(spectatorRegistry);

zuul-core/src/test/java/com/netflix/zuul/netty/connectionpool/ConnectionPoolMetricsTest.java

@@ -54,6 +54,12 @@ public void validateMetricNames() {
         validateCounter("connectionpool_maxConnsPerHostExceeded", metrics.maxConnsPerHostExceededCounter());
         validateCounter("connectionpool_closeWrtBusyConnCounter", metrics.closeWrtBusyConnCounter());
         validateCounter("connectionpool_closeCircuitBreaker", metrics.circuitBreakerClose());
+
+        validateCounter("connectionpool_idle", metrics.idleCounter());
+        validateCounter("connectionpool_inactive", metrics.inactiveCounter());
+        validateCounter("connectionpool_error", metrics.errorCounter());
+        validateCounter("connectionpool_headerClose", metrics.headerCloseCounter());
+        validateCounter("connectionpool_sslClose", metrics.sslCloseCompletionCounter());
     }
 
     private void validateCounter(String name, Counter counter) {