From dd6f439df1c9d722dccb45eae4e040609a65ca12 Mon Sep 17 00:00:00 2001 From: zshit Date: Thu, 6 Aug 2020 13:09:31 +0800 Subject: [PATCH] support collecting feign params (#5235) --- .../apm/agent/core/context/tag/Tags.java | 2 + .../http/v9/DefaultHttpClientInterceptor.java | 32 +++++++++++++ .../feign/http/v9/FeignPluginConfig.java | 45 +++++++++++++++++++ .../setup/service-agent/java-agent/README.md | 3 ++ .../scenarios/feign-scenario/bin/startup.sh | 4 +- .../feign-scenario/config/expectedData.yaml | 43 +++++++++++++++++- .../feign/controller/CaseController.java | 1 + .../feign/controller/RestController.java | 8 ++++ .../feign/controller/RestRequest.java | 5 +++ 9 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/FeignPluginConfig.java diff --git a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/tag/Tags.java b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/tag/Tags.java index 7ddb592c3..57952778a 100644 --- a/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/tag/Tags.java +++ b/apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/tag/Tags.java @@ -81,6 +81,8 @@ public final class Tags { public static final StringTag METHOD = new StringTag(10, "http.method"); public static final StringTag PARAMS = new StringTag(11, "http.params", true); + + public static final StringTag BODY = new StringTag(13, "http.body"); } public static final StringTag LOGIC_ENDPOINT = new StringTag(12, "x-le"); diff --git a/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptor.java b/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptor.java index bb8788643..6bd87acf8 100644 --- a/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptor.java @@ -20,6 +20,7 @@ package org.apache.skywalking.apm.plugin.feign.http.v9; import feign.Request; import feign.Response; +import java.util.Iterator; import org.apache.skywalking.apm.agent.core.context.CarrierItem; import org.apache.skywalking.apm.agent.core.context.ContextCarrier; import org.apache.skywalking.apm.agent.core.context.ContextManager; @@ -41,12 +42,17 @@ import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; +import org.apache.skywalking.apm.util.StringUtil; + +import static feign.Util.valuesOrEmpty; /** * {@link DefaultHttpClientInterceptor} intercept the default implementation of http calls by the Feign. */ public class DefaultHttpClientInterceptor implements InstanceMethodsAroundInterceptor { + private static final String CONTENT_TYPE_HEADER = "Content-Type"; + /** * Get the {@link feign.Request} from {@link EnhancedInstance}, then create {@link AbstractSpan} and set host, port, * kind, component, url from {@link feign.Request}. Through the reflection of the way, set the http header of @@ -82,6 +88,21 @@ public class DefaultHttpClientInterceptor implements InstanceMethodsAroundInterc Tags.URL.set(span, request.url()); SpanLayer.asHttp(span); + if (FeignPluginConfig.Plugin.Feign.COLLECT_REQUEST_BODY) { + boolean needCollectHttpBody = false; + Iterator stringIterator = valuesOrEmpty(request.headers(), CONTENT_TYPE_HEADER).iterator(); + String contentTypeHeaderValue = stringIterator.hasNext() ? stringIterator.next() : ""; + for (String contentType : FeignPluginConfig.Plugin.Feign.SUPPORTED_CONTENT_TYPES_PREFIX.split(",")) { + if (contentTypeHeaderValue.startsWith(contentType)) { + needCollectHttpBody = true; + break; + } + } + if (needCollectHttpBody) { + collectHttpBody(request, span); + } + } + Field headersField = Request.class.getDeclaredField("headers"); Field modifiersField = Field.class.getDeclaredField("modifiers"); modifiersField.setAccessible(true); @@ -101,6 +122,17 @@ public class DefaultHttpClientInterceptor implements InstanceMethodsAroundInterc headersField.set(request, Collections.unmodifiableMap(headers)); } + private void collectHttpBody(final Request request, final AbstractSpan span) { + if (request.body() == null || request.charset() == null) { + return; + } + String tagValue = new String(request.body(), request.charset()); + tagValue = FeignPluginConfig.Plugin.Feign.FILTER_LENGTH_LIMIT > 0 ? + StringUtil.cut(tagValue, FeignPluginConfig.Plugin.Feign.FILTER_LENGTH_LIMIT) : tagValue; + + Tags.HTTP.BODY.set(span, tagValue); + } + /** * Get the status code from {@link Response}, when status code greater than 400, it means there was some errors in * the server. Finish the {@link AbstractSpan}. diff --git a/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/FeignPluginConfig.java b/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/FeignPluginConfig.java new file mode 100644 index 000000000..e728f76fe --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/FeignPluginConfig.java @@ -0,0 +1,45 @@ +/* + * 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.plugin.feign.http.v9; + +import org.apache.skywalking.apm.agent.core.boot.PluginConfig; + +public class FeignPluginConfig { + public static class Plugin { + @PluginConfig(root = FeignPluginConfig.class) + public static class Feign { + /** + * This config item controls that whether the Feign plugin should collect the http body of the request. + */ + public static boolean COLLECT_REQUEST_BODY = false; + + /** + * When either {@link Plugin.Feign#COLLECT_REQUEST_BODY} is enabled, how many characters to keep and send to the OAP + * backend, use negative values to keep and send the complete body. + */ + public static int FILTER_LENGTH_LIMIT = 1024; + + /** + * When either {@link Plugin.Feign#COLLECT_REQUEST_BODY} is enabled and content-type start with SUPPORTED_CONTENT_TYPES_PREFIX, collect the body of the request + */ + public static String SUPPORTED_CONTENT_TYPES_PREFIX = "application/json,text/"; + } + } +} + diff --git a/docs/en/setup/service-agent/java-agent/README.md b/docs/en/setup/service-agent/java-agent/README.md index ba122ccc1..3a1783d2c 100755 --- a/docs/en/setup/service-agent/java-agent/README.md +++ b/docs/en/setup/service-agent/java-agent/README.md @@ -131,6 +131,9 @@ property key | Description | Default | `plugin.tomcat.collect_http_params`| This config item controls that whether the Tomcat plugin should collect the parameters of the request. Also, activate implicitly in the profiled trace. | `false` | `plugin.springmvc.collect_http_params`| This config item controls that whether the SpringMVC plugin should collect the parameters of the request, when your Spring application is based on Tomcat, consider only setting either `plugin.tomcat.collect_http_params` or `plugin.springmvc.collect_http_params`. Also, activate implicitly in the profiled trace. | `false` | `plugin.http.http_params_length_threshold`| When `COLLECT_HTTP_PARAMS` is enabled, how many characters to keep and send to the OAP backend, use negative values to keep and send the complete parameters, NB. this config item is added for the sake of performance. | `1024` | +`plugin.feign.collect_request_body`| This config item controls that whether the Feign plugin should collect the http body of the request. | `false` | +`plugin.feign.filter_length_limit`| When `COLLECT_REQUEST_BODY` is enabled, how many characters to keep and send to the OAP backend, use negative values to keep and send the complete body. | `1024` | +`plugin.feign.supported_content_types_prefix`| When `COLLECT_REQUEST_BODY` is enabled and content-type start with SUPPORTED_CONTENT_TYPES_PREFIX, collect the body of the request , multiple paths should be separated by `,` | `application/json,text/` | `plugin.influxdb.trace_influxql`|If true, trace all the influxql(query and write) in InfluxDB access, default is true.|`true`| `correlation.element_max_number`|Max element count of the correlation context.|`3`| `correlation.value_max_length`|Max value length of correlation context element.|`128`| diff --git a/test/plugin/scenarios/feign-scenario/bin/startup.sh b/test/plugin/scenarios/feign-scenario/bin/startup.sh index 671bec9f3..6ae5fbb22 100644 --- a/test/plugin/scenarios/feign-scenario/bin/startup.sh +++ b/test/plugin/scenarios/feign-scenario/bin/startup.sh @@ -17,5 +17,5 @@ # limitations under the License. home="$(cd "$(dirname $0)"; pwd)" - -java -jar ${agent_opts} ${home}/../libs/feign-scenario.jar & \ No newline at end of file +java -jar ${agent_opts} -Dskywalking.plugin.feign.collect_request_body=true \ +-Dskywalking.plugin.feign.supported_content_types_prefix='application/json,text/' ${home}/../libs/feign-scenario.jar & \ No newline at end of file diff --git a/test/plugin/scenarios/feign-scenario/config/expectedData.yaml b/test/plugin/scenarios/feign-scenario/config/expectedData.yaml index 6015330a2..d56703b55 100644 --- a/test/plugin/scenarios/feign-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/feign-scenario/config/expectedData.yaml @@ -80,6 +80,27 @@ segmentItems: refType: CrossProcess, parentSpanId: 3, parentTraceSegmentId: not null, parentServiceInstance: not null, parentService: feign-scenario, traceId: not null} skipAnalysis: 'false' + - segmentId: not null + spans: + - operationName: /feign-scenario/modify/1 + operationId: 0 + parentSpanId: -1 + spanId: 0 + spanLayer: Http + startTime: nq 0 + endTime: nq 0 + componentId: 1 + isError: false + spanType: Entry + peer: '' + tags: + - {key: url, value: 'http://localhost:8080/feign-scenario/modify/1'} + - {key: http.method, value: PUT} + refs: + - {parentEndpoint: /feign-scenario/case/feign-scenario, networkAddress: 'localhost:8080', + refType: CrossProcess, parentSpanId: 4, parentTraceSegmentId: not null, parentServiceInstance: not + null, parentService: feign-scenario, traceId: not null} + skipAnalysis: 'false' - segmentId: not null spans: - operationName: /feign-scenario/delete/1 @@ -98,7 +119,7 @@ segmentItems: - {key: http.method, value: DELETE} refs: - {parentEndpoint: /feign-scenario/case/feign-scenario, networkAddress: 'localhost:8080', - refType: CrossProcess, parentSpanId: 4, parentTraceSegmentId: not null, parentServiceInstance: not + refType: CrossProcess, parentSpanId: 5, parentTraceSegmentId: not null, parentServiceInstance: not null, parentService: feign-scenario, traceId: not null} skipAnalysis: 'false' - segmentId: not null @@ -117,6 +138,7 @@ segmentItems: tags: - {key: http.method, value: POST} - {key: url, value: 'http://localhost:8080/feign-scenario/create/'} + - {key: http.body, value: '{"id": "1", "userName": "test"}'} skipAnalysis: 'false' - operationName: /feign-scenario/get/{id} operationId: 0 @@ -147,11 +169,28 @@ segmentItems: tags: - {key: http.method, value: PUT} - {key: url, value: 'http://localhost:8080/feign-scenario/update/1'} + - {key: http.body, value: '{"id": "1", "userName": "testA"}'} + skipAnalysis: 'false' + - operationName: /feign-scenario/modify/{id} + operationId: 0 + parentSpanId: 0 + spanId: 4 + spanLayer: Http + startTime: nq 0 + endTime: nq 0 + componentId: 11 + isError: false + spanType: Exit + peer: localhost:8080 + tags: + - {key: http.method, value: PUT} + - {key: url, value: 'http://localhost:8080/feign-scenario/modify/1'} + - {key: http.body, value: testA} skipAnalysis: 'false' - operationName: /feign-scenario/delete/{id} operationId: 0 parentSpanId: 0 - spanId: 4 + spanId: 5 spanLayer: Http startTime: nq 0 endTime: nq 0 diff --git a/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/CaseController.java b/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/CaseController.java index fe30ff962..82d2b37b3 100644 --- a/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/CaseController.java +++ b/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/CaseController.java @@ -45,6 +45,7 @@ public class CaseController { User user = request.getById(1); logger.info("find Id{} user. User name is {} ", user.getId(), user.getUserName()); request.updateUser(1, "testA"); + request.modifyUser(1, "testA"); request.deleteUser(1); return "success"; } diff --git a/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/RestController.java b/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/RestController.java index f3bdcb060..2165e1173 100644 --- a/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/RestController.java +++ b/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/RestController.java @@ -64,6 +64,14 @@ public class RestController { return ResponseEntity.ok(currentUser); } + @PutMapping(value = "/modify/{id}") + @ResponseBody + public ResponseEntity modify(@PathVariable("id") int id, + @RequestBody String userName) throws InterruptedException { + User currentUser = new User(id, userName); + return ResponseEntity.ok(currentUser); + } + @DeleteMapping(value = "/delete/{id}") @ResponseBody public ResponseEntity deleteUser(@PathVariable("id") int id) throws InterruptedException { diff --git a/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/RestRequest.java b/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/RestRequest.java index 04596f778..30fe0a19c 100644 --- a/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/RestRequest.java +++ b/test/plugin/scenarios/feign-scenario/src/main/java/org/apache/skywalking/apm/testcase/feign/controller/RestRequest.java @@ -43,6 +43,11 @@ public interface RestRequest { @Body("%7B\"id\": \"{id}\", \"userName\": \"{userName}\"%7D") User updateUser(@Param("id") int id, @Param("userName") String userName); + @RequestLine("PUT /modify/{id}") + @Headers("Content-Type: text/plain") + @Body("{userName}") + User modifyUser(@Param("id") int id, @Param("userName") String userName); + @RequestLine("DELETE /delete/{id}") void deleteUser(@Param("id") int id);