diff --git a/.github/workflows/plugins-tomcat9-test.0.yaml b/.github/workflows/plugins-tomcat9-test.0.yaml new file mode 100644 index 000000000..8f5408025 --- /dev/null +++ b/.github/workflows/plugins-tomcat9-test.0.yaml @@ -0,0 +1,69 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +name: Test + +on: + pull_request: + paths: + - '.github/workflows/plugins-*.yaml' + - 'apm-application-toolkit/**' + - 'apm-commons/**' + - 'apm-protocol/**' + - 'apm-sniffer/**' + - 'test/plugin/**' + - '**/pom.xml' + - '!**.md' + +concurrency: + group: plugins-tomcat9-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + build: + name: Build + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@v2 + with: + submodules: true + - name: Build + uses: ./.github/actions/build + with: + base_image_tomcat: tomcat:9.0.71-jdk8 + + test: + needs: [ build ] + name: ${{ matrix.case }} + runs-on: ubuntu-latest + timeout-minutes: 90 + strategy: + matrix: + case: + - tomcat-9x-scenario + steps: + - uses: actions/checkout@v2 + with: + submodules: true + - uses: actions/setup-java@v2 + with: + distribution: temurin + java-version: 8 + - name: Run Plugin Test + uses: ./.github/actions/run + with: + test_case: ${{ matrix.case }} diff --git a/CHANGES.md b/CHANGES.md index 7898f75a8..3fbe3a897 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -18,6 +18,7 @@ Release Notes. * Change gRPC instrumentation point to fix plugin not working for server side. * Fix servicecomb plugin trace break. * Adapt Armeria's plugins to the latest version 1.22.x +* Fix tomcat-10x-plugin and add test case to support tomcat7.x-8.x-9.x. #### Documentation * Update docs of Tracing APIs, reorganize the API docs into six parts. diff --git a/apm-sniffer/apm-sdk-plugin/tomcat-10x-plugin/src/main/java/org/apache/skywalking/apm/plugin/tomcat10x/TomcatInvokeInterceptor.java b/apm-sniffer/apm-sdk-plugin/tomcat-10x-plugin/src/main/java/org/apache/skywalking/apm/plugin/tomcat10x/TomcatInvokeInterceptor.java index fde1fc270..52bedf994 100644 --- a/apm-sniffer/apm-sdk-plugin/tomcat-10x-plugin/src/main/java/org/apache/skywalking/apm/plugin/tomcat10x/TomcatInvokeInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/tomcat-10x-plugin/src/main/java/org/apache/skywalking/apm/plugin/tomcat10x/TomcatInvokeInterceptor.java @@ -31,7 +31,6 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedI import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor; import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult; import org.apache.skywalking.apm.agent.core.util.CollectionUtil; -import org.apache.skywalking.apm.agent.core.util.MethodUtil; import org.apache.skywalking.apm.network.trace.component.ComponentsDefine; import org.apache.skywalking.apm.util.StringUtil; import org.apache.tomcat.util.http.Parameters; @@ -43,15 +42,9 @@ import java.util.Map; public class TomcatInvokeInterceptor implements InstanceMethodsAroundInterceptor { - private static boolean IS_SERVLET_GET_STATUS_METHOD_EXIST; private static final String SERVLET_RESPONSE_CLASS = "jakarta.servlet.http.HttpServletResponse"; private static final String GET_STATUS_METHOD = "getStatus"; - static { - IS_SERVLET_GET_STATUS_METHOD_EXIST = MethodUtil.isMethodExist( - TomcatInvokeInterceptor.class.getClassLoader(), SERVLET_RESPONSE_CLASS, GET_STATUS_METHOD); - } - @Override public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, MethodInterceptResult result) throws Throwable { @@ -82,9 +75,9 @@ public class TomcatInvokeInterceptor implements InstanceMethodsAroundInterceptor HttpServletResponse response = (HttpServletResponse) allArguments[1]; AbstractSpan span = ContextManager.activeSpan(); - if (IS_SERVLET_GET_STATUS_METHOD_EXIST && response.getStatus() >= 400) { + Tags.HTTP_RESPONSE_STATUS_CODE.set(span, response.getStatus()); + if (response.getStatus() >= 400) { span.errorOccurred(); - Tags.HTTP_RESPONSE_STATUS_CODE.set(span, response.getStatus()); } // Active HTTP parameter collection automatically in the profiling context. if (!TomcatPluginConfig.Plugin.Tomcat.COLLECT_HTTP_PARAMS && span.isProfiling()) { diff --git a/test/plugin/scenarios/spring-6.x-scenario/config/expectedData.yaml b/test/plugin/scenarios/spring-6.x-scenario/config/expectedData.yaml index 15d6e7326..926a3f2b4 100644 --- a/test/plugin/scenarios/spring-6.x-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/spring-6.x-scenario/config/expectedData.yaml @@ -104,7 +104,7 @@ segmentItems: tags: - {key: url, value: 'http://localhost:8080/spring-6.x-scenario/create/'} - {key: http.method, value: POST} - - {key: http.status_code, value: '200'} + - {key: http.status_code, value: '201'} refs: - {parentEndpoint: GET:/case/resttemplate, networkAddress: 'localhost:8080', refType: CrossProcess, parentSpanId: 2, parentTraceSegmentId: not null, parentServiceInstance: not @@ -167,7 +167,7 @@ segmentItems: tags: - {key: url, value: 'http://localhost:8080/spring-6.x-scenario/delete/1'} - {key: http.method, value: DELETE} - - {key: http.status_code, value: '200'} + - {key: http.status_code, value: '204'} refs: - {parentEndpoint: GET:/case/resttemplate, networkAddress: 'localhost:8080', refType: CrossProcess, parentSpanId: 5, parentTraceSegmentId: not null, parentServiceInstance: not diff --git a/test/plugin/scenarios/tomcat-10x-scenario/config/expectedData.yaml b/test/plugin/scenarios/tomcat-10x-scenario/config/expectedData.yaml index b8fa2322f..dc03590cd 100644 --- a/test/plugin/scenarios/tomcat-10x-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/tomcat-10x-scenario/config/expectedData.yaml @@ -34,6 +34,7 @@ segmentItems: tags: - {key: url, value: 'http://localhost:8080/tomcat-10x-scenario/case/tomcat-10x-scenario'} - {key: http.method, value: POST} + - {key: http.status_code, value: "200"} refs: - {parentEndpoint: 'GET:/tomcat-10x-scenario/case/tomcat-10x-scenario', networkAddress: 'localhost:8080', refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null, @@ -72,3 +73,4 @@ segmentItems: tags: - {key: url, value: 'http://localhost:8080/tomcat-10x-scenario/case/tomcat-10x-scenario'} - {key: http.method, value: GET} + - {key: http.status_code, value: "200"} diff --git a/test/plugin/scenarios/tomcat-9x-scenario/config/expectedData.yaml b/test/plugin/scenarios/tomcat-9x-scenario/config/expectedData.yaml new file mode 100644 index 000000000..10bd8bee3 --- /dev/null +++ b/test/plugin/scenarios/tomcat-9x-scenario/config/expectedData.yaml @@ -0,0 +1,76 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +segmentItems: +- serviceName: tomcat-9x-scenario + segmentSize: ge 2 + segments: + - segmentId: not null + spans: + - operationName: POST:/tomcat-9x-scenario/case/tomcat-9x-scenario + operationId: 0 + parentSpanId: -1 + spanId: 0 + spanLayer: Http + startTime: nq 0 + endTime: nq 0 + componentId: 1 + isError: false + spanType: Entry + peer: '' + skipAnalysis: false + tags: + - {key: url, value: 'http://localhost:8080/tomcat-9x-scenario/case/tomcat-9x-scenario'} + - {key: http.method, value: POST} + - {key: http.status_code, value: "200"} + refs: + - {parentEndpoint: 'GET:/tomcat-9x-scenario/case/tomcat-9x-scenario', networkAddress: 'localhost:8080', + refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null, + parentServiceInstance: not null, parentService: tomcat-9x-scenario, + traceId: not null} + - segmentId: not null + spans: + - operationName: /tomcat-9x-scenario/case/tomcat-9x-scenario + operationId: 0 + parentSpanId: 0 + spanId: 1 + spanLayer: Http + startTime: nq 0 + endTime: nq 0 + componentId: 2 + isError: false + spanType: Exit + peer: localhost:8080 + skipAnalysis: false + tags: + - {key: url, value: 'http://localhost:8080/tomcat-9x-scenario/case/tomcat-9x-scenario'} + - {key: http.method, value: POST} + - {key: http.status_code, value: "200"} + - operationName: GET:/tomcat-9x-scenario/case/tomcat-9x-scenario + operationId: 0 + parentSpanId: -1 + spanId: 0 + spanLayer: Http + startTime: nq 0 + endTime: nq 0 + componentId: 1 + isError: false + spanType: Entry + peer: '' + skipAnalysis: false + tags: + - {key: url, value: 'http://localhost:8080/tomcat-9x-scenario/case/tomcat-9x-scenario'} + - {key: http.method, value: GET} + - {key: http.status_code, value: "200"} diff --git a/test/plugin/scenarios/tomcat-9x-scenario/configuration.yml b/test/plugin/scenarios/tomcat-9x-scenario/configuration.yml new file mode 100644 index 000000000..523a81748 --- /dev/null +++ b/test/plugin/scenarios/tomcat-9x-scenario/configuration.yml @@ -0,0 +1,19 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +type: tomcat +entryService: http://localhost:8080/tomcat-9x-scenario/case/tomcat-9x-scenario +healthCheck: http://localhost:8080/tomcat-9x-scenario/case/healthCheck diff --git a/test/plugin/scenarios/tomcat-9x-scenario/pom.xml b/test/plugin/scenarios/tomcat-9x-scenario/pom.xml new file mode 100644 index 000000000..308cb5cd1 --- /dev/null +++ b/test/plugin/scenarios/tomcat-9x-scenario/pom.xml @@ -0,0 +1,86 @@ + + + + + org.apache.skywalking.apm.testcase + tomcat-9x-scenario + 1.0.0 + war + + 4.0.0 + + skywalking-tomcat-9x-scenario + + + UTF-8 + 1.8 + 1.0 + 1.18.20 + + + + + javax.servlet + javax.servlet-api + 4.0.1 + + + org.apache.logging.log4j + log4j-api + 2.8.1 + + + org.apache.logging.log4j + log4j-core + 2.8.1 + + + org.projectlombok + lombok + ${lombok.version} + provided + + + org.apache.httpcomponents + httpclient + 4.3 + + + + + tomcat-9x-scenario + + + org.apache.maven.plugins + maven-war-plugin + 3.3.1 + + + maven-compiler-plugin + + ${compiler.version} + ${compiler.version} + ${project.build.sourceEncoding} + + + + + diff --git a/test/plugin/scenarios/tomcat-9x-scenario/src/main/java/org/apache/skywalking/apm/testcase/tomcat9x/CaseServlet.java b/test/plugin/scenarios/tomcat-9x-scenario/src/main/java/org/apache/skywalking/apm/testcase/tomcat9x/CaseServlet.java new file mode 100644 index 000000000..9b47be863 --- /dev/null +++ b/test/plugin/scenarios/tomcat-9x-scenario/src/main/java/org/apache/skywalking/apm/testcase/tomcat9x/CaseServlet.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.apm.testcase.tomcat9x; + +import org.apache.http.HttpEntity; +import org.apache.http.client.ResponseHandler; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.util.EntityUtils; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.io.PrintWriter; + +public class CaseServlet extends HttpServlet { + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + try (CloseableHttpClient httpClient = HttpClients.createDefault()) { + HttpPost httpPost = new HttpPost("http://localhost:8080/tomcat-9x-scenario/case/tomcat-9x-scenario"); + ResponseHandler responseHandler = response -> { + HttpEntity entity = response.getEntity(); + return entity != null ? EntityUtils.toString(entity) : null; + }; + httpClient.execute(httpPost, responseHandler); + } + PrintWriter writer = resp.getWriter(); + writer.write("Success1"); + writer.flush(); + writer.close(); + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + PrintWriter writer = resp.getWriter(); + writer.write("Success2"); + writer.flush(); + writer.close(); + } + +} diff --git a/test/plugin/scenarios/tomcat-9x-scenario/src/main/java/org/apache/skywalking/apm/testcase/tomcat9x/ErrorCatchServlet.java b/test/plugin/scenarios/tomcat-9x-scenario/src/main/java/org/apache/skywalking/apm/testcase/tomcat9x/ErrorCatchServlet.java new file mode 100644 index 000000000..94967b210 --- /dev/null +++ b/test/plugin/scenarios/tomcat-9x-scenario/src/main/java/org/apache/skywalking/apm/testcase/tomcat9x/ErrorCatchServlet.java @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package org.apache.skywalking.apm.testcase.tomcat9x; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.io.PrintWriter; + +public class ErrorCatchServlet extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + PrintWriter writer = resp.getWriter(); + writer.write("404"); + writer.flush(); + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + doGet(req, resp); + } +} diff --git a/test/plugin/scenarios/tomcat-9x-scenario/src/main/java/org/apache/skywalking/apm/testcase/tomcat9x/HealthCheckServlet.java b/test/plugin/scenarios/tomcat-9x-scenario/src/main/java/org/apache/skywalking/apm/testcase/tomcat9x/HealthCheckServlet.java new file mode 100644 index 000000000..c5de6f07d --- /dev/null +++ b/test/plugin/scenarios/tomcat-9x-scenario/src/main/java/org/apache/skywalking/apm/testcase/tomcat9x/HealthCheckServlet.java @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.skywalking.apm.testcase.tomcat9x; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.io.PrintWriter; + +public class HealthCheckServlet extends HttpServlet { + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + PrintWriter writer = resp.getWriter(); + writer.write("Success"); + writer.flush(); + } + + @Override + protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + doGet(req, resp); + } +} diff --git a/test/plugin/scenarios/tomcat-9x-scenario/src/main/resources/log4j2.xml b/test/plugin/scenarios/tomcat-9x-scenario/src/main/resources/log4j2.xml new file mode 100644 index 000000000..9849ed5a8 --- /dev/null +++ b/test/plugin/scenarios/tomcat-9x-scenario/src/main/resources/log4j2.xml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + \ No newline at end of file diff --git a/test/plugin/scenarios/tomcat-9x-scenario/src/main/webapp/WEB-INF/web.xml b/test/plugin/scenarios/tomcat-9x-scenario/src/main/webapp/WEB-INF/web.xml new file mode 100644 index 000000000..3a5de52af --- /dev/null +++ b/test/plugin/scenarios/tomcat-9x-scenario/src/main/webapp/WEB-INF/web.xml @@ -0,0 +1,58 @@ + + + skywalking-tomcat-9x-scenario + + + caseServlet + org.apache.skywalking.apm.testcase.tomcat9x.CaseServlet + + + + healthCheck + org.apache.skywalking.apm.testcase.tomcat9x.HealthCheckServlet + + + + errorCatch + org.apache.skywalking.apm.testcase.tomcat9x.ErrorCatchServlet + + + + errorCatch + /case/errorCatch + + + + healthCheck + /case/healthCheck + + + + caseServlet + /case/tomcat-9x-scenario + + + 404 + /case/errorCatch + + diff --git a/test/plugin/scenarios/tomcat-9x-scenario/support-version.list b/test/plugin/scenarios/tomcat-9x-scenario/support-version.list new file mode 100644 index 000000000..f4b16fb8b --- /dev/null +++ b/test/plugin/scenarios/tomcat-9x-scenario/support-version.list @@ -0,0 +1,19 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Use `.github/workflows/plugins-tomcat9-test.0.yaml` to control Tomcat verion +# Current tested Tomcat 9.0.71 +all