Netflix · sullis · Nov 21, 2022 · Nov 17, 2022 · Nov 17, 2022
diff --git a/build.gradle b/build.gradle
@@ -87,12 +87,14 @@ subprojects {
 
 dependencies {
     rewrite(platform("org.openrewrite.recipe:rewrite-recipe-bom:1.12.1"))
+    rewrite("org.openrewrite.recipe:rewrite-logging-frameworks")
     rewrite("org.openrewrite.recipe:rewrite-testing-frameworks")
 }
 
 rewrite {
     failOnDryRunResults = true
     activeRecipe("org.openrewrite.java.testing.junit5.JUnit5BestPractices")
+    activeRecipe("org.openrewrite.java.logging.slf4j.Slf4jBestPractices")
     exclusion(
             // Exclude all groovy files
             "**/*.groovy")

diff --git a/zuul-core/src/main/java/com/netflix/config/PatternListStringProperty.java b/zuul-core/src/main/java/com/netflix/config/PatternListStringProperty.java
@@ -47,7 +47,7 @@ protected List<Pattern> derive(String value)
                     ptns.add(Pattern.compile(ptnTxt.trim()));
                 }
                 catch (Exception e) {
-                    LOG.error("Error parsing regex pattern list from property! name = " + String.valueOf(this.getName()) + ", value = " + String.valueOf(this.getValue()) + ", pattern = " + String.valueOf(value));
+                    LOG.error("Error parsing regex pattern list from property! name = {}, value = {}, pattern = {}", String.valueOf(this.getName()), String.valueOf(this.getValue()), String.valueOf(value));
                 }
             }
         }

diff --git a/zuul-core/src/main/java/com/netflix/netty/common/Http1ConnectionCloseHandler.java b/zuul-core/src/main/java/com/netflix/netty/common/Http1ConnectionCloseHandler.java
@@ -115,15 +115,15 @@ protected void gracefully(ChannelHandlerContext ctx, ChannelPromise promise)
                     // either after the response finally gets written (due to us having set the CLOSE_AFTER_RESPONSE flag), or when the IdleTimeout
                     // for this conn fires.
                     if (requestInflight.get()) {
-                        LOG.debug("gracefully: firing graceful_shutdown event to close connection, but request still inflight, so leaving. channel=" + channelId);
+                        LOG.debug("gracefully: firing graceful_shutdown event to close connection, but request still inflight, so leaving. channel={}", channelId);
                     }
                     else {
-                        LOG.debug("gracefully: firing graceful_shutdown event to close connection. channel=" + channelId);
+                        LOG.debug("gracefully: firing graceful_shutdown event to close connection. channel={}", channelId);
                         ctx.close(promise);
                     }
                 }
                 else {
-                    LOG.debug("gracefully: connection already closed. channel=" + channelId);
+                    LOG.debug("gracefully: connection already closed. channel={}", channelId);
                     promise.setSuccess();
                 }
 

diff --git a/zuul-core/src/main/java/com/netflix/netty/common/Http2ConnectionCloseHandler.java b/zuul-core/src/main/java/com/netflix/netty/common/Http2ConnectionCloseHandler.java
@@ -190,7 +190,7 @@ private void gracefullyWithDelay(EventExecutor executor, Channel parent, Channel
         DefaultHttp2GoAwayFrame goaway = new DefaultHttp2GoAwayFrame(Http2Error.NO_ERROR);
         goaway.setExtraStreamIds(Integer.MAX_VALUE);
         parent.writeAndFlush(goaway);
-        LOG.debug("gracefullyWithDelay: flushed initial go_away frame. channel=" + parent.id().asShortText());
+        LOG.debug("gracefullyWithDelay: flushed initial go_away frame. channel={}", parent.id().asShortText());
 
         // In N secs time, throw an error that causes the http2 codec to send another GOAWAY frame
         // (this time with accurate lastStreamId) and then close the connection.
@@ -202,8 +202,7 @@ private void gracefullyWithDelay(EventExecutor executor, Channel parent, Channel
                 // NOTE - the netty Http2ConnectionHandler specifically does not send another goaway when we call
                 // channel.close() if one has already been sent .... so when we want more than one sent, we need to do it
                 // explicitly ourselves like this.
-                LOG.debug("gracefullyWithDelay: firing graceful_shutdown event to make netty send a final go_away frame and then close connection. channel="
-                        + parent.id().asShortText());
+                LOG.debug("gracefullyWithDelay: firing graceful_shutdown event to make netty send a final go_away frame and then close connection. channel={}", parent.id().asShortText());
                 Http2Exception h2e = new Http2Exception(Http2Error.NO_ERROR, Http2Exception.ShutdownHint.GRACEFUL_SHUTDOWN);
                 parent.pipeline().fireExceptionCaught(h2e);
 

diff --git a/zuul-core/src/main/java/com/netflix/netty/common/HttpLifecycleChannelHandler.java b/zuul-core/src/main/java/com/netflix/netty/common/HttpLifecycleChannelHandler.java
@@ -57,8 +57,7 @@ protected static boolean fireStartEvent(ChannelHandlerContext ctx, HttpRequest r
         if (state == State.STARTED) {
             // This could potentially happen if a bad client sends a 2nd request on the same connection
             // without waiting for the response from the first. And we don't support HTTP Pipelining.
-            LOG.error("Received a http request on connection where we already have a request being processed. " +
-                    "Closing the connection now. channel = " + channel.id().asLongText());
+            LOG.error("Received a http request on connection where we already have a request being processed. Closing the connection now. channel = {}", channel.id().asLongText());
             channel.attr(ATTR_HTTP_PIPELINE_REJECT).set(Boolean.TRUE);
             channel.close();
             return false;

diff --git a/zuul-core/src/main/java/com/netflix/netty/common/LeastConnsEventLoopChooserFactory.java b/zuul-core/src/main/java/com/netflix/netty/common/LeastConnsEventLoopChooserFactory.java
@@ -92,9 +92,7 @@ private EventExecutor chooseWithLeastConns()
             }
 
             if (LOG.isDebugEnabled()) {
-                LOG.debug("Chose eventloop: " + String.valueOf(leastExec)
-                        + ", leastValue=" + leastValue
-                        + ", connsPer=" + String.valueOf(connsPer));
+                LOG.debug("Chose eventloop: {}, leastValue={}, connsPer={}", String.valueOf(leastExec), leastValue, String.valueOf(connsPer));
             }
 
             return leastExec;

diff --git a/zuul-core/src/main/java/com/netflix/netty/common/throttle/MaxInboundConnectionsHandler.java b/zuul-core/src/main/java/com/netflix/netty/common/throttle/MaxInboundConnectionsHandler.java
@@ -62,7 +62,7 @@ public void channelActive(ChannelHandlerContext ctx) throws Exception
             int currentCount = connections.getAndIncrement();
 
             if (currentCount + 1 > maxConnections) {
-                LOG.warn("Throttling incoming connection as above configured max connections threshold of " + maxConnections);
+                LOG.warn("Throttling incoming connection as above configured max connections threshold of {}", maxConnections);
                 Channel channel = ctx.channel();
                 channel.attr(ATTR_CH_THROTTLED).set(Boolean.TRUE);
                 CurrentPassport.fromChannel(channel).add(PassportState.SERVER_CH_THROTTLING);

diff --git a/zuul-core/src/main/java/com/netflix/zuul/DynamicFilterLoader.java b/zuul-core/src/main/java/com/netflix/zuul/DynamicFilterLoader.java
@@ -35,7 +35,7 @@
 
 @Singleton
 public final class DynamicFilterLoader implements FilterLoader {
-    private static final Logger LOG = LoggerFactory.getLogger(FilterLoader.class);
+    private static final Logger LOG = LoggerFactory.getLogger(DynamicFilterLoader.class);
 
     private final ConcurrentMap<String, Long> filterClassLastModified = new ConcurrentHashMap<>();
     private final ConcurrentMap<String, String> filterClassCode = new ConcurrentHashMap<>();
@@ -114,7 +114,7 @@ public boolean putFilter(File file) {
             String sName = file.getAbsolutePath();
             if (filterClassLastModified.get(sName) != null
                     && (file.lastModified() != filterClassLastModified.get(sName))) {
-                LOG.debug("reloading filter " + sName);
+                LOG.debug("reloading filter {}", sName);
                 filterRegistry.remove(sName);
             }
             ZuulFilter<?, ?> filter = filterRegistry.get(sName);
@@ -127,7 +127,7 @@ public boolean putFilter(File file) {
                 }
             }
         } catch (Exception e) {
-            LOG.error("Error loading filter! Continuing. file=" + file, e);
+            LOG.error("Error loading filter! Continuing. file={}", file, e);
             return false;
         }
 

diff --git a/zuul-core/src/main/java/com/netflix/zuul/FilterFileManager.java b/zuul-core/src/main/java/com/netflix/zuul/FilterFileManager.java
@@ -80,7 +80,7 @@ public void init() throws Exception
         manageFiles();
         startPoller();
 
-        LOG.warn("Finished loading all zuul filters. Duration = " + (System.currentTimeMillis() - startTime) + " ms.");
+        LOG.warn("Finished loading all zuul filters. Duration = {} ms.", (System.currentTimeMillis() - startTime));
     }
 
     /**
@@ -128,7 +128,7 @@ public File getDirectory(String sPath) {
             try {
                 directory = new File(resource.toURI());
             } catch (Exception e) {
-                LOG.error("Error accessing directory in classloader. path=" + sPath, e);
+                LOG.error("Error accessing directory in classloader. path={}", sPath, e);
             }
             if (!directory.isDirectory()) {
                 throw new RuntimeException(directory.getAbsolutePath() + " is not a valid directory");
@@ -173,7 +173,7 @@ void processGroovyFiles(List<File> aFiles) throws Exception {
                     return filterLoader.putFilter(file);
                 }
                 catch(Exception e) {
-                    LOG.error("Error loading groovy filter from disk! file = " + String.valueOf(file), e);
+                    LOG.error("Error loading groovy filter from disk! file = {}", String.valueOf(file), e);
                     return false;
                 }
             });

diff --git a/zuul-core/src/main/java/com/netflix/zuul/filters/endpoint/ProxyEndpoint.java b/zuul-core/src/main/java/com/netflix/zuul/filters/endpoint/ProxyEndpoint.java
@@ -444,7 +444,7 @@ private void proxyRequestToOrigin() {
             }
         }
         catch (Exception ex) {
-            LOG.error("Error while connecting to origin, UUID {} " + context.getUUID(), ex);
+            LOG.error("Error while connecting to origin, UUID {} {}", context.getUUID(), ex);
             storeAndLogOriginRequestInfo();
             if (promise != null && ! promise.isDone()) {
                 promise.setFailure(ex);
@@ -608,15 +608,15 @@ private void processErrorFromOrigin(final Throwable ex, final Channel origCh) {
             // Be cautious about how much we log about errors from origins, as it can have perf implications at high rps.
             if (zuulCtx.isInBrownoutMode()) {
                 // Don't include the stacktrace or the channel info.
-                LOG.warn(err.getStatusCategory().name() + ", origin = " + origin.getName() + ": " + String.valueOf(ex));
+                LOG.warn("{}, origin = {}: {}", err.getStatusCategory().name(), origin.getName(), String.valueOf(ex));
             } else {
                 final String origChInfo = (origCh != null) ? ChannelUtils.channelInfoForLogging(origCh) : "";
                 if (LOG.isInfoEnabled()) {
                     // Include the stacktrace.
-                    LOG.warn(err.getStatusCategory().name() + ", origin = " + origin.getName() + ", origin channel info = " + origChInfo, ex);
+                    LOG.warn("{}, origin = {}, origin channel info = {}", err.getStatusCategory().name(), origin.getName(), origChInfo, ex);
                 }
                 else {
-                    LOG.warn(err.getStatusCategory().name() + ", origin = " + origin.getName() + ", " + String.valueOf(ex) + ", origin channel info = " + origChInfo);
+                    LOG.warn("{}, origin = {}, {}, origin channel info = {}", err.getStatusCategory().name(), origin.getName(), String.valueOf(ex), origChInfo);
                 }
             }
 
@@ -952,7 +952,7 @@ private static HttpRequestMessage massageRequestURI(HttpRequestMessage request)
                         }
                     }
                 } catch (UnsupportedEncodingException e) {
-                    LOG.error("Error decoding url query param - " + paramString, e);
+                    LOG.error("Error decoding url query param - {}", paramString, e);
                 }
             } else {
                 modifiedPath = uri;

diff --git a/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpRequestMessageImpl.java b/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpRequestMessageImpl.java
@@ -387,8 +387,8 @@ public Cookies reParseCookies()
                 }
             }
             catch (Exception e) {
-                LOG.error(String.format("Error parsing request Cookie header. cookie=%s, request-info=%s",
-                        aCookieHeader, getInfoForLogging()));
+                LOG.error("Error parsing request Cookie header. cookie={}, request-info={}",
+                        aCookieHeader, getInfoForLogging());
             }
 
         }

diff --git a/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpResponseMessageImpl.java b/zuul-core/src/main/java/com/netflix/zuul/message/http/HttpResponseMessageImpl.java
@@ -56,9 +56,7 @@ public HttpResponseMessageImpl(SessionContext context, Headers headers, HttpRequ
         this.message = new ZuulMessageImpl(context, headers);
         this.outboundRequest = request;
         if (this.outboundRequest.getInboundRequest() == null) {
-            LOG.warn("HttpResponseMessage created with a request that does not have a stored inboundRequest! " +
-                    "Probably a bug in the filter that is creating this response.",
-                    new RuntimeException("Invalid HttpRequestMessage"));
+            LOG.warn("HttpResponseMessage created with a request that does not have a stored inboundRequest! Probably a bug in the filter that is creating this response.", new RuntimeException("Invalid HttpRequestMessage"));
         }
         this.status = status;
     }

diff --git a/zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/ConnectionPoolHandler.java b/zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/ConnectionPoolHandler.java
@@ -136,7 +136,7 @@ private void closeConnection(ChannelHandlerContext ctx, String msg) {
         else {
             // If somehow we don't have a PooledConnection instance attached to this channel, then
             // close the channel directly.
-            LOG.warn(msg + " But no PooledConnection attribute. So just closing Channel.");
+            LOG.warn("{} But no PooledConnection attribute. So just closing Channel.", msg);
             ctx.close();
         }
     }

diff --git a/...core/src/main/java/com/netflix/zuul/netty/connectionpool/DefaultClientChannelManager.java b/...core/src/main/java/com/netflix/zuul/netty/connectionpool/DefaultClientChannelManager.java
@@ -275,7 +275,7 @@ else if (!conn.isActive()) {
                 conn.close();
             }
 
-            if (LOG.isDebugEnabled()) LOG.debug("PooledConnection released: " + conn.toString());
+            if (LOG.isDebugEnabled()) LOG.debug("PooledConnection released: {}", conn.toString());
         }
 
         return released;

diff --git a/zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/PerServerConnectionPool.java b/zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/PerServerConnectionPool.java
@@ -140,7 +140,7 @@ protected void onAcquire(final PooledConnection conn, CurrentPassport passport)
         removeIdleStateHandler(conn);
 
         conn.setInUse();
-        if (LOG.isDebugEnabled()) LOG.debug("PooledConnection acquired: " + conn.toString());
+        if (LOG.isDebugEnabled()) LOG.debug("PooledConnection acquired: {}", conn.toString());
     }
 
     protected void removeIdleStateHandler(PooledConnection conn) {
@@ -231,12 +231,7 @@ protected void tryMakingNewConnection(
             promise.setFailure(new OriginConnectException(
                 "maxConnectionsPerHost=" + maxConnectionsPerHost + ", connectionsPerHost=" + openAndOpeningConnectionCount,
                     OutboundErrorType.ORIGIN_SERVER_MAX_CONNS));
-            LOG.warn("Unable to create new connection because at MaxConnectionsPerHost! "
-                            + "maxConnectionsPerHost=" + maxConnectionsPerHost
-                            + ", connectionsPerHost=" + openAndOpeningConnectionCount
-                            + ", host=" + server.getServerId()
-                            + "origin=" + config.getOriginName()
-                    );
+            LOG.warn("Unable to create new connection because at MaxConnectionsPerHost! maxConnectionsPerHost={}, connectionsPerHost={}, host={}origin={}", maxConnectionsPerHost, openAndOpeningConnectionCount, server.getServerId(), config.getOriginName());
             return;
         }
 
@@ -261,10 +256,7 @@ protected void tryMakingNewConnection(
                         if (! promise.isDone()) {
                             promise.setFailure(e);
                         }
-                        LOG.warn("Error creating new connection! "
-                                        + "origin=" + config.getOriginName()
-                                        + ", host=" + server.getServerId()
-                                );
+                        LOG.warn("Error creating new connection! origin={}, host={}", config.getOriginName(), server.getServerId());
                     }
                 });
             }

diff --git a/zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/PooledConnection.java b/zuul-core/src/main/java/com/netflix/zuul/netty/connectionpool/PooledConnection.java
@@ -199,8 +199,7 @@ public boolean release()
 
         if (! isShouldClose() && connectionState != ConnectionState.WRITE_READY) {
             CurrentPassport passport = CurrentPassport.fromChannel(channel);
-            LOG.info("Error - Attempt to put busy connection into the pool = " + this.toString()
-                    + ", " + String.valueOf(passport));
+            LOG.info("Error - Attempt to put busy connection into the pool = {}, {}", this.toString(), String.valueOf(passport));
             this.shouldClose = true;
         }
 

diff --git a/zuul-core/src/main/java/com/netflix/zuul/netty/filter/ZuulFilterChainHandler.java b/zuul-core/src/main/java/com/netflix/zuul/netty/filter/ZuulFilterChainHandler.java
@@ -81,7 +81,7 @@ else if ((msg instanceof HttpContent)&&(zuulRequest != null)) {
             requestFilterChain.filter(zuulRequest, (HttpContent) msg);
         }
         else {
-            LOG.debug("Received unrecognized message type. " + msg.getClass().getName());
+            LOG.debug("Received unrecognized message type. {}", msg.getClass().getName());
             ReferenceCountUtil.release(msg);
         }
     }
@@ -158,7 +158,7 @@ private void finishResponseFilters(ChannelHandlerContext ctx) {
 
     @Override
     public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception {
-        LOG.error("zuul filter chain handler caught exception. cause=" + String.valueOf(cause), cause);
+        LOG.error("zuul filter chain handler caught exception. cause={}", String.valueOf(cause), cause);
         if (zuulRequest != null && !isClientChannelClosed(cause)) {
             final SessionContext zuulCtx = zuulRequest.getContext();
             zuulCtx.setError(cause);