Fix for the monitoring jetty9 async servlet requests. #715
Conversation
|
Any chance somebody can have a quick look at this please? |
tempredirect
force-pushed
the
fix_jetty9_async
branch
from
db8bb55
to
7bcb379
Compare
|
Have rebased this request against master. |
Async requests are monitored via the addition of an async listener. The bug was that `onStartAsync(AsyncEvent)` is called when the handler invokes `startAsync()` but the `InstrumentedHandler` only added the listener after the call to the delegate handler, thus would miss the all onStartAsync event. The net result of the bug was that the start time of all async requests was reported as 0 so the calculated elapsed times where then rather large (i.e. currentTimeMillis()).
tempredirect
force-pushed
the
fix_jetty9_async
branch
from
7bcb379
to
aa39e42
Compare
|
Hi, anybody fancy having a look at this?? |
|
@tempredirect I'm afraid I'm not terribly knowledgable about Jetty. I've been hoping that someone who is more knowledgable would come along and sign off on this PR! |
|
I can confirm that InstrumentedHandler is currently not working with async servlets and this change fixes it. This will also solve #914 |
|
Can also confirm this fixes the same problem as others are seeing. EDIT - this definitely causes a reasonable number to be reported but I'm not sure if it is the right one. The metric reported does not seem to be the same as that coming from the Jetty request log. In my environment, over an hour window the average response latency from the Jetty request log is 287 ms, but the metric reported with this fix is more like 140ms. |
|
I have the same issue, are there any plans on merging this PR? |
|
@jonathanmgrimm have you had a chance to look into this any further? I was going to merge it, but your comment concerns me. |
|
Merging to 3.2 in #1074 |
Async requests are monitored via the addition of an async listener.
The bug was that
onStartAsync(AsyncEvent)is called when the handlerinvokes
startAsync()but theInstrumentedHandleronly added thelistener after the call to the delegate handler, thus would miss
the all onStartAsync event.
The net result of the bug was that the start time of all async requests
was reported as 0 so the calculated elapsed times where then rather
large (i.e. currentTimeMillis()).