From 2ffb0ea46a71034bd4bcc049856c4d4cb3a294b5 Mon Sep 17 00:00:00 2001 From: ascrutae Date: Wed, 11 Oct 2017 15:51:05 +0800 Subject: [PATCH] fix jetty plugin works incorrect --- .../jetty/v9/server/HandleInterceptor.java | 10 +++-- ...ntation.java => JettyInstrumentation.java} | 9 +++-- .../src/main/resources/skywalking-plugin.def | 2 +- .../v9/server/HandleInterceptorTest.java | 37 ++++++++++--------- 4 files changed, 32 insertions(+), 26 deletions(-) rename apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/java/org/skywalking/apm/plugin/jetty/v9/server/define/{HandlerListInstrumentation.java => JettyInstrumentation.java} (84%) diff --git a/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/java/org/skywalking/apm/plugin/jetty/v9/server/HandleInterceptor.java b/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/java/org/skywalking/apm/plugin/jetty/v9/server/HandleInterceptor.java index 35633385ae..c94834d1c1 100644 --- a/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/java/org/skywalking/apm/plugin/jetty/v9/server/HandleInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/java/org/skywalking/apm/plugin/jetty/v9/server/HandleInterceptor.java @@ -21,6 +21,7 @@ package org.skywalking.apm.plugin.jetty.v9.server; import java.lang.reflect.Method; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.server.HttpChannel; import org.skywalking.apm.agent.core.context.CarrierItem; import org.skywalking.apm.agent.core.context.ContextCarrier; import org.skywalking.apm.agent.core.context.ContextManager; @@ -36,8 +37,8 @@ public class HandleInterceptor implements InstanceMethodsAroundInterceptor { @Override public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, MethodInterceptResult result) throws Throwable { - String requestURI = (String)allArguments[0]; - HttpServletRequest servletRequest = (HttpServletRequest)allArguments[2]; + HttpChannel httpChannel = (HttpChannel)allArguments[0]; + HttpServletRequest servletRequest = httpChannel.getRequest(); ContextCarrier contextCarrier = new ContextCarrier(); @@ -47,7 +48,7 @@ public class HandleInterceptor implements InstanceMethodsAroundInterceptor { next.setHeadValue(servletRequest.getHeader(next.getHeadKey())); } - AbstractSpan span = ContextManager.createEntrySpan(requestURI, contextCarrier); + AbstractSpan span = ContextManager.createEntrySpan(servletRequest.getRequestURI(), contextCarrier); Tags.URL.set(span, servletRequest.getRequestURL().toString()); Tags.HTTP.METHOD.set(span, servletRequest.getMethod()); span.setComponent(ComponentsDefine.JETTY_SERVER); @@ -57,7 +58,8 @@ public class HandleInterceptor implements InstanceMethodsAroundInterceptor { @Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Object ret) throws Throwable { - HttpServletResponse servletResponse = (HttpServletResponse)allArguments[3]; + HttpChannel httpChannel = (HttpChannel)allArguments[0]; + HttpServletResponse servletResponse = httpChannel.getResponse(); AbstractSpan span = ContextManager.activeSpan(); if (servletResponse.getStatus() >= 400) { span.errorOccurred(); diff --git a/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/java/org/skywalking/apm/plugin/jetty/v9/server/define/HandlerListInstrumentation.java b/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/java/org/skywalking/apm/plugin/jetty/v9/server/define/JettyInstrumentation.java similarity index 84% rename from apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/java/org/skywalking/apm/plugin/jetty/v9/server/define/HandlerListInstrumentation.java rename to apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/java/org/skywalking/apm/plugin/jetty/v9/server/define/JettyInstrumentation.java index e2cf781f6b..0004ab160a 100644 --- a/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/java/org/skywalking/apm/plugin/jetty/v9/server/define/HandlerListInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/java/org/skywalking/apm/plugin/jetty/v9/server/define/JettyInstrumentation.java @@ -26,17 +26,18 @@ import org.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMet import org.skywalking.apm.agent.core.plugin.match.ClassMatch; import static net.bytebuddy.matcher.ElementMatchers.named; +import static org.skywalking.apm.agent.core.plugin.bytebuddy.ArgumentTypeNameMatch.takesArgumentWithType; import static org.skywalking.apm.agent.core.plugin.match.NameMatch.byName; /** - * {@link HandlerListInstrumentation} enhance the handle method in org.eclipse.jetty.server.handler.HandlerList + * {@link JettyInstrumentation} enhance the handle method in org.eclipse.jetty.server.handler.HandlerList * by org.skywalking.apm.plugin.jetty.v9.server.HandleInterceptor * * @author zhangxin */ -public class HandlerListInstrumentation extends ClassInstanceMethodsEnhancePluginDefine { +public class JettyInstrumentation extends ClassInstanceMethodsEnhancePluginDefine { - private static final String ENHANCE_CLASS = "org.eclipse.jetty.server.handler.HandlerList"; + private static final String ENHANCE_CLASS = "org.eclipse.jetty.server.Server"; private static final String ENHANCE_METHOD = "handle"; private static final String INTERCEPTOR_CLASS = "org.skywalking.apm.plugin.jetty.v9.server.HandleInterceptor"; @@ -48,7 +49,7 @@ public class HandlerListInstrumentation extends ClassInstanceMethodsEnhancePlugi return new InstanceMethodsInterceptPoint[] { new InstanceMethodsInterceptPoint() { @Override public ElementMatcher getMethodsMatcher() { - return named(ENHANCE_METHOD); + return named(ENHANCE_METHOD).and(takesArgumentWithType(0, "org.eclipse.jetty.server.HttpChannel")); } @Override public String getMethodsInterceptor() { diff --git a/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/resources/skywalking-plugin.def b/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/resources/skywalking-plugin.def index 0c7fcd02d8..053c257c2e 100644 --- a/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/resources/skywalking-plugin.def +++ b/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/main/resources/skywalking-plugin.def @@ -1 +1 @@ -jetty-server-9.x=org.skywalking.apm.plugin.jetty.v9.server.define.HandlerListInstrumentation +jetty-server-9.x=org.skywalking.apm.plugin.jetty.v9.server.define.JettyInstrumentation diff --git a/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/test/java/org/skywalking/apm/plugin/jetty/v9/server/HandleInterceptorTest.java b/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/test/java/org/skywalking/apm/plugin/jetty/v9/server/HandleInterceptorTest.java index bbc59cbc7c..089a565ccb 100644 --- a/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/test/java/org/skywalking/apm/plugin/jetty/v9/server/HandleInterceptorTest.java +++ b/apm-sniffer/apm-sdk-plugin/jetty-plugin/jetty-server-9.x-plugin/src/test/java/org/skywalking/apm/plugin/jetty/v9/server/HandleInterceptorTest.java @@ -19,9 +19,9 @@ package org.skywalking.apm.plugin.jetty.v9.server; import java.util.List; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; +import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.Response; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -58,7 +58,7 @@ import static org.skywalking.apm.agent.test.tools.SpanAssert.assertTag; @PowerMockRunnerDelegate(TracingSegmentRunner.class) public class HandleInterceptorTest { - private HandleInterceptor tomcatInvokeInterceptor; + private HandleInterceptor jettyInvokeInterceptor; @SegmentStoragePoint private SegmentStorage segmentStorage; @@ -66,36 +66,39 @@ public class HandleInterceptorTest { public AgentServiceRule serviceRule = new AgentServiceRule(); @Mock - private HttpServletRequest request; + private Request request; @Mock - private Request baseRequest; - @Mock - private HttpServletResponse response; + private Response response; @Mock private MethodInterceptResult methodInterceptResult; @Mock private EnhancedInstance enhancedInstance; + @Mock + private HttpChannel httpChannel; + private Object[] arguments; private Class[] argumentType; @Before public void setUp() throws Exception { - tomcatInvokeInterceptor = new HandleInterceptor(); + jettyInvokeInterceptor = new HandleInterceptor(); when(request.getRequestURI()).thenReturn("/test/testRequestURL"); when(request.getRequestURL()).thenReturn(new StringBuffer("http://localhost:8080/test/testRequestURL")); when(response.getStatus()).thenReturn(200); - arguments = new Object[] {"/test/testRequestURL", baseRequest, request, response}; - argumentType = new Class[] {String.class, baseRequest.getClass(), request.getClass(), response.getClass()}; + when(httpChannel.getResponse()).thenReturn(response); + when(httpChannel.getRequest()).thenReturn(request); + arguments = new Object[] {httpChannel}; + argumentType = new Class[] {httpChannel.getClass()}; } @Test public void testWithoutSerializedContextData() throws Throwable { - tomcatInvokeInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); - tomcatInvokeInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); + jettyInvokeInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); + jettyInvokeInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); assertThat(segmentStorage.getTraceSegments().size(), is(1)); TraceSegment traceSegment = segmentStorage.getTraceSegments().get(0); @@ -107,8 +110,8 @@ public class HandleInterceptorTest { public void testWithSerializedContextData() throws Throwable { when(request.getHeader(SW3CarrierItem.HEADER_NAME)).thenReturn("1.234.111|3|1|1|#192.168.1.8:18002|#/portal/|#/testEntrySpan|#AQA*#AQA*Et0We0tQNQA*"); - tomcatInvokeInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); - tomcatInvokeInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); + jettyInvokeInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); + jettyInvokeInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); assertThat(segmentStorage.getTraceSegments().size(), is(1)); TraceSegment traceSegment = segmentStorage.getTraceSegments().get(0); @@ -120,9 +123,9 @@ public class HandleInterceptorTest { @Test public void testWithOccurException() throws Throwable { - tomcatInvokeInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); - tomcatInvokeInterceptor.handleMethodException(enhancedInstance, null, arguments, argumentType, new RuntimeException()); - tomcatInvokeInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); + jettyInvokeInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); + jettyInvokeInterceptor.handleMethodException(enhancedInstance, null, arguments, argumentType, new RuntimeException()); + jettyInvokeInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); assertThat(segmentStorage.getTraceSegments().size(), is(1)); TraceSegment traceSegment = segmentStorage.getTraceSegments().get(0); -- GitLab