apache / skywalking

APM, Application Performance Monitoring System
https://skywalking.apache.org/
Apache License 2.0
23.55k stars 6.47k forks source link

[Bug] Spring MVC And Tomcat plugin not work correctly in certain case #12425

Closed honganan closed 2 weeks ago

honganan commented 2 weeks ago

Search before asking

Apache SkyWalking Component

Java Agent (apache/skywalking-java)

What happened

If users are using Spring framework 6.x or Tomcat 10.x and imported javax.servlet:javax.servlet-api at the same time, both these two plugin would throw exceptions as their witness class and version recognization is not rigorous enough.

What you expected to happen

Error logs:

ERROR 2024-05-22 07:18:08:050 http-nio-7023-exec-1 InstMethodsInter : class[class com.xxx.boot.autoconfigure.web.HealthCheckServletApi] before method[doHC] intercept failure 
java.lang.IllegalStateException: this line should not be reached
    at org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor.AbstractMethodInterceptor.beforeMethod(AbstractMethodInterceptor.java:184)
    at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:76)
    at com.xxx.boot.autoconfigure.web.HealthCheckServletApi.doHC(HealthCheckServletApi.java)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
...

ERROR 2024-05-22 07:18:08:051 http-nio-7023-exec-1 InstMethodsInter : class[class com.xxx.boot.autoconfigure.web.HealthCheckServletApi] after method[doHC] intercept failure 
org.apache.skywalking.apm.plugin.spring.mvc.commons.exception.IllegalMethodStackDepthException: Please submit an new issue to Apache Skywalking if this Exception was happened.
    at org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor.AbstractMethodInterceptor.afterMethod(AbstractMethodInterceptor.java:217)
    at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:97)
    at com.xxx.boot.autoconfigure.web.HealthCheckServletApi.doHC(HealthCheckServletApi.java)
ERROR 2024-05-22 07:18:08:137 http-nio-7023-exec-1 InstMethodsInter : class[class org.apache.catalina.core.StandardHostValve] after method[invoke] intercept failure 
java.lang.ClassCastException: class org.apache.catalina.connector.Response cannot be cast to class javax.servlet.http.HttpServletResponse (org.apache.catalina.connector.Response and javax.servlet.http.HttpServletResponse are in unnamed module of loader org.springframework.boot.loader.LaunchedURLClassLoader @4451f60c)
    at org.apache.skywalking.apm.plugin.tomcat78x.TomcatInvokeInterceptor.afterMethod(TomcatInvokeInterceptor.java:93)
    at org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstMethodsInter.intercept(InstMethodsInter.java:97)
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java)
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:93)
...

How to reproduce

Just adding a javax.servlet:javax.servlet-api dependency in 'spring-6.x-scenario' test:

        <dependency>
            <groupId>javax.servlet</groupId>
            <artifactId>javax.servlet-api</artifactId>
            <version>3.1.0</version>
        </dependency>

then run this scenario test:

bash ./test/plugin/run.sh -f spring-6.x-scenario

After it runs finished we can check the comparasion result and agent log, for me is test/workspace/spring-6.x-scenario/6.0.4/skywalking-api.log

Anything else

trigger condition conclusion: case 1: using Spring 6.x together with Spring 5.x plugin or olders & imported javax.servlet-api dependency case 2: using Tomcat 10.x together with Tomcat 78.x plugin & imported javax.servlet-api dependency

Are you willing to submit a pull request to fix on your own?

Code of Conduct

wu-sheng commented 2 weeks ago

I don't think we supported these ways.

Are you going to fix?

honganan commented 2 weeks ago

Hi Wu! Yes, I am going to fix it. I am verifying it in our internal environment.

What do you mean these ways are not supported? Using an agent containing both newer and older version plugins is normal for us, and importing a commonly used dependency shouldn't affect them.

From my perspective, witnessing a class that does not belong to the framework itself is not robust enough.

wu-sheng commented 2 weeks ago

If you are going to fix, good to go.

I mean the plugins don't cover your cases, this is an enhancement rather than a bug.

wu-sheng commented 2 weeks ago

Cose for now. The pull request includes more context.