me0wster / javamelody

Automatically exported from code.google.com/p/javamelody
0 stars 0 forks source link

Alternative servlet to generate the same monitoring reports #227

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
First off, this is a great and helpful package -- easy and quick to use, thanks!

My request may be similar to others but it is specifically that the generation 
of the monitoring report be separated from the monitoring filter.

In our case, depending on its location in the filtering chain we either expose 
the /monitoring url outside of security or we miss monitoring the security 
portion (and thereby don't monitor aspects of the system which affect our 
users).

My approach to addressing this was to separate the report generation into a 
servlet which could be included and configured separately. While there are 
other approaches, in our case they would require significant reconfiguration of 
our application whereas this was relatively simple to do.

More importantly, this separates the session monitoring (the filter) from the 
report generation. It seems unexpected and more than a little confusing to me 
to have the filter handle such a task, shortcutting the rest of the chain and 
returning output more complex than expected of a filter.

To demonstrate my thought I've included a patch, I'm sure you could put 
together something better though.

Original issue reported on code.google.com by petersky on 13 Jun 2012 at 10:59

Attachments:

GoogleCodeExporter commented 9 years ago
Thank you "petersky".
Note to myself: the patch is missing "isRequestNotAllowed".

Original comment by evernat@free.fr on 14 Jun 2012 at 10:35

GoogleCodeExporter commented 9 years ago
I have made some changes, such as keeping the monitoring reports in 
MonitoringFilter, in order to be simpler in general (no added servlet) and to 
be backward compatible.
The new patch is attached.

Is it ok for your case?

Original comment by evernat@free.fr on 17 Jun 2012 at 12:12

Attachments:

GoogleCodeExporter commented 9 years ago
I would also appreciate much the seperation of the data gathering (via 
ServletFilter) and report generation (via Servlet) for two important reasons:

1. Both tasks are so different that they should not reside inside the same 
class/layer
2. The "monitoring" task fits better to a ServletFilter while the report 
generation fits better to a Servlet. It's confusing that a Filter "creates" a 
page - this is not the work that servlet filters are intended to do.

Original comment by mineral_...@gmx.de on 17 Jun 2012 at 12:23

GoogleCodeExporter commented 9 years ago
I think the new patch would work and I agree it is good to be backward 
compatible -- one thing I don't see with this patch though is a way to disable 
the filter from serving the request. That is, when the servlet is being used to 
serve the report then it is probably desired to not have the filter doing it 
also. Some kind of flag perhaps?

Original comment by petersky on 20 Jun 2012 at 4:54

GoogleCodeExporter commented 9 years ago
Yes, you can use some kind of flag. You can add an init parameter inside the 
<filter>...</filter> tags of the MonitoringFilter in the web.xml file, in order 
to disable the reports of the filter:

        <!-- the monitoring reports of the MonitoringFilter are disabled -->
        <init-param>
            <param-name>monitoring-path</param-name>
            <param-value>???</param-value>
        </init-param>

Original comment by evernat@free.fr on 24 Jun 2012 at 12:42

GoogleCodeExporter commented 9 years ago
The ReportServlet is now included in trunk (see revision 2917, which is the 
same as the ReportServlet.patch above except the FILTER_CONTEXT_KEY).
It is ready for the next release (1.40).

If you want to use it before the release, I have made a new build and it is 
available at:
https://javamelody.googlecode.com/files/javamelody-20120624.jar

For all other people seeing this issue and thinking if it may be for them: 
probably not, because this is not the standard way explained in the user guide.
And this ReportServlet is rarely needed, because the standard .../monitoring 
url of the MonitoringFilter is enough for the reports of most webapps.

Original comment by evernat@free.fr on 24 Jun 2012 at 1:08

GoogleCodeExporter commented 9 years ago

Original comment by evernat@free.fr on 24 Jun 2012 at 1:11