Improve getAggregateRequestName for the http-counter to support spring best matched pattern

[GoogleCodeExporter]

Improvement:
If using spring-mvc (at least with @RequestMapping annotations), spring sets 
the attribute 
"org.springframework.web.servlet.HandlerMapping.bestMatchingPattern" to the 
pattern that receives the current request.

This is more powerful and easy way to do what {http-transform-pattern} almost 
does.

I've attached a patch against trunk (r1565) for an implementation of this.
It's a "change as little as possible" patch. E.g. the use of 
CounterError.getCurrentRequest() should possibly be moved somewhere reasonable.

Espen

Original issue reported on code.google.com by espenamb...@gmail.com on 17 Dec 2010 at 12:33

Attachments:

• spring-best-match.patch

[GoogleCodeExporter]

Original comment by evernat@free.fr on 18 Dec 2010 at 1:21

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

Thanks for the issue and for the patch.
But I suggest using a simpler patch (MonitoringFilter.patch joined) which is 
based on MonitoringFilter.java revision 1612. The modified 
MonitoringFilter.java is also joined.
I think that this patch is equivalent to the other patch.

I have made a new build from the current trunk + the patch and it is available 
at:
http://javamelody.googlecode.com/files/javamelody-20110102.jar

Does it work for you?

Original comment by evernat@free.fr on 2 Jan 2011 at 3:31

Attachments:

• MonitoringFilter.patch
• MonitoringFilter.java

[GoogleCodeExporter]

oups, I updated the 3 files: it is even better now

Original comment by evernat@free.fr on 2 Jan 2011 at 4:50

Attachments:

• MonitoringFilter.patch
• MonitoringFilter.java

[GoogleCodeExporter]

Thanks for your work on this!

However the current patch does not work. getCompleteRequestName is called 
before chain.doFilter so the attribute 
org.springframework.web.servlet.HandlerMapping.bestMatchingPattern has not been 
set by spring yet. (Since we have MonitoringFilter as the first filter in the 
chain.)

Just a thought.
For those who do not want to use this feature, it should probably be 
configurable whether this feature is enabled or not. Default should be disabled 
for backwards compatibility?

Original comment by espenamb...@gmail.com on 3 Jan 2011 at 7:59

[GoogleCodeExporter]

Ah yes.
I try again: I have joined a new patch which should work as well as your patch 
(I think). I have also updated the new build with the patch available at:
http://javamelody.googlecode.com/files/javamelody-20110102.jar

Does it work this time?

But I have thought about this, I think that your patch and also the last of 
mine may have both a big overhead on memory and on hard disk.
The reason is that the "current requests" part of the reports in 
HtmlCounterRequestContextReport will call 
Counter.getCounterRequest(CounterRequestContext) with the initial "bad" 
requestName and each report may add many instances of CounterRequest in the 
statistics.

It seems to me that your patch has this issue because the call in your patch to 
CounterError.getCurrentRequest() will not work in this case for the threads and 
the contexts in the "current requests" part: it could only work for the current 
thread.
And my patch doesn't even try to call this method.
So I think that we can think about that again, but perhaps later.

Original comment by evernat@free.fr on 3 Jan 2011 at 10:41

Attachments:

• MonitoringFilter.patch
• MonitoringFilter.java

[GoogleCodeExporter]

The patch works :)

I've tried to check if there are overhead issues with this patch, and it seems 
there is some. However my understanding of the code is not nearly complete so 
...

The attached patch attempts to remove the overhead. It does not add a 
CounterRequest to Counter.requests when called from 
HtmlCounterRequestContextReport. This way there is no "extra" CounterRequest's 
added ...

I still think there should be an option to turn is on/off though.

Original comment by espenamb...@gmail.com on 4 Jan 2011 at 11:01

Attachments:

• Counter.patch

[GoogleCodeExporter]

Hi,

Have you looked at this lately?

Espen

Original comment by espenamb...@gmail.com on 31 Jan 2011 at 1:55

[GoogleCodeExporter]

Not lately (there is no schedule for this enhancement).

Original comment by evernat@free.fr on 2 Feb 2011 at 1:15