Merge pull request #6176 from eclipse/jetty-10.0.x-5817-CustomRequestLogFiltering

Issue #5817 - allow CustomRequestLog to be filtered with BiPredicate

jetty-server/src/main/java/org/eclipse/jetty/server/CustomRequestLog.java

@@ -22,6 +22,7 @@ import java.util.List;
 import java.util.Locale;
 import java.util.TimeZone;
 import java.util.concurrent.TimeUnit;
+import java.util.function.BiPredicate;
 import java.util.function.Supplier;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -279,6 +280,7 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
     private final String _formatString;
     private transient PathMappings<String> _ignorePathMap;
     private String[] _ignorePaths;
+    private BiPredicate<Request, Response> _filter;
 
     public CustomRequestLog()
     {
@@ -311,6 +313,16 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
         }
     }
 
+    /**
+     * This allows you to set a custom filter to decide whether to log a request or omit it from the request log.
+     * This filter is evaluated after path filtering is applied from {@link #setIgnorePaths(String[])}.
+     * @param filter - a BiPredicate which returns true if this request should be logged.
+     */
+    public void setFilter(BiPredicate<Request, Response> filter)
+    {
+        _filter = filter;
+    }
+
     @ManagedAttribute("The RequestLogWriter")
     public RequestLog.Writer getWriter()
     {
@@ -325,11 +337,14 @@ public class CustomRequestLog extends ContainerLifeCycle implements RequestLog
     @Override
     public void log(Request request, Response response)
     {
+        if (_ignorePathMap != null && _ignorePathMap.getMatch(request.getRequestURI()) != null)
+            return;
+
+        if (_filter != null && !_filter.test(request, response))
+            return;
+
         try
         {
-            if (_ignorePathMap != null && _ignorePathMap.getMatch(request.getRequestURI()) != null)
-                return;
-
             StringBuilder sb = _buffers.get();
             sb.setLength(0);
 

tests/test-integration/src/test/java/org/eclipse/jetty/test/CustomRequestLogTest.java

@@ -14,7 +14,6 @@
 package org.eclipse.jetty.test;
 
 import java.io.IOException;
-import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.InetAddress;
 import java.net.NetworkInterface;
@@ -27,7 +26,7 @@ import java.util.Locale;
 import java.util.Objects;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.TimeUnit;
-import javax.servlet.ServletException;
+import java.util.concurrent.atomic.AtomicReference;
 import javax.servlet.ServletOutputStream;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
@@ -53,6 +52,7 @@ import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
 import org.eclipse.jetty.util.BlockingArrayQueue;
 import org.eclipse.jetty.util.DateCache;
+import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.security.Constraint;
 import org.eclipse.jetty.util.security.Credential;
 import org.junit.jupiter.api.AfterEach;
@@ -66,22 +66,24 @@ import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.lessThanOrEqualTo;
 import static org.hamcrest.Matchers.not;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.fail;
 
 public class CustomRequestLogTest
 {
-    CustomRequestLog _log;
+    private final BlockingQueue<String> _entries = new BlockingArrayQueue<>();
-    Server _server;
+    private final BlockingQueue<Long> requestTimes = new BlockingArrayQueue<>();
-    LocalConnector _connector;
+    private CustomRequestLog _log;
-    BlockingQueue<String> _entries = new BlockingArrayQueue<>();
+    private Server _server;
-    BlockingQueue<Long> requestTimes = new BlockingArrayQueue<>();
+    private LocalConnector _connector;
-    ServerConnector _serverConnector;
+    private ServerConnector _serverConnector;
-    URI _serverURI;
+    private URI _serverURI;
 
     private static final long DELAY = 2000;
 
     @BeforeEach
-    public void before() throws Exception
+    public void before()
     {
         _server = new Server();
         _connector = new LocalConnector(_server);
@@ -111,6 +113,7 @@ public class CustomRequestLogTest
         _serverURI = new URI(String.format("http://%s:%d/", host, localPort));
     }
 
+    @SuppressWarnings("SameParameterValue")
     private static SecurityHandler getSecurityHandler(String username, String password, String realm)
     {
         HashLoginService loginService = new HashLoginService();
@@ -142,6 +145,22 @@ public class CustomRequestLogTest
         _server.stop();
     }
 
+    @Test
+    public void testRequestFilter() throws Exception
+    {
+        AtomicReference<Boolean> logRequest = new AtomicReference<>();
+        testHandlerServerStart("RequestPath: %U");
+        _log.setFilter((request, response) -> logRequest.get());
+
+        logRequest.set(true);
+        _connector.getResponse("GET /path HTTP/1.0\n\n");
+        assertThat(_entries.poll(5, TimeUnit.SECONDS), is("RequestPath: /path"));
+
+        logRequest.set(false);
+        _connector.getResponse("GET /path HTTP/1.0\n\n");
+        assertNull(_entries.poll(1, TimeUnit.SECONDS));
+    }
+
     @Test
     public void testLogRemoteUser() throws Exception
     {
@@ -197,16 +216,16 @@ public class CustomRequestLogTest
             "%{server}a|%{server}p|" +
             "%{client}a|%{client}p");
 
-        Enumeration e = NetworkInterface.getNetworkInterfaces();
+        Enumeration<NetworkInterface> e = NetworkInterface.getNetworkInterfaces();
         while (e.hasMoreElements())
         {
-            NetworkInterface n = (NetworkInterface)e.nextElement();
+            NetworkInterface n = e.nextElement();
             if (n.isLoopback())
             {
-                Enumeration ee = n.getInetAddresses();
+                Enumeration<InetAddress> ee = n.getInetAddresses();
                 while (ee.hasMoreElements())
                 {
-                    InetAddress i = (InetAddress)ee.nextElement();
+                    InetAddress i = ee.nextElement();
                     try (Socket client = newSocket(i.getHostAddress(), _serverURI.getPort()))
                     {
                         OutputStream os = client.getOutputStream();
@@ -217,7 +236,7 @@ public class CustomRequestLogTest
                         os.write(request.getBytes(StandardCharsets.ISO_8859_1));
                         os.flush();
 
-                        String[] log = _entries.poll(5, TimeUnit.SECONDS).split("\\|");
+                        String[] log = Objects.requireNonNull(_entries.poll(5, TimeUnit.SECONDS)).split("\\|");
                         assertThat(log.length, is(8));
 
                         String localAddr = log[0];
@@ -428,7 +447,7 @@ public class CustomRequestLogTest
 
         _connector.getResponse("GET / HTTP/1.0\n\n");
         String log = _entries.poll(5, TimeUnit.SECONDS);
-        long requestTime = requestTimes.poll(5, TimeUnit.SECONDS);
+        long requestTime = getTimeRequestReceived();
         DateCache dateCache = new DateCache(CustomRequestLog.DEFAULT_DATE_FORMAT, Locale.getDefault(), "GMT");
         assertThat(log, is("RequestTime: [" + dateCache.format(requestTime) + "]"));
     }
@@ -442,7 +461,8 @@ public class CustomRequestLogTest
 
         _connector.getResponse("GET / HTTP/1.0\n\n");
         String log = _entries.poll(5, TimeUnit.SECONDS);
-        long requestTime = requestTimes.poll(5, TimeUnit.SECONDS);
+        assertNotNull(log);
+        long requestTime = getTimeRequestReceived();
 
         DateCache dateCache1 = new DateCache("EEE MMM dd HH:mm:ss zzz yyyy", Locale.getDefault(), "GMT");
         DateCache dateCache2 = new DateCache("EEE MMM dd HH:mm:ss zzz yyyy", Locale.getDefault(), "EST");
@@ -461,7 +481,8 @@ public class CustomRequestLogTest
 
         _connector.getResponse("GET /delay HTTP/1.0\n\n");
         String log = _entries.poll(5, TimeUnit.SECONDS);
-        long lowerBound = requestTimes.poll(5, TimeUnit.SECONDS);
+        assertNotNull(log);
+        long lowerBound = getTimeRequestReceived();
         long upperBound = System.currentTimeMillis();
 
         long measuredDuration = Long.parseLong(log);
@@ -479,7 +500,8 @@ public class CustomRequestLogTest
 
         _connector.getResponse("GET /delay HTTP/1.0\n\n");
         String log = _entries.poll(5, TimeUnit.SECONDS);
-        long lowerBound = requestTimes.poll(5, TimeUnit.SECONDS);
+        assertNotNull(log);
+        long lowerBound = getTimeRequestReceived();
         long upperBound = System.currentTimeMillis();
 
         long measuredDuration = Long.parseLong(log);
@@ -497,7 +519,8 @@ public class CustomRequestLogTest
 
         _connector.getResponse("GET /delay HTTP/1.0\n\n");
         String log = _entries.poll(5, TimeUnit.SECONDS);
-        long lowerBound = requestTimes.poll(5, TimeUnit.SECONDS);
+        assertNotNull(log);
+        long lowerBound = getTimeRequestReceived();
         long upperBound = System.currentTimeMillis();
 
         long measuredDuration = Long.parseLong(log);
@@ -575,11 +598,6 @@ public class CustomRequestLogTest
         fail(log);
     }
 
-    protected Socket newSocket() throws Exception
-    {
-        return newSocket(_serverURI.getHost(), _serverURI.getPort());
-    }
-
     protected Socket newSocket(String host, int port) throws Exception
     {
         Socket socket = new Socket(host, port);
@@ -604,10 +622,17 @@ public class CustomRequestLogTest
         }
     }
 
+    private long getTimeRequestReceived() throws InterruptedException
+    {
+        Long requestTime = requestTimes.poll(5, TimeUnit.SECONDS);
+        assertNotNull(requestTime);
+        return requestTime;
+    }
+
     private class TestServlet extends HttpServlet
     {
         @Override
-        protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException
+        protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException
         {
             Request baseRequest = Objects.requireNonNull(Request.getBaseRequest(request));
 
@@ -652,8 +677,7 @@ public class CustomRequestLogTest
 
             if (request.getContentLength() > 0)
             {
-                InputStream in = request.getInputStream();
+                IO.readBytes(request.getInputStream());
-                while (in.read() > 0);
             }
         }
     }