Skip to content

Commit 122d006

Browse files
xzyJavaXxzyJavaX
authored
fix tomcat-10x-plugin and add test case to support tomcat7.x-8.x-9.x (#466)
1 parent 8b93e4a commit 122d006

14 files changed

Lines changed: 505 additions & 11 deletions

File tree

‎.github/workflows/plugins-tomcat9-test.0.yaml‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
name: Test
18+
19+
on:
20+
pull_request:
21+
paths:
22+
- '.github/workflows/plugins-*.yaml'
23+
- 'apm-application-toolkit/**'
24+
- 'apm-commons/**'
25+
- 'apm-protocol/**'
26+
- 'apm-sniffer/**'
27+
- 'test/plugin/**'
28+
- '**/pom.xml'
29+
- '!**.md'
30+
31+
concurrency:
32+
group: plugins-tomcat9-${{ github.event.pull_request.number || github.ref }}
33+
cancel-in-progress: true
34+
35+
jobs:
36+
build:
37+
name: Build
38+
runs-on: ubuntu-latest
39+
timeout-minutes: 30
40+
steps:
41+
- uses: actions/checkout@v2
42+
with:
43+
submodules: true
44+
- name: Build
45+
uses: ./.github/actions/build
46+
with:
47+
base_image_tomcat: tomcat:9.0.71-jdk8
48+
49+
test:
50+
needs: [ build ]
51+
name: ${{ matrix.case }}
52+
runs-on: ubuntu-latest
53+
timeout-minutes: 90
54+
strategy:
55+
matrix:
56+
case:
57+
- tomcat-9x-scenario
58+
steps:
59+
- uses: actions/checkout@v2
60+
with:
61+
submodules: true
62+
- uses: actions/setup-java@v2
63+
with:
64+
distribution: temurin
65+
java-version: 8
66+
- name: Run Plugin Test
67+
uses: ./.github/actions/run
68+
with:
69+
test_case: ${{ matrix.case }}

‎CHANGES.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ Release Notes.
1818
* Change gRPC instrumentation point to fix plugin not working for server side.
1919
* Fix servicecomb plugin trace break.
2020
* Adapt Armeria's plugins to the latest version 1.22.x
21+
* Fix tomcat-10x-plugin and add test case to support tomcat7.x-8.x-9.x.
2122

2223
#### Documentation
2324
* Update docs of Tracing APIs, reorganize the API docs into six parts.

‎apm-sniffer/apm-sdk-plugin/tomcat-10x-plugin/src/main/java/org/apache/skywalking/apm/plugin/tomcat10x/TomcatInvokeInterceptor.java‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
3232
import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
3333
import org.apache.skywalking.apm.agent.core.util.CollectionUtil;
34-
import org.apache.skywalking.apm.agent.core.util.MethodUtil;
3534
import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
3635
import org.apache.skywalking.apm.util.StringUtil;
3736
import org.apache.tomcat.util.http.Parameters;
@@ -43,15 +42,9 @@
4342

4443
public class TomcatInvokeInterceptor implements InstanceMethodsAroundInterceptor {
4544

46-
private static boolean IS_SERVLET_GET_STATUS_METHOD_EXIST;
4745
private static final String SERVLET_RESPONSE_CLASS = "jakarta.servlet.http.HttpServletResponse";
4846
private static final String GET_STATUS_METHOD = "getStatus";
4947

50-
static {
51-
IS_SERVLET_GET_STATUS_METHOD_EXIST = MethodUtil.isMethodExist(
52-
TomcatInvokeInterceptor.class.getClassLoader(), SERVLET_RESPONSE_CLASS, GET_STATUS_METHOD);
53-
}
54-
5548
@Override
5649
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
5750
MethodInterceptResult result) throws Throwable {
@@ -82,9 +75,9 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA
8275
HttpServletResponse response = (HttpServletResponse) allArguments[1];
8376

8477
AbstractSpan span = ContextManager.activeSpan();
85-
if (IS_SERVLET_GET_STATUS_METHOD_EXIST && response.getStatus() >= 400) {
78+
Tags.HTTP_RESPONSE_STATUS_CODE.set(span, response.getStatus());
79+
if (response.getStatus() >= 400) {
8680
span.errorOccurred();
87-
Tags.HTTP_RESPONSE_STATUS_CODE.set(span, response.getStatus());
8881
}
8982
// Active HTTP parameter collection automatically in the profiling context.
9083
if (!TomcatPluginConfig.Plugin.Tomcat.COLLECT_HTTP_PARAMS && span.isProfiling()) {

‎test/plugin/scenarios/spring-6.x-scenario/config/expectedData.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ segmentItems:
104104
tags:
105105
- {key: url, value: 'http://localhost:8080/spring-6.x-scenario/create/'}
106106
- {key: http.method, value: POST}
107-
- {key: http.status_code, value: '200'}
107+
- {key: http.status_code, value: '201'}
108108
refs:
109109
- {parentEndpoint: GET:/case/resttemplate, networkAddress: 'localhost:8080', refType: CrossProcess,
110110
parentSpanId: 2, parentTraceSegmentId: not null, parentServiceInstance: not
@@ -167,7 +167,7 @@ segmentItems:
167167
tags:
168168
- {key: url, value: 'http://localhost:8080/spring-6.x-scenario/delete/1'}
169169
- {key: http.method, value: DELETE}
170-
- {key: http.status_code, value: '200'}
170+
- {key: http.status_code, value: '204'}
171171
refs:
172172
- {parentEndpoint: GET:/case/resttemplate, networkAddress: 'localhost:8080', refType: CrossProcess,
173173
parentSpanId: 5, parentTraceSegmentId: not null, parentServiceInstance: not

‎test/plugin/scenarios/tomcat-10x-scenario/config/expectedData.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ segmentItems:
3434
tags:
3535
- {key: url, value: 'http://localhost:8080/tomcat-10x-scenario/case/tomcat-10x-scenario'}
3636
- {key: http.method, value: POST}
37+
- {key: http.status_code, value: "200"}
3738
refs:
3839
- {parentEndpoint: 'GET:/tomcat-10x-scenario/case/tomcat-10x-scenario', networkAddress: 'localhost:8080',
3940
refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null,
@@ -72,3 +73,4 @@ segmentItems:
7273
tags:
7374
- {key: url, value: 'http://localhost:8080/tomcat-10x-scenario/case/tomcat-10x-scenario'}
7475
- {key: http.method, value: GET}
76+
- {key: http.status_code, value: "200"}

‎test/plugin/scenarios/tomcat-9x-scenario/config/expectedData.yaml‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
segmentItems:
17+
- serviceName: tomcat-9x-scenario
18+
segmentSize: ge 2
19+
segments:
20+
- segmentId: not null
21+
spans:
22+
- operationName: POST:/tomcat-9x-scenario/case/tomcat-9x-scenario
23+
operationId: 0
24+
parentSpanId: -1
25+
spanId: 0
26+
spanLayer: Http
27+
startTime: nq 0
28+
endTime: nq 0
29+
componentId: 1
30+
isError: false
31+
spanType: Entry
32+
peer: ''
33+
skipAnalysis: false
34+
tags:
35+
- {key: url, value: 'http://localhost:8080/tomcat-9x-scenario/case/tomcat-9x-scenario'}
36+
- {key: http.method, value: POST}
37+
- {key: http.status_code, value: "200"}
38+
refs:
39+
- {parentEndpoint: 'GET:/tomcat-9x-scenario/case/tomcat-9x-scenario', networkAddress: 'localhost:8080',
40+
refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null,
41+
parentServiceInstance: not null, parentService: tomcat-9x-scenario,
42+
traceId: not null}
43+
- segmentId: not null
44+
spans:
45+
- operationName: /tomcat-9x-scenario/case/tomcat-9x-scenario
46+
operationId: 0
47+
parentSpanId: 0
48+
spanId: 1
49+
spanLayer: Http
50+
startTime: nq 0
51+
endTime: nq 0
52+
componentId: 2
53+
isError: false
54+
spanType: Exit
55+
peer: localhost:8080
56+
skipAnalysis: false
57+
tags:
58+
- {key: url, value: 'http://localhost:8080/tomcat-9x-scenario/case/tomcat-9x-scenario'}
59+
- {key: http.method, value: POST}
60+
- {key: http.status_code, value: "200"}
61+
- operationName: GET:/tomcat-9x-scenario/case/tomcat-9x-scenario
62+
operationId: 0
63+
parentSpanId: -1
64+
spanId: 0
65+
spanLayer: Http
66+
startTime: nq 0
67+
endTime: nq 0
68+
componentId: 1
69+
isError: false
70+
spanType: Entry
71+
peer: ''
72+
skipAnalysis: false
73+
tags:
74+
- {key: url, value: 'http://localhost:8080/tomcat-9x-scenario/case/tomcat-9x-scenario'}
75+
- {key: http.method, value: GET}
76+
- {key: http.status_code, value: "200"}

‎test/plugin/scenarios/tomcat-9x-scenario/configuration.yml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing, software
12+
# distributed under the License is distributed on an "AS IS" BASIS,
13+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
# See the License for the specific language governing permissions and
15+
# limitations under the License.
16+
17+
type: tomcat
18+
entryService: http://localhost:8080/tomcat-9x-scenario/case/tomcat-9x-scenario
19+
healthCheck: http://localhost:8080/tomcat-9x-scenario/case/healthCheck

‎test/plugin/scenarios/tomcat-9x-scenario/pom.xml‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!--
3+
~ Licensed to the Apache Software Foundation (ASF) under one or more
4+
~ contributor license agreements. See the NOTICE file distributed with
5+
~ this work for additional information regarding copyright ownership.
6+
~ The ASF licenses this file to You under the Apache License, Version 2.0
7+
~ (the "License"); you may not use this file except in compliance with
8+
~ the License. You may obtain a copy of the License at
9+
~
10+
~ http://www.apache.org/licenses/LICENSE-2.0
11+
~
12+
~ Unless required by applicable law or agreed to in writing, software
13+
~ distributed under the License is distributed on an "AS IS" BASIS,
14+
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
~ See the License for the specific language governing permissions and
16+
~ limitations under the License.
17+
~
18+
-->
19+
<project xmlns="http://maven.apache.org/POM/4.0.0"
20+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
21+
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
22+
23+
<groupId>org.apache.skywalking.apm.testcase</groupId>
24+
<artifactId>tomcat-9x-scenario</artifactId>
25+
<version>1.0.0</version>
26+
<packaging>war</packaging>
27+
28+
<modelVersion>4.0.0</modelVersion>
29+
30+
<name>skywalking-tomcat-9x-scenario</name>
31+
32+
<properties>
33+
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
34+
<compiler.version>1.8</compiler.version>
35+
<test.framework.version>1.0</test.framework.version>
36+
<lombok.version>1.18.20</lombok.version>
37+
</properties>
38+
39+
<dependencies>
40+
<dependency>
41+
<groupId>javax.servlet</groupId>
42+
<artifactId>javax.servlet-api</artifactId>
43+
<version>4.0.1</version>
44+
</dependency>
45+
<dependency>
46+
<groupId>org.apache.logging.log4j</groupId>
47+
<artifactId>log4j-api</artifactId>
48+
<version>2.8.1</version>
49+
</dependency>
50+
<dependency>
51+
<groupId>org.apache.logging.log4j</groupId>
52+
<artifactId>log4j-core</artifactId>
53+
<version>2.8.1</version>
54+
</dependency>
55+
<dependency>
56+
<groupId>org.projectlombok</groupId>
57+
<artifactId>lombok</artifactId>
58+
<version>${lombok.version}</version>
59+
<scope>provided</scope>
60+
</dependency>
61+
<dependency>
62+
<groupId>org.apache.httpcomponents</groupId>
63+
<artifactId>httpclient</artifactId>
64+
<version>4.3</version>
65+
</dependency>
66+
</dependencies>
67+
68+
<build>
69+
<finalName>tomcat-9x-scenario</finalName>
70+
<plugins>
71+
<plugin>
72+
<groupId>org.apache.maven.plugins</groupId>
73+
<artifactId>maven-war-plugin</artifactId>
74+
<version>3.3.1</version>
75+
</plugin>
76+
<plugin>
77+
<artifactId>maven-compiler-plugin</artifactId>
78+
<configuration>
79+
<source>${compiler.version}</source>
80+
<target>${compiler.version}</target>
81+
<encoding>${project.build.sourceEncoding}</encoding>
82+
</configuration>
83+
</plugin>
84+
</plugins>
85+
</build>
86+
</project>

‎test/plugin/scenarios/tomcat-9x-scenario/src/main/java/org/apache/skywalking/apm/testcase/tomcat9x/CaseServlet.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
package org.apache.skywalking.apm.testcase.tomcat9x;
20+
21+
import org.apache.http.HttpEntity;
22+
import org.apache.http.client.ResponseHandler;
23+
import org.apache.http.client.methods.HttpPost;
24+
import org.apache.http.impl.client.CloseableHttpClient;
25+
import org.apache.http.impl.client.HttpClients;
26+
import org.apache.http.util.EntityUtils;
27+
28+
import javax.servlet.ServletException;
29+
import javax.servlet.http.HttpServlet;
30+
import javax.servlet.http.HttpServletRequest;
31+
import javax.servlet.http.HttpServletResponse;
32+
import java.io.IOException;
33+
import java.io.PrintWriter;
34+
35+
public class CaseServlet extends HttpServlet {
36+
37+
@Override
38+
protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
39+
try (CloseableHttpClient httpClient = HttpClients.createDefault()) {
40+
HttpPost httpPost = new HttpPost("http://localhost:8080/tomcat-9x-scenario/case/tomcat-9x-scenario");
41+
ResponseHandler<String> responseHandler = response -> {
42+
HttpEntity entity = response.getEntity();
43+
return entity != null ? EntityUtils.toString(entity) : null;
44+
};
45+
httpClient.execute(httpPost, responseHandler);
46+
}
47+
PrintWriter writer = resp.getWriter();
48+
writer.write("Success1");
49+
writer.flush();
50+
writer.close();
51+
}
52+
53+
@Override
54+
protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
55+
PrintWriter writer = resp.getWriter();
56+
writer.write("Success2");
57+
writer.flush();
58+
writer.close();
59+
}
60+
61+
}

0 commit comments

Comments
 (0)