Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GWT-RPC feature patch #88

Closed
GoogleCodeExporter opened this issue Jun 21, 2015 · 10 comments
Closed

GWT-RPC feature patch #88

GoogleCodeExporter opened this issue Jun 21, 2015 · 10 comments

Comments

@GoogleCodeExporter
Copy link

Attached please find for contribution (LGPL/ownership of javamelody) a small 
patch to support GWT-RPC detail statistic collection.

==================
GWT-RPC detailed statistic collection feature:

Breaks up general servlet calls (GwtServiceServlet) into the specific GWT-RPC 
calls for statistic collection (GwtServiceServlet.LOGIN GWT-RPC, 
GwtServiceServlet.Data_retrieve_method GWT-RPC, GwtServiceServlet.update_data 
GWT-RPC, etc).

Only enabled when GWT-RPC is detected (and 'http').

Works with GWT 1.3 to 2.1 (to date).  Will not work with deRPC/RequestFactory 
transport mechanisms, only GWT-RPC specifically.

==================
tech notes: Been running on jboss and tomcat servers since December 2009, based 
originally on 1.10 version of javamelody, re-applied to 1.15 version and still 
fine.  

This 1.26 patch/contribution is provided for inclusion to Javamelody as it has 
been of tremendous value as a GWT developer.  Note that the new JSPWrapper code 
in the MonitoringFilter may or may not intercede/mix well with the GWT patch, 
but to date I have not had any issues.

Humbly provided for the excellent javamelody project,
-dhartford

Original issue reported on code.google.com by binarymo...@gmail.com on 3 Feb 2011 at 10:45

Attachments:

@GoogleCodeExporter
Copy link
Author

Many thanks for the contribution. I think that it will be useful for GWT 
applications.

I have included your patch (revision 1736) and I have also tried to refactor 
the style to the javamelody's style, the "originalPayload" type and the streams 
(revision 1737, revision 1738 and revision 1739).
The files are now:
http://code.google.com/p/javamelody/source/browse/trunk/javamelody-core/src/main
/java/net/bull/javamelody/GWTRequestWrapper.java
and 
http://code.google.com/p/javamelody/source/browse/trunk/javamelody-core/src/main
/java/net/bull/javamelody/MonitoringFilter.java
And I have made a new build from the current trunk and it is available at:
http://javamelody.googlecode.com/files/javamelody-20110206.jar

Is this ok for you and does it work for your application?

The behavior may be different on charsets, so you could test some characters 
such as the euro symbols or accented characters if applicable.

And one part where I am not sure is "payload = new String(originalPayload)" : 
perhaps we could use "payload = new String(originalPayload, 
request.getCharacterEncoding())" when the character encoding is not null, or 
perhaps this will not change anything.

Original comment by evernat@free.fr on 6 Feb 2011 at 10:42

  • Changed state: Accepted
  • Added labels: Type-Enhancement
  • Removed labels: Type-Defect

@GoogleCodeExporter
Copy link
Author

Thanks for looking at the patch.  The charset issue is something that I've had 
trouble with in the past, but somehow I got it worked out.

Unfortunately, using the 20110206.jar my deploy is coming back with:

 java.lang.ClassCastException: org.apache.catalina.connector.RequestFacade cannot be cast to net.bull.javamelody.GWTRequestWrapper

Original comment by binarymo...@gmail.com on 7 Feb 2011 at 12:31

@GoogleCodeExporter
Copy link
Author

Rest of stack:

    at net.bull.javamelody.MonitoringFilter.getCompleteRequestName(MonitoringFilter.java:323)
    at net.bull.javamelody.MonitoringFilter.doFilter(MonitoringFilter.java:147) 

    at net.bull.javamelody.MonitoringFilter.doFilter(MonitoringFilter.java:131) 


Original comment by binarymo...@gmail.com on 7 Feb 2011 at 12:35

@GoogleCodeExporter
Copy link
Author

Further research similar issues with other applications (struts for example) 
where going from jboss 4 to jboss 5 also had similar issues with the 
RequestFacade.  Some issues pointed to servlet-api.jar within the application.  
Other people found updating the servlet-api.jar from jboss 5 with an older 
version of jboss 4 corrected the issue.

I'm using a stock jboss 6 install, and the same application (and javamelody) 
without the servlet-api.jar anywhere in the application.  Replacing the jboss 
version of servlet-api is needless to say not an option :-)

The old (inefficient) one was working fine, I'll compare the old with the 
refactored and see if I find anything.

Original comment by binarymo...@gmail.com on 7 Feb 2011 at 12:56

@GoogleCodeExporter
Copy link
Author

MonitoringFilter: 136

If I comment out
//HttpServletRequest wrappedRequest = 
JspWrapper.createHttpRequestWrapper(httpRequest);
and change the rest of the 'wrappedRequest' back to normal httpRequest variable 
names, everything works fine.

Also tested special characters without the JspWrapper and those do work fine. 

1.26-SNAPSHOT of javamelody is the first time I've run the JspWrapper on Jboss, 
so this may be a latent issue.

Original comment by binarymo...@gmail.com on 7 Feb 2011 at 1:37

@GoogleCodeExporter
Copy link
Author

Sorry, it is just a bug in my refactoring. I will fix that and build a new jar 
for tomorrow. Thanks.

Original comment by evernat@free.fr on 7 Feb 2011 at 1:53

@GoogleCodeExporter
Copy link
Author

It should be fixed now (revision 1744).
You can use the new build:
http://javamelody.googlecode.com/files/javamelody-20110207.jar

Is this ok for you and does it work?

Original comment by evernat@free.fr on 7 Feb 2011 at 8:17

@GoogleCodeExporter
Copy link
Author

confirmed, looks good now!  Special characters passed. Also, thanks for keeping 
the same format for the method name, my historical information and this jar are 
showing the information correctly and incrementing correctly.


Original comment by binarymo...@gmail.com on 8 Feb 2011 at 1:48

@GoogleCodeExporter
Copy link
Author

It is all good then.
Thanks.

Original comment by evernat@free.fr on 8 Feb 2011 at 10:51

  • Changed state: Fixed

@GoogleCodeExporter
Copy link
Author

Original comment by evernat@free.fr on 8 Feb 2011 at 10:52

  • Changed state: Verified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant