diff --git a/CHANGES.md b/CHANGES.md index 7d9f030558..bd24e54cd1 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -17,6 +17,7 @@ Release Notes. * Fix OOM due to too many span logs. * Fix ClassLoader cache OOM issue with WeakHashMap. * Fix Jetty client cannot receive the HTTP response body. +* Eliminate repeated code with HttpServletRequestWrapper in mvc-annotation-commons. All issues and pull requests are [here](https://github.com/apache/skywalking/milestone/242?closed=1) diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/HttpServletRequestWrapper.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/HttpServletRequestWrapper.java new file mode 100644 index 0000000000..d258f7a5f2 --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/HttpServletRequestWrapper.java @@ -0,0 +1,37 @@ +/* + * 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.spring.mvc.commons; + +import java.util.Enumeration; +import java.util.Map; + +public interface HttpServletRequestWrapper { + + String getHeader(String name); + + String getMethod(); + + StringBuffer getRequestURL(); + + String getRemoteHost(); + + Map getParameterMap(); + + public Enumeration getHeaders(String name); +} diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/HttpServletRequestWrappers.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/HttpServletRequestWrappers.java new file mode 100644 index 0000000000..663ad53c31 --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/HttpServletRequestWrappers.java @@ -0,0 +1,112 @@ +/* + * 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.spring.mvc.commons; + +import java.util.Enumeration; +import java.util.Map; + +public class HttpServletRequestWrappers { + + public static HttpServletRequestWrapper wrap(jakarta.servlet.http.HttpServletRequest request) { + return new JakartaHttpServletRequest(request); + } + + public static HttpServletRequestWrapper wrap(javax.servlet.http.HttpServletRequest request) { + return new JavaxHttpServletRequest(request); + } + + public static class JakartaHttpServletRequest implements HttpServletRequestWrapper { + + private jakarta.servlet.http.HttpServletRequest jakartaRequest; + + public JakartaHttpServletRequest(jakarta.servlet.http.HttpServletRequest jakartaRequest) { + this.jakartaRequest = jakartaRequest; + } + + @Override + public String getHeader(String name) { + return jakartaRequest.getHeader(name); + } + + @Override + public String getMethod() { + return jakartaRequest.getMethod(); + } + + @Override + public StringBuffer getRequestURL() { + return jakartaRequest.getRequestURL(); + } + + @Override + public String getRemoteHost() { + return jakartaRequest.getRemoteHost(); + } + + @Override + public Map getParameterMap() { + return jakartaRequest.getParameterMap(); + } + + @Override + public Enumeration getHeaders(String name) { + return jakartaRequest.getHeaders(name); + } + + } + + public static class JavaxHttpServletRequest implements HttpServletRequestWrapper { + private javax.servlet.http.HttpServletRequest javaxRequest; + + public JavaxHttpServletRequest(javax.servlet.http.HttpServletRequest javaxRequest) { + this.javaxRequest = javaxRequest; + } + + @Override + public String getHeader(String name) { + return javaxRequest.getHeader(name); + } + + @Override + public String getMethod() { + return javaxRequest.getMethod(); + } + + @Override + public StringBuffer getRequestURL() { + return javaxRequest.getRequestURL(); + } + + @Override + public String getRemoteHost() { + return javaxRequest.getRemoteHost(); + } + + @Override + public Map getParameterMap() { + return javaxRequest.getParameterMap(); + } + + @Override + public Enumeration getHeaders(String name) { + return javaxRequest.getHeaders(name); + } + + } +} diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/RequestUtil.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/RequestUtil.java index b278c8f9e2..f4f23b1c5b 100644 --- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/RequestUtil.java +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/RequestUtil.java @@ -52,6 +52,16 @@ public static void collectHttpParam(jakarta.servlet.http.HttpServletRequest requ } } + public static void collectHttpParam(HttpServletRequestWrapper request, AbstractSpan span) { + final Map parameterMap = request.getParameterMap(); + if (parameterMap != null && !parameterMap.isEmpty()) { + String tagValue = CollectionUtil.toString(parameterMap); + tagValue = SpringMVCPluginConfig.Plugin.Http.HTTP_PARAMS_LENGTH_THRESHOLD > 0 ? + StringUtil.cut(tagValue, SpringMVCPluginConfig.Plugin.Http.HTTP_PARAMS_LENGTH_THRESHOLD) : tagValue; + Tags.HTTP.PARAMS.set(span, tagValue); + } + } + public static void collectHttpParam(ServerHttpRequest request, AbstractSpan span) { Map parameterMap = new HashMap<>(request.getQueryParams().size()); request.getQueryParams().forEach((key, value) -> { @@ -65,6 +75,25 @@ public static void collectHttpParam(ServerHttpRequest request, AbstractSpan span } } + public static void collectHttpHeaders(HttpServletRequestWrapper request, AbstractSpan span) { + final List headersList = new ArrayList<>(SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS.size()); + SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS.stream() + .filter( + headerName -> request.getHeaders(headerName) != null) + .forEach(headerName -> { + Enumeration headerValues = request.getHeaders( + headerName); + List valueList = Collections.list( + headerValues); + if (!CollectionUtil.isEmpty(valueList)) { + String headerValue = valueList.toString(); + headersList.add(headerName + "=" + headerValue); + } + }); + + collectHttpHeaders(headersList, span); + } + public static void collectHttpHeaders(HttpServletRequest request, AbstractSpan span) { final List headersList = new ArrayList<>(SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS.size()); SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS.stream() diff --git a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java index df3aa7583f..ac51e60b93 100644 --- a/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java @@ -18,6 +18,9 @@ package org.apache.skywalking.apm.plugin.spring.mvc.commons.interceptor; +import java.lang.reflect.Method; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; 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; @@ -34,6 +37,8 @@ import org.apache.skywalking.apm.agent.core.util.MethodUtil; import org.apache.skywalking.apm.network.trace.component.ComponentsDefine; import org.apache.skywalking.apm.plugin.spring.mvc.commons.EnhanceRequireObjectCache; +import org.apache.skywalking.apm.plugin.spring.mvc.commons.HttpServletRequestWrapper; +import org.apache.skywalking.apm.plugin.spring.mvc.commons.HttpServletRequestWrappers; import org.apache.skywalking.apm.plugin.spring.mvc.commons.RequestUtil; import org.apache.skywalking.apm.plugin.spring.mvc.commons.SpringMVCPluginConfig; import org.apache.skywalking.apm.plugin.spring.mvc.commons.exception.IllegalMethodStackDepthException; @@ -41,10 +46,6 @@ import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import java.lang.reflect.Method; - import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.CONTROLLER_METHOD_STACK_DEPTH; import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.FORWARD_REQUEST_FLAG; import static org.apache.skywalking.apm.plugin.spring.mvc.commons.Constants.REACTIVE_ASYNC_SPAN_IN_RUNTIME_CONTEXT; @@ -68,10 +69,11 @@ public abstract class AbstractMethodInterceptor implements InstanceMethodsAround static { IS_SERVLET_GET_STATUS_METHOD_EXIST = MethodUtil.isMethodExist( - AbstractMethodInterceptor.class.getClassLoader(), SERVLET_RESPONSE_CLASS, GET_STATUS_METHOD); + AbstractMethodInterceptor.class.getClassLoader(), SERVLET_RESPONSE_CLASS, GET_STATUS_METHOD); IS_JAKARTA_SERVLET_GET_STATUS_METHOD_EXIST = MethodUtil.isMethodExist( - AbstractMethodInterceptor.class.getClassLoader(), - JAKARTA_SERVLET_RESPONSE_CLASS, GET_STATUS_METHOD); + AbstractMethodInterceptor.class.getClassLoader(), + JAKARTA_SERVLET_RESPONSE_CLASS, GET_STATUS_METHOD + ); try { Class.forName(SERVLET_RESPONSE_CLASS, true, AbstractMethodInterceptor.class.getClassLoader()); IN_SERVLET_CONTAINER = true; @@ -113,53 +115,15 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr if (IN_SERVLET_CONTAINER && IS_JAVAX && HttpServletRequest.class.isAssignableFrom(request.getClass())) { final HttpServletRequest httpServletRequest = (HttpServletRequest) request; - CarrierItem next = contextCarrier.items(); - while (next.hasNext()) { - next = next.next(); - next.setHeadValue(httpServletRequest.getHeader(next.getHeadKey())); - } - - String operationName = this.buildOperationName(method, httpServletRequest.getMethod(), - (EnhanceRequireObjectCache) objInst.getSkyWalkingDynamicField()); - AbstractSpan span = ContextManager.createEntrySpan(operationName, contextCarrier); - Tags.URL.set(span, httpServletRequest.getRequestURL().toString()); - Tags.HTTP.METHOD.set(span, httpServletRequest.getMethod()); - span.setComponent(ComponentsDefine.SPRING_MVC_ANNOTATION); - SpanLayer.asHttp(span); + handleBeforeMethod( + objInst, method, HttpServletRequestWrappers.wrap(httpServletRequest), contextCarrier); - if (SpringMVCPluginConfig.Plugin.SpringMVC.COLLECT_HTTP_PARAMS) { - RequestUtil.collectHttpParam(httpServletRequest, span); - } - - if (!CollectionUtil.isEmpty(SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS)) { - RequestUtil.collectHttpHeaders(httpServletRequest, span); - } - } else if (IN_SERVLET_CONTAINER && IS_JAKARTA && jakarta.servlet.http.HttpServletRequest.class.isAssignableFrom(request.getClass())) { + } else if (IN_SERVLET_CONTAINER && IS_JAKARTA && jakarta.servlet.http.HttpServletRequest.class.isAssignableFrom( + request.getClass())) { final jakarta.servlet.http.HttpServletRequest httpServletRequest = (jakarta.servlet.http.HttpServletRequest) request; - CarrierItem next = contextCarrier.items(); - while (next.hasNext()) { - next = next.next(); - next.setHeadValue(httpServletRequest.getHeader(next.getHeadKey())); - } - - String operationName = - this.buildOperationName(method, httpServletRequest.getMethod(), - (EnhanceRequireObjectCache) objInst.getSkyWalkingDynamicField()); - AbstractSpan span = - ContextManager.createEntrySpan(operationName, contextCarrier); - Tags.URL.set(span, httpServletRequest.getRequestURL().toString()); - Tags.HTTP.METHOD.set(span, httpServletRequest.getMethod()); - span.setComponent(ComponentsDefine.SPRING_MVC_ANNOTATION); - SpanLayer.asHttp(span); - - if (SpringMVCPluginConfig.Plugin.SpringMVC.COLLECT_HTTP_PARAMS) { - RequestUtil.collectHttpParam(httpServletRequest, span); - } + handleBeforeMethod( + objInst, method, HttpServletRequestWrappers.wrap(httpServletRequest), contextCarrier); - if (!CollectionUtil - .isEmpty(SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS)) { - RequestUtil.collectHttpHeaders(httpServletRequest, span); - } } else if (ServerHttpRequest.class.isAssignableFrom(request.getClass())) { final ServerHttpRequest serverHttpRequest = (ServerHttpRequest) request; CarrierItem next = contextCarrier.items(); @@ -168,8 +132,10 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr next.setHeadValue(serverHttpRequest.getHeaders().getFirst(next.getHeadKey())); } - String operationName = this.buildOperationName(method, serverHttpRequest.getMethod().name(), - (EnhanceRequireObjectCache) objInst.getSkyWalkingDynamicField()); + String operationName = this.buildOperationName( + method, serverHttpRequest.getMethod().name(), + (EnhanceRequireObjectCache) objInst.getSkyWalkingDynamicField() + ); AbstractSpan span = ContextManager.createEntrySpan(operationName, contextCarrier); Tags.URL.set(span, serverHttpRequest.getURI().toString()); Tags.HTTP.METHOD.set(span, serverHttpRequest.getMethod().name()); @@ -198,6 +164,31 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr } } + private void handleBeforeMethod(EnhancedInstance objInst, Method method, + HttpServletRequestWrapper httpServletRequest, ContextCarrier contextCarrier) { + CarrierItem next = contextCarrier.items(); + while (next.hasNext()) { + next = next.next(); + next.setHeadValue(httpServletRequest.getHeader(next.getHeadKey())); + } + + String operationName = this.buildOperationName( + method, httpServletRequest.getMethod(), (EnhanceRequireObjectCache) objInst.getSkyWalkingDynamicField()); + AbstractSpan span = ContextManager.createEntrySpan(operationName, contextCarrier); + Tags.URL.set(span, httpServletRequest.getRequestURL().toString()); + Tags.HTTP.METHOD.set(span, httpServletRequest.getMethod()); + span.setComponent(ComponentsDefine.SPRING_MVC_ANNOTATION); + SpanLayer.asHttp(span); + + if (SpringMVCPluginConfig.Plugin.SpringMVC.COLLECT_HTTP_PARAMS) { + RequestUtil.collectHttpParam(httpServletRequest, span); + } + + if (!CollectionUtil.isEmpty(SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS)) { + RequestUtil.collectHttpHeaders(httpServletRequest, span); + } + } + @Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, Object ret) throws Throwable { @@ -232,9 +223,11 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA Integer statusCode = null; - if (IS_SERVLET_GET_STATUS_METHOD_EXIST && HttpServletResponse.class.isAssignableFrom(response.getClass())) { + if (IS_SERVLET_GET_STATUS_METHOD_EXIST && HttpServletResponse.class.isAssignableFrom( + response.getClass())) { statusCode = ((HttpServletResponse) response).getStatus(); - } else if (IS_JAKARTA_SERVLET_GET_STATUS_METHOD_EXIST && jakarta.servlet.http.HttpServletResponse.class.isAssignableFrom(response.getClass())) { + } else if (IS_JAKARTA_SERVLET_GET_STATUS_METHOD_EXIST && jakarta.servlet.http.HttpServletResponse.class.isAssignableFrom( + response.getClass())) { statusCode = ((jakarta.servlet.http.HttpServletResponse) response).getStatus(); } else if (ServerHttpResponse.class.isAssignableFrom(response.getClass())) { if (IS_SERVLET_GET_STATUS_METHOD_EXIST || IS_JAKARTA_SERVLET_GET_STATUS_METHOD_EXIST) { @@ -263,7 +256,8 @@ public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allA if (!SpringMVCPluginConfig.Plugin.SpringMVC.COLLECT_HTTP_PARAMS && span.isProfiling()) { if (IS_JAVAX && HttpServletRequest.class.isAssignableFrom(request.getClass())) { RequestUtil.collectHttpParam((HttpServletRequest) request, span); - } else if (IS_JAKARTA && jakarta.servlet.http.HttpServletRequest.class.isAssignableFrom(request.getClass())) { + } else if (IS_JAKARTA && jakarta.servlet.http.HttpServletRequest.class.isAssignableFrom( + request.getClass())) { RequestUtil.collectHttpParam((jakarta.servlet.http.HttpServletRequest) request, span); } else if (ServerHttpRequest.class.isAssignableFrom(request.getClass())) { RequestUtil.collectHttpParam((ServerHttpRequest) request, span); @@ -309,7 +303,7 @@ private String buildOperationName(Method method, String httpMethod, EnhanceRequi pathMappingCache.addPathMapping(method, requestURL); requestURL = pathMappingCache.findPathMapping(method); } - operationName = String.join(":", httpMethod, requestURL); + operationName = String.join(":", httpMethod, requestURL); } return operationName;