Commit a04843a6 authored by 吴晟's avatar 吴晟 Committed by GitHub
Browse files

Merge pull request #376 from ascrutae/zhangxin/fix/tomcat-error-capture 

Fix issue that tomcat plugin cannot capture exception
parents aac05bce fa42cd47

apm-sniffer/apm-agent-core/src/main/java/org/skywalking/apm/agent/core/context/util/KeyValuePair.java

+3 −1
Original line number Diff line number Diff line
@@ -27,7 +27,9 @@ public class KeyValuePair { 
    public KeyWithStringValue transform() {

        KeyWithStringValue.Builder keyValueBuilder = KeyWithStringValue.newBuilder();

        keyValueBuilder.setKey(key);

        if (value != null) {

            keyValueBuilder.setValue(value);

        }

        return keyValueBuilder.build();

    }

}

apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatExceptionInterceptor.java

0 → 100644
+26 −0
Original line number Diff line number Diff line 
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) {



    }

}

apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatInterceptor.java→apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptor.java

+2 −2
Original line number Diff line number Diff line
@@ -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

apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/skywalking/apm/plugin/tomcat78x/define/TomcatInstrumentation.java

+27 −7
Original line number Diff line number Diff line
@@ -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<MethodDescription> getMethodsMatcher() {

                    return named("exception");

                }



                @Override public String getMethodsInterceptor() {

                    return EXCEPTION_INTERCEPT_CLASS;

                }



                @Override public boolean isOverrideArgs() {

                    return false;

                }

            }

        };

    }

apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/test/java/org/skywalking/apm/plugin/tomcat78x/TomcatInterceptorTest.java→apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/test/java/org/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptorTest.java

+34 −10
Original line number Diff line number Diff line
@@ -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<AbstractTracingSpan> spans = SegmentHelper.getSpans(traceSegment);



        assertHttpSpan(spans.get(0));

        List<LogDataEntity> 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);