add resin plugin #187
add resin plugin #187
Conversation
There was a problem hiding this comment.
My only concern is just the package name. Beyond that, I prefer you can provide a more complex demo, maybe like this:
HttpClient ---> Resin ---> MySQL.
I have invent you to join @sky-walking. After confirmed, you can create a repo for demo.
Later, we maybe set a mechanism to check agent by these demo automatically.
| @@ -0,0 +1,65 @@ | |||
| package org.skywalking.apm.plugin.resin.v3x.v4x; | |||
There was a problem hiding this comment.
The package name didn't fit the Java Package Tradition. In Java, *.v3x.v4x means 3 is major version and 4 is minor version. But, according from my understanding, 3 and 4 are all major versions.
So I prefer the package name is org.skywalking.apm.plugin.resin34x. TomcatInterceptor class under package org.skywalking.apm.plugin.tomcat78x is for your reference.
|
@wu-sheng , I've changed package name. |
|
Why the coverage decrease? |
| } | ||
|
|
||
| @Override | ||
| public Object afterMethod(EnhancedClassInstanceContext context, InstanceMethodInvokeContext interceptorContext, |
There was a problem hiding this comment.
Current span need to record status code. and also check the status code, Please see Tomcat plugin
| HttpServletRequest request = (HttpServletRequest)args[0]; | ||
| Span span = ContextManager.createSpan(request.getRequestURI()); | ||
| Tags.COMPONENT.set(span, RESIN_COMPONENT); | ||
| Tags.PEER_HOST.set(span, request.getServerName()); |
There was a problem hiding this comment.
Request#getServerName() cannot get the correct client host if Resin is behind a reverse proxy or load balancer. You may need to use HttpServletRequest.getHeader("x-forwarded-proto") instead.
|
I hava just removed the |
|
@wu-sheng , resin-3.x.x and resin-4.x.x are available |
|
@sky-walking, @sky-walking/contributors, Can we merge this now? |
* Remove JDK 14 from tests * Remove jdk14-with-gson test.
add resin plugin #184