Eliminate repeated code with HttpServletRequestWrapper in mvc-annotation-commons.

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)
 

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<String, String[]> getParameterMap();
+
+    public Enumeration<String> getHeaders(String name);
+}

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<String, String[]> getParameterMap() {
+            return jakartaRequest.getParameterMap();
+        }
+
+        @Override
+        public Enumeration<String> 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<String, String[]> getParameterMap() {
+            return javaxRequest.getParameterMap();
+        }
+
+        @Override
+        public Enumeration<String> getHeaders(String name) {
+            return javaxRequest.getHeaders(name);
+        }
+
+    }
+}

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<String, String[]> 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<String, String[]> 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<String> 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<String> headerValues = request.getHeaders(
+                            headerName);
+                    List<String> 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<String> headersList = new ArrayList<>(SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS.size());
         SpringMVCPluginConfig.Plugin.Http.INCLUDE_HTTP_HEADERS.stream()

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,17 +37,15 @@
 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;
 import org.apache.skywalking.apm.plugin.spring.mvc.commons.exception.ServletResponseNotFoundException;
 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;