From 451ead83fde12c35e39b5ec59803737a4cc51a58 Mon Sep 17 00:00:00 2001 From: Ludovic Orban Date: Wed, 18 Jan 2023 12:56:02 +0100 Subject: [PATCH] #8069 handled review comments again Signed-off-by: Ludovic Orban --- .../AbstractLatencyRecordingHandler.java | 24 +++++++++++-------- .../server/LatencyRecordingHandlerTest.java | 9 +++++++ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AbstractLatencyRecordingHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AbstractLatencyRecordingHandler.java index fecc01d19275..6f177859a844 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/AbstractLatencyRecordingHandler.java +++ b/jetty-core/jetty-server/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 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/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/LatencyRecordingHandlerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/LatencyRecordingHandlerTest.java index ec0e53bd2077..f7364c72b488 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/LatencyRecordingHandlerTest.java +++ b/jetty-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 @@ -99,6 +100,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 +115,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)); + } } }