Don't overwrite start time in InstrumentedHandler with concurrent asy…

…nc requests (#2429) Conflicts: metrics-jetty11/src/main/java/io/dropwizard/metrics5/jetty11/InstrumentedHandler.java Closes #2121 Refs #2413 (cherry picked from commit 533cac7) Co-authored-by: Jacob Schlather <[email protected]>
dropwizard · Feb 3, 2022 · 2e9b0f0 · 2e9b0f0
1 parent b6adbdb
commit 2e9b0f0
Show file tree

Hide file tree

Showing 6 changed files with 300 additions and 179 deletions.
diff --git a/metrics-jetty10/src/main/java/io/dropwizard/metrics5/jetty10/InstrumentedHandler.java b/metrics-jetty10/src/main/java/io/dropwizard/metrics5/jetty10/InstrumentedHandler.java
@@ -210,36 +210,7 @@ public Ratio getRatio() {
         });
 
 
-        this.listener = new AsyncListener() {
-            private long startTime;
-
-            @Override
-            public void onTimeout(AsyncEvent event) throws IOException {
-                asyncTimeouts.mark();
-            }
-
-            @Override
-            public void onStartAsync(AsyncEvent event) throws IOException {
-                startTime = System.currentTimeMillis();
-                event.getAsyncContext().addListener(this);
-            }
-
-            @Override
-            public void onError(AsyncEvent event) throws IOException {
-            }
-
-            @Override
-            public void onComplete(AsyncEvent event) throws IOException {
-                final AsyncContextState state = (AsyncContextState) event.getAsyncContext();
-                final HttpServletRequest request = (HttpServletRequest) state.getRequest();
-                final HttpServletResponse response = (HttpServletResponse) state.getResponse();
-                updateResponses(request, response, startTime, true);
-                if (state.getHttpChannelState().getState() != HttpChannelState.State.HANDLING &&
-                        state.getHttpChannelState().isSuspended()) {
-                    activeSuspended.dec();
-                }
-            }
-        };
+        this.listener = new AsyncAttachingListener();
     }
 
     @Override
@@ -369,4 +340,53 @@ private void updateResponses(HttpServletRequest request, HttpServletResponse res
     private MetricName getMetricPrefix() {
         return this.prefix == null ? name(getHandler().getClass(), name) : name(this.prefix, name);
     }
+
+    private class AsyncAttachingListener implements AsyncListener {
+
+        @Override
+        public void onTimeout(AsyncEvent event) throws IOException {}
+
+        @Override
+        public void onStartAsync(AsyncEvent event) throws IOException {
+            event.getAsyncContext().addListener(new InstrumentedAsyncListener());
+        }
+
+        @Override
+        public void onError(AsyncEvent event) throws IOException {}
+
+        @Override
+        public void onComplete(AsyncEvent event) throws IOException {}
+    };
+
+    private class InstrumentedAsyncListener implements AsyncListener {
+        private final long startTime;
+
+        InstrumentedAsyncListener() {
+            this.startTime = System.currentTimeMillis();
+        }
+
+        @Override
+        public void onTimeout(AsyncEvent event) throws IOException {
+            asyncTimeouts.mark();
+        }
+
+        @Override
+        public void onStartAsync(AsyncEvent event) throws IOException {
+        }
+
+        @Override
+        public void onError(AsyncEvent event) throws IOException {
+        }
+
+        @Override
+        public void onComplete(AsyncEvent event) throws IOException {
+            final AsyncContextState state = (AsyncContextState) event.getAsyncContext();
+            final HttpServletRequest request = (HttpServletRequest) state.getRequest();
+            final HttpServletResponse response = (HttpServletResponse) state.getResponse();
+            updateResponses(request, response, startTime, true);
+            if (!state.getHttpChannelState().isSuspended()) {
+                activeSuspended.dec();
+            }
+        }
+    }
 }
diff --git a/...jetty10/src/main/java/io/dropwizard/metrics5/jetty10/InstrumentedHttpChannelListener.java b/...jetty10/src/main/java/io/dropwizard/metrics5/jetty10/InstrumentedHttpChannelListener.java
@@ -166,35 +166,7 @@ public RatioGauge.Ratio getRatio() {
             }
         });
 
-        this.listener = new AsyncListener() {
-            private long startTime;
-
-            @Override
-            public void onTimeout(AsyncEvent event) throws IOException {
-                asyncTimeouts.mark();
-            }
-
-            @Override
-            public void onStartAsync(AsyncEvent event) throws IOException {
-                startTime = System.currentTimeMillis();
-                event.getAsyncContext().addListener(this);
-            }
-
-            @Override
-            public void onError(AsyncEvent event) throws IOException {
-            }
-
-            @Override
-            public void onComplete(AsyncEvent event) throws IOException {
-                final AsyncContextState state = (AsyncContextState) event.getAsyncContext();
-                final HttpServletRequest request = (HttpServletRequest) state.getRequest();
-                final HttpServletResponse response = (HttpServletResponse) state.getResponse();
-                updateResponses(request, response, startTime, true);
-                if (!state.getHttpChannelState().isSuspended()) {
-                    activeSuspended.dec();
-                }
-            }
-        };
+        this.listener = new AsyncAttachingListener();
     }
 
     @Override
@@ -355,4 +327,53 @@ private Timer requestTimer(String method) {
             }
         }
     }
+
+    private class AsyncAttachingListener implements AsyncListener {
+
+        @Override
+        public void onTimeout(AsyncEvent event) throws IOException {}
+
+        @Override
+        public void onStartAsync(AsyncEvent event) throws IOException {
+            event.getAsyncContext().addListener(new InstrumentedAsyncListener());
+        }
+
+        @Override
+        public void onError(AsyncEvent event) throws IOException {}
+
+        @Override
+        public void onComplete(AsyncEvent event) throws IOException {}
+    };
+
+    private class InstrumentedAsyncListener implements AsyncListener {
+        private final long startTime;
+
+        InstrumentedAsyncListener() {
+            this.startTime = System.currentTimeMillis();
+        }
+
+        @Override
+        public void onTimeout(AsyncEvent event) throws IOException {
+            asyncTimeouts.mark();
+        }
+
+        @Override
+        public void onStartAsync(AsyncEvent event) throws IOException {
+        }
+
+        @Override
+        public void onError(AsyncEvent event) throws IOException {
+        }
+
+        @Override
+        public void onComplete(AsyncEvent event) throws IOException {
+            final AsyncContextState state = (AsyncContextState) event.getAsyncContext();
+            final HttpServletRequest request = (HttpServletRequest) state.getRequest();
+            final HttpServletResponse response = (HttpServletResponse) state.getResponse();
+            updateResponses(request, response, startTime, true);
+            if (!state.getHttpChannelState().isSuspended()) {
+                activeSuspended.dec();
+            }
+        }
+    }
 }
diff --git a/metrics-jetty11/src/main/java/io/dropwizard/metrics5/jetty11/InstrumentedHandler.java b/metrics-jetty11/src/main/java/io/dropwizard/metrics5/jetty11/InstrumentedHandler.java
@@ -209,38 +209,7 @@ public Ratio getRatio() {
             }
         });
 
-
-
-        this.listener = new AsyncListener() {
-            private long startTime;
-
-            @Override
-            public void onTimeout(AsyncEvent event) throws IOException {
-                asyncTimeouts.mark();
-            }
-
-            @Override
-            public void onStartAsync(AsyncEvent event) throws IOException {
-                startTime = System.currentTimeMillis();
-                event.getAsyncContext().addListener(this);
-            }
-
-            @Override
-            public void onError(AsyncEvent event) throws IOException {
-            }
-
-            @Override
-            public void onComplete(AsyncEvent event) throws IOException {
-                final AsyncContextState state = (AsyncContextState) event.getAsyncContext();
-                final HttpServletRequest request = (HttpServletRequest) state.getRequest();
-                final HttpServletResponse response = (HttpServletResponse) state.getResponse();
-                updateResponses(request, response, startTime, true);
-                if (state.getHttpChannelState().getState() != HttpChannelState.State.HANDLING &&
-                        state.getHttpChannelState().isSuspended()) {
-                    activeSuspended.dec();
-                }
-            }
-        };
+        this.listener = new AsyncAttachingListener();
     }
 
     @Override
@@ -370,4 +339,53 @@ private void updateResponses(HttpServletRequest request, HttpServletResponse res
     private MetricName getMetricPrefix() {
         return this.prefix == null ? name(getHandler().getClass(), name) : name(this.prefix, name);
     }
+
+    private class AsyncAttachingListener implements AsyncListener {
+
+        @Override
+        public void onTimeout(AsyncEvent event) throws IOException {}
+
+        @Override
+        public void onStartAsync(AsyncEvent event) throws IOException {
+            event.getAsyncContext().addListener(new InstrumentedAsyncListener());
+        }
+
+        @Override
+        public void onError(AsyncEvent event) throws IOException {}
+
+        @Override
+        public void onComplete(AsyncEvent event) throws IOException {}
+    };
+
+    private class InstrumentedAsyncListener implements AsyncListener {
+        private final long startTime;
+
+        InstrumentedAsyncListener() {
+            this.startTime = System.currentTimeMillis();
+        }
+
+        @Override
+        public void onTimeout(AsyncEvent event) throws IOException {
+            asyncTimeouts.mark();
+        }
+
+        @Override
+        public void onStartAsync(AsyncEvent event) throws IOException {
+        }
+
+        @Override
+        public void onError(AsyncEvent event) throws IOException {
+        }
+
+        @Override
+        public void onComplete(AsyncEvent event) throws IOException {
+            final AsyncContextState state = (AsyncContextState) event.getAsyncContext();
+            final HttpServletRequest request = (HttpServletRequest) state.getRequest();
+            final HttpServletResponse response = (HttpServletResponse) state.getResponse();
+            updateResponses(request, response, startTime, true);
+            if (!state.getHttpChannelState().isSuspended()) {
+                activeSuspended.dec();
+            }
+        }
+    }
 }
diff --git a/...jetty11/src/main/java/io/dropwizard/metrics5/jetty11/InstrumentedHttpChannelListener.java b/...jetty11/src/main/java/io/dropwizard/metrics5/jetty11/InstrumentedHttpChannelListener.java
@@ -166,35 +166,7 @@ public RatioGauge.Ratio getRatio() {
             }
         });
 
-        this.listener = new AsyncListener() {
-            private long startTime;
-
-            @Override
-            public void onTimeout(AsyncEvent event) throws IOException {
-                asyncTimeouts.mark();
-            }
-
-            @Override
-            public void onStartAsync(AsyncEvent event) throws IOException {
-                startTime = System.currentTimeMillis();
-                event.getAsyncContext().addListener(this);
-            }
-
-            @Override
-            public void onError(AsyncEvent event) throws IOException {
-            }
-
-            @Override
-            public void onComplete(AsyncEvent event) throws IOException {
-                final AsyncContextState state = (AsyncContextState) event.getAsyncContext();
-                final HttpServletRequest request = (HttpServletRequest) state.getRequest();
-                final HttpServletResponse response = (HttpServletResponse) state.getResponse();
-                updateResponses(request, response, startTime, true);
-                if (!state.getHttpChannelState().isSuspended()) {
-                    activeSuspended.dec();
-                }
-            }
-        };
+        this.listener = new AsyncAttachingListener();
     }
 
     @Override
@@ -355,4 +327,53 @@ private Timer requestTimer(String method) {
             }
         }
     }
+
+    private class AsyncAttachingListener implements AsyncListener {
+
+        @Override
+        public void onTimeout(AsyncEvent event) throws IOException {}
+
+        @Override
+        public void onStartAsync(AsyncEvent event) throws IOException {
+            event.getAsyncContext().addListener(new InstrumentedAsyncListener());
+        }
+
+        @Override
+        public void onError(AsyncEvent event) throws IOException {}
+
+        @Override
+        public void onComplete(AsyncEvent event) throws IOException {}
+    };
+
+    private class InstrumentedAsyncListener implements AsyncListener {
+        private final long startTime;
+
+        InstrumentedAsyncListener() {
+            this.startTime = System.currentTimeMillis();
+        }
+
+        @Override
+        public void onTimeout(AsyncEvent event) throws IOException {
+            asyncTimeouts.mark();
+        }
+
+        @Override
+        public void onStartAsync(AsyncEvent event) throws IOException {
+        }
+
+        @Override
+        public void onError(AsyncEvent event) throws IOException {
+        }
+
+        @Override
+        public void onComplete(AsyncEvent event) throws IOException {
+            final AsyncContextState state = (AsyncContextState) event.getAsyncContext();
+            final HttpServletRequest request = (HttpServletRequest) state.getRequest();
+            final HttpServletResponse response = (HttpServletResponse) state.getResponse();
+            updateResponses(request, response, startTime, true);
+            if (!state.getHttpChannelState().isSuspended()) {
+                activeSuspended.dec();
+            }
+        }
+    }
 }