#8069 handled review comments again

Signed-off-by: Ludovic Orban <[email protected]>
jetty · Jan 18, 2023 · be93d13 · be93d13
1 parent 0c664a6
commit be93d13
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 11 deletions.
diff --git a/...erver/src/main/java/org/eclipse/jetty/server/handler/AbstractLatencyRecordingHandler.java b/...erver/src/main/java/org/eclipse/jetty/server/handler/AbstractLatencyRecordingHandler.java
@@ -13,8 +13,6 @@
 
 package org.eclipse.jetty.server.handler;
 
-import java.util.function.Function;
-
 import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.HttpStream;
 import org.eclipse.jetty.server.Request;
@@ -34,31 +32,37 @@ public abstract class AbstractLatencyRecordingHandler extends Handler.Wrapper
 {
     private static final Logger LOG = LoggerFactory.getLogger(AbstractLatencyRecordingHandler.class);
 
-    private final Function<HttpStream, HttpStream> recordingWrapper;
-
     public AbstractLatencyRecordingHandler()
     {
-        this.recordingWrapper = httpStream -> new HttpStream.Wrapper(httpStream)
+    }
+
+    private HttpStream recordingWrapper(HttpStream httpStream)
+    {
+        return new HttpStream.Wrapper(httpStream)
         {
             @Override
             public void succeeded()
             {
+                // Take the httpStream nano timestamp before calling super.
+                long begin = httpStream.getNanoTime();
                 super.succeeded();
-                fireOnRequestComplete();
+                fireOnRequestComplete(begin);
             }
 
             @Override
             public void failed(Throwable x)
             {
+                // Take the httpStream nano timestamp before calling super.
+                long begin = httpStream.getNanoTime();
                 super.failed(x);
-                fireOnRequestComplete();
+                fireOnRequestComplete(begin);
             }
 
-            private void fireOnRequestComplete()
+            private void fireOnRequestComplete(long begin)
             {
                 try
                 {
-                    onRequestComplete(NanoTime.since(httpStream.getNanoTime()));
+                    onRequestComplete(NanoTime.since(begin));
                 }
                 catch (Throwable t)
                 {
@@ -72,7 +76,7 @@ private void fireOnRequestComplete()
     @Override
     public boolean process(Request request, Response response, Callback callback) throws Exception
     {
-        request.addHttpStreamWrapper(recordingWrapper);
+        request.addHttpStreamWrapper(this::recordingWrapper);
         return super.process(request, response, callback);
     }
 

diff --git a/...core/jetty-server/src/test/java/org/eclipse/jetty/server/LatencyRecordingHandlerTest.java b/...core/jetty-server/src/test/java/org/eclipse/jetty/server/LatencyRecordingHandlerTest.java
@@ -30,6 +30,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.is;
 
 public class LatencyRecordingHandlerTest
@@ -59,7 +60,8 @@ public boolean process(Request request, Response response, Callback callback)
                 return true;
             }
         };
-        AbstractLatencyRecordingHandler latencyRecordingHandler = new AbstractLatencyRecordingHandler() {
+        AbstractLatencyRecordingHandler latencyRecordingHandler = new AbstractLatencyRecordingHandler()
+        {
             @Override
             protected void onRequestComplete(long durationInNs)
             {
@@ -99,6 +101,10 @@ public void testLatenciesRecodingUponSuccess() throws Exception
             assertThat(response, containsString(" 200 OK"));
         }
         Awaitility.await().atMost(5, TimeUnit.SECONDS).until(_latencies::size, is(100));
+        for (Long latency : _latencies)
+        {
+            assertThat(latency, greaterThan(0L));
+        }
     }
 
     @Test
@@ -110,5 +116,9 @@ public void testLatenciesRecodingUponFailure() throws Exception
             assertThat(response, containsString(" 500 Server Error"));
         }
         Awaitility.await().atMost(5, TimeUnit.SECONDS).until(_latencies::size, is(100));
+        for (Long latency : _latencies)
+        {
+            assertThat(latency, greaterThan(0L));
+        }
     }
 }