Format Springmvc & Tomcat EntrySpan operation name to METHOD:URI

CHANGES.md

@@ -28,6 +28,7 @@ Release Notes.
 * Implement Kafka Log Reporter. Add config item of `agnt.conf`, `plugin.kafka.topic_logging`.
 * Upgrade byte-buddy to 1.11.18
 * Add plugin to support Apache HttpClient 5.
+* Format Springmvc & Tomcat EntrySpan operation name to `METHOD:URI`
 
 #### Documentation
 

apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/AbstractMethodInterceptor.java

@@ -73,8 +73,6 @@ public abstract class AbstractMethodInterceptor implements InstanceMethodsAround
 
     public abstract String getRequestURL(Method method);
 
-    public abstract String getAcceptedMethodTypes(Method method);
-
     @Override
     public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
                              MethodInterceptResult result) throws Throwable {
@@ -88,20 +86,6 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
             return;
         }
 
-        String operationName;
-        if (SpringMVCPluginConfig.Plugin.SpringMVC.USE_QUALIFIED_NAME_AS_ENDPOINT_NAME) {
-            operationName = MethodUtil.generateOperationName(method);
-        } else {
-            EnhanceRequireObjectCache pathMappingCache = (EnhanceRequireObjectCache) objInst.getSkyWalkingDynamicField();
-            String requestURL = pathMappingCache.findPathMapping(method);
-            if (requestURL == null) {
-                requestURL = getRequestURL(method);
-                pathMappingCache.addPathMapping(method, requestURL);
-                requestURL = pathMappingCache.findPathMapping(method);
-            }
-            operationName = getAcceptedMethodTypes(method) + requestURL;
-        }
-
         Object request = ContextManager.getRuntimeContext().get(REQUEST_KEY_IN_RUNTIME_CONTEXT);
 
         if (request != null) {
@@ -118,6 +102,8 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
                         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());
@@ -139,6 +125,8 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
                         next.setHeadValue(serverHttpRequest.getHeaders().getFirst(next.getHeadKey()));
                     }
 
+                    String operationName = this.buildOperationName(method, serverHttpRequest.getMethodValue(),
+                                                                   (EnhanceRequireObjectCache) objInst.getSkyWalkingDynamicField());
                     AbstractSpan span = ContextManager.createEntrySpan(operationName, contextCarrier);
                     Tags.URL.set(span, serverHttpRequest.getURI().toString());
                     Tags.HTTP.METHOD.set(span, serverHttpRequest.getMethodValue());
@@ -167,21 +155,6 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
         }
     }
 
-    private String buildOperationName(Object invoker, Method method) {
-        StringBuilder operationName = new StringBuilder(invoker.getClass().getName()).append(".")
-                .append(method.getName())
-                .append("(");
-        for (Class<?> type : method.getParameterTypes()) {
-            operationName.append(type.getName()).append(",");
-        }
-
-        if (method.getParameterTypes().length > 0) {
-            operationName = operationName.deleteCharAt(operationName.length() - 1);
-        }
-
-        return operationName.append(")").toString();
-    }
-
     @Override
     public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
                               Object ret) throws Throwable {
@@ -260,4 +233,36 @@ public void handleMethodException(EnhancedInstance objInst, Method method, Objec
                                       Class<?>[] argumentsTypes, Throwable t) {
         ContextManager.activeSpan().log(t);
     }
+
+    private String buildOperationName(Object invoker, Method method) {
+        StringBuilder operationName = new StringBuilder(invoker.getClass().getName()).append(".")
+                                                                                     .append(method.getName())
+                                                                                     .append("(");
+        for (Class<?> type : method.getParameterTypes()) {
+            operationName.append(type.getName()).append(",");
+        }
+
+        if (method.getParameterTypes().length > 0) {
+            operationName = operationName.deleteCharAt(operationName.length() - 1);
+        }
+
+        return operationName.append(")").toString();
+    }
+
+    private String buildOperationName(Method method, String httpMethod, EnhanceRequireObjectCache pathMappingCache) {
+        String operationName;
+        if (SpringMVCPluginConfig.Plugin.SpringMVC.USE_QUALIFIED_NAME_AS_ENDPOINT_NAME) {
+            operationName = MethodUtil.generateOperationName(method);
+        } else {
+            String requestURL = pathMappingCache.findPathMapping(method);
+            if (requestURL == null) {
+                requestURL = getRequestURL(method);
+                pathMappingCache.addPathMapping(method, requestURL);
+                requestURL = pathMappingCache.findPathMapping(method);
+            }
+            operationName =  String.join(":", httpMethod, requestURL);
+        }
+
+        return operationName;
+    }
 }

apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RequestMappingMethodInterceptor.java

@@ -40,24 +40,4 @@ public String getRequestURL(Method method) {
             return requestURL;
         });
     }
-
-    @Override
-    public String getAcceptedMethodTypes(Method method) {
-        return ParsePathUtil.recursiveParseMethodAnnotation(method, m -> {
-            RequestMapping methodRequestMapping = AnnotationUtils.getAnnotation(m, RequestMapping.class);
-            if (methodRequestMapping == null || methodRequestMapping.method().length == 0) {
-                return null;
-            }
-            StringBuilder methodTypes = new StringBuilder();
-            methodTypes.append("{");
-            for (int i = 0; i < methodRequestMapping.method().length; i++) {
-                methodTypes.append(methodRequestMapping.method()[i].toString());
-                if (methodRequestMapping.method().length > (i + 1)) {
-                    methodTypes.append(",");
-                }
-            }
-            methodTypes.append("}");
-            return methodTypes.toString();
-        });
-    }
 }

apm-sniffer/apm-sdk-plugin/spring-plugins/mvc-annotation-commons/src/main/java/org/apache/skywalking/apm/plugin/spring/mvc/commons/interceptor/RestMappingMethodInterceptor.java

@@ -77,23 +77,4 @@ public String getRequestURL(Method method) {
             return requestURL;
         });
     }
-
-    @Override
-    public String getAcceptedMethodTypes(Method method) {
-        return ParsePathUtil.recursiveParseMethodAnnotation(method, m -> {
-            if (AnnotationUtils.getAnnotation(m, GetMapping.class) != null) {
-                return "{GET}";
-            } else if (AnnotationUtils.getAnnotation(m, PostMapping.class) != null) {
-                return "{POST}";
-            } else if (AnnotationUtils.getAnnotation(m, PutMapping.class) != null) {
-                return "{PUT}";
-            } else if (AnnotationUtils.getAnnotation(m, DeleteMapping.class) != null) {
-                return "{DELETE}";
-            } else if (AnnotationUtils.getAnnotation(m, PatchMapping.class) != null) {
-                return "{PATCH}";
-            } else {
-                return null;
-            }
-        });
-    }
 }

apm-sniffer/apm-sdk-plugin/tomcat-7.x-8.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/tomcat78x/TomcatInvokeInterceptor.java

@@ -74,8 +74,8 @@ public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allAr
             next = next.next();
             next.setHeadValue(request.getHeader(next.getHeadKey()));
         }
-
-        AbstractSpan span = ContextManager.createEntrySpan(request.getRequestURI(), contextCarrier);
+        String operationName =  String.join(":", request.getMethod(), request.getRequestURI());
+        AbstractSpan span = ContextManager.createEntrySpan(operationName, contextCarrier);
         Tags.URL.set(span, request.getRequestURL().toString());
         Tags.HTTP.METHOD.set(span, request.getMethod());
         span.setComponent(ComponentsDefine.TOMCAT);

test/plugin/scenarios/activemq-scenario/config/expectedData.yaml

@@ -20,7 +20,6 @@ segmentItems:
   - segmentId: not null
     spans:
     - operationName: ActiveMQ/Queue/test/Producer
-      operationId: 0
       parentSpanId: 0
       spanId: 1
       spanLayer: MQ
@@ -34,8 +33,7 @@ segmentItems:
       - {key: mq.broker, value: not null}
       - {key: mq.queue, value: test}
       skipAnalysis: 'false'
-    - operationName: /activemq-scenario/case/activemq
-      operationId: 0
+    - operationName: GET:/activemq-scenario/case/activemq
       parentSpanId: -1
       spanId: 0
       spanLayer: Http
@@ -52,7 +50,6 @@ segmentItems:
   - segmentId: not null
     spans:
     - operationName: ActiveMQ/Queue/test/Consumer
-      operationId: 0
       parentSpanId: -1
       spanId: 0
       spanLayer: MQ
@@ -67,7 +64,7 @@ segmentItems:
       - {key: mq.queue, value: test}
       - {key: transmission.latency, value: not null}
       refs:
-      - {parentEndpoint: /activemq-scenario/case/activemq, networkAddress: not null,
+      - {parentEndpoint: GET:/activemq-scenario/case/activemq, networkAddress: not null,
         refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null,
         parentServiceInstance: not null, parentService: not null, traceId: not null}
       skipAnalysis: 'false'

test/plugin/scenarios/apm-toolkit-trace-scenario/config/expectedData.yaml

@@ -19,9 +19,17 @@ segmentItems:
   segments:
   - segmentId: not null
     spans:
-    - {operationName: tool-kit-set-operation-name, parentSpanId: 0,
-      spanId: 1, spanLayer: Unknown, startTime: nq 0, endTime: nq 0,
-      componentId: 0, isError: false, spanType: Local, peer: '', skipAnalysis: true}
+    - operationName: tool-kit-set-operation-name
+      parentSpanId: 0
+      spanId: 1
+      spanLayer: Unknown
+      startTime: nq 0
+      endTime: nq 0
+      componentId: 0
+      isError: false
+      spanType: Local
+      peer: ''
+      skipAnalysis: true
     - operationName: test.apache.skywalking.apm.testcase.toolkit.controller.TestService.testTag()
       parentSpanId: 0
       spanId: 2
@@ -145,7 +153,7 @@ segmentItems:
       - {key: p2, value: '16'}
       - {key: username, value: lisi}
       skipAnalysis: 'true'
-    - operationName: /case/tool-kit
+    - operationName: GET:/case/tool-kit
       parentSpanId: -1
       spanId: 0
       spanLayer: Http
@@ -169,7 +177,7 @@ segmentItems:
 
   - segmentId: not null
     spans:
-    - operationName: /case/asyncVisit/callable
+    - operationName: GET:/case/asyncVisit/callable
       parentSpanId: -1
       spanId: 0
       spanLayer: Http
@@ -216,7 +224,7 @@ segmentItems:
       spanType: Local
       peer: ''
       refs:
-      - {parentEndpoint: /case/tool-kit, networkAddress: '', refType: CrossThread,
+      - {parentEndpoint: GET:/case/tool-kit, networkAddress: '', refType: CrossThread,
         parentSpanId: 0, parentTraceSegmentId: not null, parentServiceInstance: not
           null, parentService: apm-toolkit-trace-scenario, traceId: not null}
       skipAnalysis: 'true'
@@ -237,7 +245,6 @@ segmentItems:
       - {key: http.method, value: GET}
       skipAnalysis: 'true'
     - operationName: Thread/org.apache.skywalking.apm.toolkit.trace.CallableWrapper/call
-
       parentSpanId: -1
       spanId: 0
       spanLayer: Unknown
@@ -248,13 +255,13 @@ segmentItems:
       spanType: Local
       peer: ''
       refs:
-      - {parentEndpoint: /case/tool-kit, networkAddress: '', refType: CrossThread,
+      - {parentEndpoint: GET:/case/tool-kit, networkAddress: '', refType: CrossThread,
         parentSpanId: 0, parentTraceSegmentId: not null, parentServiceInstance: not
           null, parentService: apm-toolkit-trace-scenario, traceId: not null}
       skipAnalysis: 'true'
   - segmentId: not null
     spans:
-    - operationName: /case/asyncVisit/runnable
+    - operationName: GET:/case/asyncVisit/runnable
       parentSpanId: -1
       spanId: 0
       spanLayer: Http
@@ -301,13 +308,13 @@ segmentItems:
       spanType: Local
       peer: ''
       refs:
-      - {parentEndpoint: /case/tool-kit, networkAddress: '', refType: CrossThread,
+      - {parentEndpoint: GET:/case/tool-kit, networkAddress: '', refType: CrossThread,
         parentSpanId: 0, parentTraceSegmentId: not null, parentServiceInstance: not
           null, parentService: apm-toolkit-trace-scenario, traceId: not null}
       skipAnalysis: 'true'
   - segmentId: not null
     spans:
-    - operationName: /case/asyncVisit/supplier
+    - operationName: GET:/case/asyncVisit/supplier
       parentSpanId: -1
       spanId: 0
       spanLayer: Http

test/plugin/scenarios/armeria-0.96minus-scenario/config/expectedData.yaml

@@ -20,7 +20,6 @@ segmentItems:
   - segmentId: not null
     spans:
     - operationName: /greet/skywalking
-      operationId: 0
       parentSpanId: -1
       spanId: 0
       spanLayer: Http
@@ -34,14 +33,13 @@ segmentItems:
       - {key: url, value: /greet/skywalking}
       - {key: http.method, value: GET}
       refs:
-      - {parentEndpoint: /greet/skywalking, networkAddress: '127.0.0.1:8085', refType: CrossProcess,
+      - {parentEndpoint: GET:/greet/skywalking, networkAddress: '127.0.0.1:8085', refType: CrossProcess,
         parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not
           null, parentService: not null, traceId: not null}
       skipAnalysis: 'false'
   - segmentId: not null
     spans:
     - operationName: /greet/skywalking
-      operationId: 0
       parentSpanId: 0
       spanId: 1
       spanLayer: Http
@@ -54,8 +52,7 @@ segmentItems:
       tags:
       - {key: http.method, value: GET}
       skipAnalysis: 'false'
-    - operationName: /greet/skywalking
-      operationId: 0
+    - operationName: GET:/greet/skywalking
       parentSpanId: -1
       spanId: 0
       spanLayer: Http

test/plugin/scenarios/armeria-0.96plus-scenario/config/expectedData.yaml

@@ -20,7 +20,6 @@ segmentItems:
   - segmentId: not null
     spans:
     - operationName: /greet/skywalking
-      operationId: 0
       parentSpanId: -1
       spanId: 0
       spanLayer: Http
@@ -34,14 +33,13 @@ segmentItems:
       - {key: url, value: /greet/skywalking}
       - {key: http.method, value: GET}
       refs:
-      - {parentEndpoint: /greet/skywalking, networkAddress: '127.0.0.1:8085', refType: CrossProcess,
+      - {parentEndpoint: GET:/greet/skywalking, networkAddress: '127.0.0.1:8085', refType: CrossProcess,
         parentSpanId: 1, parentTraceSegmentId: not null, parentServiceInstance: not
           null, parentService: not null, traceId: not null}
       skipAnalysis: 'false'
   - segmentId: not null
     spans:
     - operationName: /greet/skywalking
-      operationId: 0
       parentSpanId: 0
       spanId: 1
       spanLayer: Http
@@ -54,8 +52,7 @@ segmentItems:
       tags:
       - {key: http.method, value: GET}
       skipAnalysis: 'false'
-    - operationName: /greet/skywalking
-      operationId: 0
+    - operationName: GET:/greet/skywalking
       parentSpanId: -1
       spanId: 0
       spanLayer: Http