diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/skywalking/apm/agent/core/context/util/KeyValuePair.java b/apm-sniffer/apm-agent-core/src/main/java/org/skywalking/apm/agent/core/context/util/KeyValuePair.java index 76f4227dfe9c1ee158dd8fe6c77721238d72ab2a..4e6b93d417b76ba73d0bb3e87e95dc24a6c7825c 100644 --- a/apm-sniffer/apm-agent-core/src/main/java/org/skywalking/apm/agent/core/context/util/KeyValuePair.java +++ b/apm-sniffer/apm-agent-core/src/main/java/org/skywalking/apm/agent/core/context/util/KeyValuePair.java @@ -27,7 +27,9 @@ public class KeyValuePair { public KeyWithStringValue transform() { KeyWithStringValue.Builder keyValueBuilder = KeyWithStringValue.newBuilder(); keyValueBuilder.setKey(key); - keyValueBuilder.setValue(value); + if (value != null) { + keyValueBuilder.setValue(value); + } return keyValueBuilder.build(); } } diff --git a/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatExceptionInterceptor.java b/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatExceptionInterceptor.java new file mode 100644 index 0000000000000000000000000000000000000000..7d4bcef417d44e579d4224e56fb975f9eb2bc2f2 --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatExceptionInterceptor.java @@ -0,0 +1,26 @@ +package org.skywalking.apm.plugin.tomcat78x; + +import java.lang.reflect.Method; +import org.skywalking.apm.agent.core.context.ContextManager; +import org.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance; +import org.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor; +import org.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult; + +public class TomcatExceptionInterceptor implements InstanceMethodsAroundInterceptor { + @Override + public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, + MethodInterceptResult result) throws Throwable { + ContextManager.activeSpan().errorOccurred().log((Throwable)allArguments[2]); + } + + @Override + public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, + Object ret) throws Throwable { + return ret; + } + + @Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, + Class[] argumentsTypes, Throwable t) { + + } +} diff --git a/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatInterceptor.java b/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptor.java similarity index 93% rename from apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatInterceptor.java rename to apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptor.java index fed1f7f1c18db9d854f1d6551a2f8abf09f9210a..90348dd3e56fd7889092eb5802252f20c3625f80 100644 --- a/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptor.java @@ -16,11 +16,11 @@ import org.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptR import org.skywalking.apm.network.trace.component.ComponentsDefine; /** - * {@link TomcatInterceptor} fetch the serialized context data by using {@link HttpServletRequest#getHeader(String)}. + * {@link TomcatInvokeInterceptor} fetch the serialized context data by using {@link HttpServletRequest#getHeader(String)}. * The {@link TraceSegment#refs} of current trace segment will reference to the trace * segment id of the previous level if the serialized context is not null. */ -public class TomcatInterceptor implements InstanceMethodsAroundInterceptor { +public class TomcatInvokeInterceptor implements InstanceMethodsAroundInterceptor { /** * * The {@link TraceSegment#refs} of current trace segment will reference to the diff --git a/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/define/TomcatInstrumentation.java b/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/define/TomcatInstrumentation.java index 971aa51262abfc44fb868960d3d97c0e779d74d5..c8119c52b128b2b744cc80326fbfa1dc3cf6a28f 100644 --- a/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/define/TomcatInstrumentation.java +++ b/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/define/TomcatInstrumentation.java @@ -8,14 +8,16 @@ import org.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoin import org.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint; import org.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine; import org.skywalking.apm.agent.core.plugin.match.ClassMatch; -import org.skywalking.apm.plugin.tomcat78x.TomcatInterceptor; +import org.skywalking.apm.plugin.tomcat78x.TomcatInvokeInterceptor; import static net.bytebuddy.matcher.ElementMatchers.named; import static org.skywalking.apm.agent.core.plugin.match.NameMatch.byName; /** - * {@link TomcatInstrumentation} presents that skywalking using class {@link TomcatInterceptor} to - * intercept {@link org.apache.catalina.core.StandardEngineValve#invoke(Request, Response)}. + * {@link TomcatInstrumentation} presents that skywalking using class {@link TomcatInvokeInterceptor} to intercept + * {@link org.apache.catalina.core.StandardWrapperValve#invoke(Request, Response)} and using class {@link + * org.skywalking.apm.plugin.tomcat78x.TomcatExceptionInterceptor} to intercept {@link + * org.apache.catalina.core.StandardWrapperValve#exception(Request, Response, Throwable)}. * * @author zhangxin */ @@ -24,12 +26,17 @@ public class TomcatInstrumentation extends ClassInstanceMethodsEnhancePluginDefi /** * Enhance class. */ - private static final String ENHANCE_CLASS = "org.apache.catalina.core.StandardEngineValve"; + private static final String ENHANCE_CLASS = "org.apache.catalina.core.StandardWrapperValve"; /** - * Intercept class. + * The intercept class for "invoke" method in the class "org.apache.catalina.core.StandardWrapperValve" */ - private static final String INTERCEPT_CLASS = "org.skywalking.apm.plugin.tomcat78x.TomcatInterceptor"; + private static final String INVOKE_INTERCEPT_CLASS = "org.skywalking.apm.plugin.tomcat78x.TomcatInvokeInterceptor"; + + /** + * The intercept class for "exception" method in the class "org.apache.catalina.core.StandardWrapperValve" + */ + private static final String EXCEPTION_INTERCEPT_CLASS = "org.skywalking.apm.plugin.tomcat78x.TomcatExceptionInterceptor"; @Override protected ClassMatch enhanceClass() { @@ -52,13 +59,26 @@ public class TomcatInstrumentation extends ClassInstanceMethodsEnhancePluginDefi @Override public String getMethodsInterceptor() { - return INTERCEPT_CLASS; + return INVOKE_INTERCEPT_CLASS; } @Override public boolean isOverrideArgs() { return false; } + }, + new InstanceMethodsInterceptPoint() { + @Override public ElementMatcher getMethodsMatcher() { + return named("exception"); + } + + @Override public String getMethodsInterceptor() { + return EXCEPTION_INTERCEPT_CLASS; + } + + @Override public boolean isOverrideArgs() { + return false; + } } }; } diff --git a/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/test/java/org/skywalking/apm/plugin/tomcat78x/TomcatInterceptorTest.java b/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/test/java/org/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptorTest.java similarity index 68% rename from apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/test/java/org/skywalking/apm/plugin/tomcat78x/TomcatInterceptorTest.java rename to apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/test/java/org/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptorTest.java index f834140425fbe7569ed562c87f8c772fcf4ecee8..8a995d6e55034bffe80693e328bacfb78ab0eef3 100644 --- a/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/test/java/org/skywalking/apm/plugin/tomcat78x/TomcatInterceptorTest.java +++ b/apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/test/java/org/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptorTest.java @@ -37,9 +37,10 @@ import static org.skywalking.apm.agent.test.tools.SpanAssert.assertTag; @RunWith(PowerMockRunner.class) @PowerMockRunnerDelegate(TracingSegmentRunner.class) -public class TomcatInterceptorTest { +public class TomcatInvokeInterceptorTest { - private TomcatInterceptor tomcatInterceptor; + private TomcatInvokeInterceptor tomcatInvokeInterceptor; + private TomcatExceptionInterceptor tomcatExceptionInterceptor; @SegmentStoragePoint private SegmentStorage segmentStorage; @@ -59,20 +60,27 @@ public class TomcatInterceptorTest { private Object[] arguments; private Class[] argumentType; + private Object[] exceptionArguments; + private Class[] exceptionArgumentType; + @Before public void setUp() throws Exception { - tomcatInterceptor = new TomcatInterceptor(); + tomcatInvokeInterceptor = new TomcatInvokeInterceptor(); + tomcatExceptionInterceptor = new TomcatExceptionInterceptor(); 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[] {request, response}; argumentType = new Class[] {request.getClass(), response.getClass()}; + + exceptionArguments = new Object[] {request, response, new RuntimeException()}; + exceptionArgumentType = new Class[] {request.getClass(), response.getClass(), new RuntimeException().getClass()}; } @Test public void testWithoutSerializedContextData() throws Throwable { - tomcatInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); - tomcatInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); + tomcatInvokeInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); + tomcatInvokeInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); assertThat(segmentStorage.getTraceSegments().size(), is(1)); TraceSegment traceSegment = segmentStorage.getTraceSegments().get(0); @@ -84,8 +92,8 @@ public class TomcatInterceptorTest { public void testWithSerializedContextData() throws Throwable { when(request.getHeader(Config.Plugin.Propagation.HEADER_NAME)).thenReturn("#AQA*#AQA*4WcWe0tQNQA*|3|1|1|#192.168.1.8:18002|#/portal/|#/testEntrySpan|#AQA*#AQA*Et0We0tQNQA*"); - tomcatInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); - tomcatInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); + tomcatInvokeInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); + tomcatInvokeInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); assertThat(segmentStorage.getTraceSegments().size(), is(1)); TraceSegment traceSegment = segmentStorage.getTraceSegments().get(0); @@ -97,9 +105,25 @@ public class TomcatInterceptorTest { @Test public void testWithOccurException() throws Throwable { - tomcatInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); - tomcatInterceptor.handleMethodException(enhancedInstance, null, arguments, argumentType, new RuntimeException()); - tomcatInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); + tomcatInvokeInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); + tomcatInvokeInterceptor.handleMethodException(enhancedInstance, null, arguments, argumentType, new RuntimeException()); + tomcatInvokeInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); + + assertThat(segmentStorage.getTraceSegments().size(), is(1)); + TraceSegment traceSegment = segmentStorage.getTraceSegments().get(0); + List spans = SegmentHelper.getSpans(traceSegment); + + assertHttpSpan(spans.get(0)); + List logDataEntities = SpanHelper.getLogs(spans.get(0)); + assertThat(logDataEntities.size(), is(1)); + assertException(logDataEntities.get(0), RuntimeException.class); + } + + @Test + public void testWithTomcatException() throws Throwable { + tomcatInvokeInterceptor.beforeMethod(enhancedInstance, null, arguments, argumentType, methodInterceptResult); + tomcatExceptionInterceptor.beforeMethod(enhancedInstance, null, exceptionArguments, exceptionArgumentType, null); + tomcatInvokeInterceptor.afterMethod(enhancedInstance, null, arguments, argumentType, null); assertThat(segmentStorage.getTraceSegments().size(), is(1)); TraceSegment traceSegment = segmentStorage.getTraceSegments().get(0);