wu-sheng commented on a change in pull request #5348:
URL: https://github.com/apache/skywalking/pull/5348#discussion_r473127939
##########
File path:
apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java
##########
@@ -208,4 +218,32 @@ private void collectHttpParam(HttpServletRequest request,
AbstractSpan span) {
Tags.HTTP.PARAMS.set(span, tagValue);
}
}
+
+ private boolean shouldCollectHeader() {
+ List<String> includeHeaders =
SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS;
+ return !CollectionUtil.isEmpty(includeHeaders) ;
+ }
+
+ private void collectHttpHeaders(HttpServletRequest request, AbstractSpan
span) {
+ final Enumeration<String> headerNames = request.getHeaderNames();
+ if (headerNames == null) {
+ return;
+ }
+ final List<String> headersList = new LinkedList<>();
+
SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS.stream().filter(headerName
-> request.getHeaders(headerName) != null).forEach(headerName -> {
+ Enumeration<String> headerValues = request.getHeaders(headerName);
+ List<String> valueList = Collections.list(headerValues);
+ if (!CollectionUtil.isEmpty(valueList)) {
+ String headerValue = valueList.toString();
+ headersList.add(headerName + "=" + headerValue);
+ }
+ });
+
+ if (!headersList.isEmpty()) {
+ String tagValue =
headersList.stream().collect(Collectors.joining("\n"));
+ tagValue =
SpringMVCPluginConfig.Plugin.Http.HTTP_HEADERS_LENGTH_THRESHOLD > 0 ?
Review comment:
`SpringMVCPluginConfig.Plugin.Http.HTTP_HEADERS_LENGTH_THRESHOLD > 0`,
according to the comments, if the `value < 0`, then collect all, I don't find
the codes.
##########
File path:
apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java
##########
@@ -208,4 +218,32 @@ private void collectHttpParam(HttpServletRequest request,
AbstractSpan span) {
Tags.HTTP.PARAMS.set(span, tagValue);
}
}
+
+ private boolean shouldCollectHeader() {
+ List<String> includeHeaders =
SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS;
Review comment:
Why assign again?
##########
File path:
apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java
##########
@@ -208,4 +218,32 @@ private void collectHttpParam(HttpServletRequest request,
AbstractSpan span) {
Tags.HTTP.PARAMS.set(span, tagValue);
}
}
+
+ private boolean shouldCollectHeader() {
+ List<String> includeHeaders =
SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS;
+ return !CollectionUtil.isEmpty(includeHeaders) ;
+ }
+
+ private void collectHttpHeaders(HttpServletRequest request, AbstractSpan
span) {
+ final Enumeration<String> headerNames = request.getHeaderNames();
+ if (headerNames == null) {
Review comment:
I mean check `SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS`,
because, you were trying to collect header in profiling, which has been
removed.
I think `headerNames` can't be null, right?
##########
File path: docs/en/setup/service-agent/java-agent/README.md
##########
@@ -132,6 +132,8 @@ 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.http.http_headers_length_threshold`| When `include_http_headers`
declares header names, this threshold controls the length limitation of all
header values. Note. this config item is added for the sake of performance. |
`2048` |
Review comment:
This description is not same as the comments. Please recheck.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
