Increase Vert.x Compatability (#4736)

* vertx web plugin re-impl attempt

* added license

* update test for new plugin structure

* got rid of registryItems

* remove unnecessary dependencies

* re-impl eventbus scenario

* more precise naming

* fix expected data

* removed unsupported versions and removed vertx caching (avoids permission error)

* rewrote to remove copied code

* Update plugins-test.3.yaml

* fixes issue of .vertx files remaining on v3.0.0

* only present in v3.0.0

* consistency

* add more supported versions

* track status code

* spelling fix

* track status code

* more correct naming

* reverts 213f5a69 (status code isn't sent at this point)

* made HttpServerResponseImplEndInstrumentation more strict based on version
added status code tracking
fixes issue not finishing async spans

* works 3.0.0,3.7.0

* increase compatibility

* 3.9.0 default

* versioned witness classes (advice from wu-sheng)
Co-authored-by: 吴晟 Wu Sheng <wu.sheng@foxmail.com>
Co-authored-by: Nzhang-wei <pknfe@outlook.com>

.github/workflows/plugins-test.3.yaml

@@ -41,8 +41,8 @@ jobs:
           - { name: 'spring-4.1.x-scenario', title: 'Spring 4.1.x-4.2.x (20)' }
           - { name: 'spring-4.3.x-scenario', title: 'Spring 4.3.x-5.2.x (54)' }
           - { name: 'spring-async-scenario', title: 'Spring Async 4.3.x-5.1.x (35)' }
-          - { name: 'vertx-eventbus-3.x-scenario', title: 'Vert.x EventBus 3.2.x-3.7.x (19)' }
-          - { name: 'vertx-web-3.x-scenario', title: 'Vert.x Web 3.0.x-3.7.x (21)' }
+          - { name: 'vertx-eventbus-3.x-scenario', title: 'Vert.x EventBus 3.2.0-3.9.0 (27)' }
+          - { name: 'vertx-web-3.x-scenario', title: 'Vert.x Web 3.0.0-3.9.0 (29)' }
     steps:
       - uses: actions/checkout@v2
         with:

apm-sniffer/apm-agent-core/src/main/java/org/apache/skywalking/apm/agent/core/context/trace/AbstractTracingSpan.java

@@ -311,7 +311,7 @@ public abstract class AbstractTracingSpan implements AbstractSpan {
             throw new RuntimeException("Span is not in async mode, please use '#prepareForAsync' to active.");
         }
         if (isAsyncStopped) {
-            throw new RuntimeException("Can not do async finish for the span repeately.");
+            throw new RuntimeException("Can not do async finish for the span repeatedly.");
         }
         this.endTime = System.currentTimeMillis();
         owner.asyncStop(this);

apm-sniffer/apm-sdk-plugin/vertx-plugins/vertx-core-3.x-plugin/pom.xml

@@ -31,7 +31,7 @@
     <url>http://maven.apache.org</url>
 
     <properties>
-        <vertx.version>3.6.3</vertx.version>
+        <vertx.version>3.9.0</vertx.version>
     </properties>
 
     <dependencies>

apm-sniffer/apm-sdk-plugin/vertx-plugins/vertx-core-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/vertx3/HttpClientRequestImplEndInterceptor.java

@@ -33,29 +33,42 @@ import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
 
 import java.lang.reflect.Method;
 
-public class HttpClientRequestImplEndInterceptor implements InstanceMethodsAroundInterceptor, InstanceConstructorInterceptor {
+public class HttpClientRequestImplEndInterceptor implements InstanceMethodsAroundInterceptor {
 
-    @Override
-    public void onConstruct(EnhancedInstance objInst, Object[] allArguments) {
-        String host;
-        int port;
-        if (allArguments[3] instanceof Integer) {
-            host = (String) allArguments[2];
-            port = (Integer) allArguments[3];
-        } else {
-            host = (String) allArguments[3];
-            port = (Integer) allArguments[4];
+    public static class Version30XTo33XConstructorInterceptor implements InstanceConstructorInterceptor {
+        @Override
+        public void onConstruct(EnhancedInstance objInst, Object[] allArguments) {
+            String host = (String) allArguments[2];
+            int port = (Integer) allArguments[3];
+            objInst.setSkyWalkingDynamicField(host + ":" + port);
+        }
+    }
+
+    public static class Version34XTo37XConstructorInterceptor implements InstanceConstructorInterceptor {
+        @Override
+        public void onConstruct(EnhancedInstance objInst, Object[] allArguments) {
+            String host = (String) allArguments[3];
+            int port = (Integer) allArguments[4];
+            objInst.setSkyWalkingDynamicField(host + ":" + port);
+        }
+    }
+
+    public static class Version38PlusConstructorInterceptor implements InstanceConstructorInterceptor {
+        @Override
+        public void onConstruct(EnhancedInstance objInst, Object[] allArguments) {
+            String host = (String) allArguments[4];
+            int port = (Integer) allArguments[5];
+            objInst.setSkyWalkingDynamicField(host + ":" + port);
         }
-        objInst.setSkyWalkingDynamicField(host + ":" + port);
     }
 
     @Override
-    @SuppressWarnings("unchecked")
     public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
-        MethodInterceptResult result) throws Throwable {
+                             MethodInterceptResult result) {
         HttpClientRequest request = (HttpClientRequest) objInst;
         ContextCarrier contextCarrier = new ContextCarrier();
-        AbstractSpan span = ContextManager.createExitSpan(toPath(request.uri()), contextCarrier, (String) objInst.getSkyWalkingDynamicField());
+        AbstractSpan span = ContextManager.createExitSpan(toPath(request.uri()), contextCarrier,
+                (String) objInst.getSkyWalkingDynamicField());
         span.setComponent(ComponentsDefine.VERTX);
         SpanLayer.asHttp(span);
         Tags.HTTP.METHOD.set(span, request.method().toString());
@@ -71,14 +84,14 @@ public class HttpClientRequestImplEndInterceptor implements InstanceMethodsAroun
 
     @Override
     public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
-        Object ret) throws Throwable {
+                              Object ret) {
         ContextManager.stopSpan();
         return ret;
     }
 
     @Override
     public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
-        Class<?>[] argumentsTypes, Throwable t) {
+                                      Class<?>[] argumentsTypes, Throwable t) {
         ContextManager.activeSpan().errorOccurred().log(t);
     }
 

apm-sniffer/apm-sdk-plugin/vertx-plugins/vertx-core-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/vertx3/HttpClientRequestImplHandleResponseInterceptor.java

@@ -18,7 +18,9 @@
 
 package org.apache.skywalking.apm.plugin.vertx3;
 
+import io.vertx.core.http.HttpClientResponse;
 import org.apache.skywalking.apm.agent.core.context.ContextManager;
+import org.apache.skywalking.apm.agent.core.context.tag.Tags;
 import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan;
 import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
@@ -31,10 +33,10 @@ import java.lang.reflect.Method;
 public class HttpClientRequestImplHandleResponseInterceptor implements InstanceMethodsAroundInterceptor {
 
     @Override
-    @SuppressWarnings("unchecked")
     public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
         MethodInterceptResult result) throws Throwable {
         VertxContext context = (VertxContext) objInst.getSkyWalkingDynamicField();
+        Tags.STATUS_CODE.set(context.getSpan(), Integer.toString(((HttpClientResponse) allArguments[0]).statusCode()));
         context.getSpan().asyncFinish();
 
         AbstractSpan span = ContextManager.createLocalSpan("#" + context.getSpan().getOperationName());

apm-sniffer/apm-sdk-plugin/vertx-plugins/vertx-core-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/vertx3/HttpServerResponseImplEndInterceptor.java

@@ -18,7 +18,9 @@
 
 package org.apache.skywalking.apm.plugin.vertx3;
 
+import io.vertx.core.http.HttpServerResponse;
 import org.apache.skywalking.apm.agent.core.context.ContextManager;
+import org.apache.skywalking.apm.agent.core.context.tag.Tags;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor;
 import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult;
@@ -28,13 +30,11 @@ import java.lang.reflect.Method;
 public class HttpServerResponseImplEndInterceptor implements InstanceMethodsAroundInterceptor {
 
     @Override
-    @SuppressWarnings("unchecked")
     public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
         MethodInterceptResult result) throws Throwable {
-        if (allArguments.length == 0) {
-            VertxContext context = (VertxContext) objInst.getSkyWalkingDynamicField();
-            context.getSpan().asyncFinish();
-        }
+        VertxContext context = (VertxContext) objInst.getSkyWalkingDynamicField();
+        Tags.STATUS_CODE.set(context.getSpan(), Integer.toString(((HttpServerResponse) objInst).getStatusCode()));
+        context.getSpan().asyncFinish();
     }
 
     @Override

apm-sniffer/apm-sdk-plugin/vertx-plugins/vertx-core-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/vertx3/define/HttpClientRequestImplEndInstrumentation.java

@@ -26,8 +26,8 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInst
 import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
 import org.apache.skywalking.apm.agent.core.plugin.match.NameMatch;
 
-import static net.bytebuddy.matcher.ElementMatchers.any;
 import static net.bytebuddy.matcher.ElementMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 
 /**
  * {@link HttpClientRequestImplEndInstrumentation} enhance the <code>end</code> method in
@@ -42,40 +42,62 @@ public class HttpClientRequestImplEndInstrumentation extends ClassInstanceMethod
 
     @Override
     public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
-        return new ConstructorInterceptPoint[] {
-            new ConstructorInterceptPoint() {
-                @Override
-                public ElementMatcher<MethodDescription> getConstructorMatcher() {
-                    return any();
-                }
+        return new ConstructorInterceptPoint[]{
+                new ConstructorInterceptPoint() {
+                    @Override
+                    public ElementMatcher<MethodDescription> getConstructorMatcher() {
+                        return takesArgument(3, int.class);
+                    }
+
+                    @Override
+                    public String getConstructorInterceptor() {
+                        return INTERCEPT_CLASS + "$Version30XTo33XConstructorInterceptor";
+                    }
+                },
+                new ConstructorInterceptPoint() {
+                    @Override
+                    public ElementMatcher<MethodDescription> getConstructorMatcher() {
+                        return takesArgument(4, int.class);
+                    }
+
+                    @Override
+                    public String getConstructorInterceptor() {
+                        return INTERCEPT_CLASS + "$Version34XTo37XConstructorInterceptor";
+                    }
+                },
+                new ConstructorInterceptPoint() {
+                    @Override
+                    public ElementMatcher<MethodDescription> getConstructorMatcher() {
+                        return takesArgument(5, int.class);
+                    }
 
-                @Override
-                public String getConstructorInterceptor() {
-                    return INTERCEPT_CLASS;
+                    @Override
+                    public String getConstructorInterceptor() {
+                        return INTERCEPT_CLASS + "$Version38PlusConstructorInterceptor";
+                    }
                 }
-            }
         };
     }
 
     @Override
     public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() {
-        return new InstanceMethodsInterceptPoint[] {
-            new InstanceMethodsInterceptPoint() {
-                @Override
-                public ElementMatcher<MethodDescription> getMethodsMatcher() {
-                    return named(ENHANCE_METHOD);
-                }
+        return new InstanceMethodsInterceptPoint[]{
+                new InstanceMethodsInterceptPoint() {
+                    @Override
+                    public ElementMatcher<MethodDescription> getMethodsMatcher() {
+                        return named(ENHANCE_METHOD);
+                    }
 
-                @Override
-                public String getMethodsInterceptor() {
-                    return INTERCEPT_CLASS;
-                }
+                    @Override
+                    public String getMethodsInterceptor() {
+                        return INTERCEPT_CLASS;
+                    }
 
-                @Override
-                public boolean isOverrideArgs() {
-                    return false;
+                    @Override
+                    public boolean isOverrideArgs() {
+                        return false;
+                    }
                 }
-            }
         };
     }
 

apm-sniffer/apm-sdk-plugin/vertx-plugins/vertx-core-3.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/vertx3/define/HttpServerResponseImplEndInstrumentation.java

@@ -27,6 +27,7 @@ import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch;
 import org.apache.skywalking.apm.agent.core.plugin.match.NameMatch;
 
 import static net.bytebuddy.matcher.ElementMatchers.named;
+import static org.apache.skywalking.apm.agent.core.plugin.bytebuddy.ArgumentTypeNameMatch.takesArgumentWithType;
 
 /**
  * {@link HttpServerResponseImplEndInstrumentation} enhance the <code>end</code> method in
@@ -36,8 +37,7 @@ import static net.bytebuddy.matcher.ElementMatchers.named;
 public class HttpServerResponseImplEndInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
 
     private static final String ENHANCE_CLASS = "io.vertx.core.http.impl.HttpServerResponseImpl";
-    private static final String ENHANCE_METHOD = "end";
-    private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.vertx3.HttpServerResponseImplEndInterceptor";
+    private static final String INTERCEPTOR_CLASS = "org.apache.skywalking.apm.plugin.vertx3.HttpServerResponseImplEndInterceptor";
 
     @Override
     public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
@@ -50,12 +50,17 @@ public class HttpServerResponseImplEndInstrumentation extends ClassInstanceMetho
             new InstanceMethodsInterceptPoint() {
                 @Override
                 public ElementMatcher<MethodDescription> getMethodsMatcher() {
-                    return named(ENHANCE_METHOD);
+                    return named("end0") //ver. 3.0.0 - 3.5.4
+                            .or(named("end") //ver. 3.6.0 - 3.7.0
+                                    .and(takesArgumentWithType(0, "io.vertx.core.buffer.Buffer")))
+                            .or(named("end") //ver. 3.7.1+
+                                    .and(takesArgumentWithType(0, "io.vertx.core.buffer.Buffer"))
+                                    .and(takesArgumentWithType(1, "io.netty.channel.ChannelPromise")));
                 }
 
                 @Override
                 public String getMethodsInterceptor() {
-                    return INTERCEPT_CLASS;
+                    return INTERCEPTOR_CLASS;
                 }
 
                 @Override

test/plugin/scenarios/vertx-eventbus-3.x-scenario/config/expectedData.yaml

@@ -15,8 +15,26 @@
 # limitations under the License.
 segmentItems:
   - serviceName: vertx-eventbus-3.x-scenario
-    segmentSize: 7
+    segmentSize: 8
     segments:
+      - segmentId: not null
+        spans:
+          - operationName: /vertx-eventbus-3-scenario/case/healthCheck
+            operationId: 0
+            parentSpanId: -1
+            spanId: 0
+            spanLayer: Http
+            startTime: nq 0
+            endTime: nq 0
+            componentId: 59
+            isError: false
+            spanType: Entry
+            peer: ''
+            skipAnalysis: false
+            tags:
+              - {key: http.method, value: HEAD}
+              - {key: url, value: /vertx-eventbus-3-scenario/case/healthCheck}
+              - {key: status_code, value: '200'}
       - segmentId: not null
         spans:
           - operationName: local-message-receiver
@@ -58,6 +76,7 @@ segmentItems:
             tags:
               - {key: http.method, value: GET}
               - {key: url, value: /vertx-eventbus-3-scenario/case/executeTest}
+              - {key: status_code, value: '200'}
           - operationName: /vertx-eventbus-3-scenario/case/eventbus-case
             operationId: 0
             parentSpanId: -1
@@ -73,6 +92,7 @@ segmentItems:
             tags:
               - {key: http.method, value: GET}
               - {key: url, value: /vertx-eventbus-3-scenario/case/eventbus-case}
+              - {key: status_code, value: '200'}
       - segmentId: not null
         spans:
           - operationName: '#/vertx-eventbus-3-scenario/case/executeTest'
@@ -133,6 +153,7 @@ segmentItems:
             tags:
               - {key: http.method, value: GET}
               - {key: url, value: /vertx-eventbus-3-scenario/case/executeTest}
+              - {key: status_code, value: '200'}
             refs:
               - {parentEndpoint: /vertx-eventbus-3-scenario/case/eventbus-case, networkAddress: 'localhost:8080',
                  refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null,

test/plugin/scenarios/vertx-eventbus-3.x-scenario/pom.xml

@@ -30,7 +30,7 @@
     <properties>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <compiler.version>1.8</compiler.version>
-        <test.framework.version>3.7.0</test.framework.version>
+        <test.framework.version>3.9.0</test.framework.version>
     </properties>
 
     <name>skywalking-vertx-eventbus-3.x-scenario</name>

test/plugin/scenarios/vertx-eventbus-3.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/vertxeventbus/controller/VertxEventbusController.java

@@ -30,7 +30,7 @@ public class VertxEventbusController extends AbstractVerticle {
     @Override
     public void start() {
         Router router = Router.router(vertx);
-        router.get("/vertx-eventbus-3-scenario/case/eventbus-case").handler(this::handleCoreCase);
+        router.get("/vertx-eventbus-3-scenario/case/eventbus-case").handler(this::handleEventbusCase);
         router.get("/vertx-eventbus-3-scenario/case/executeTest").handler(this::executeTest);
         router.head("/vertx-eventbus-3-scenario/case/healthCheck").handler(this::healthCheck);
         vertx.createHttpServer().requestHandler(router::accept).listen(8080);
@@ -39,7 +39,7 @@ public class VertxEventbusController extends AbstractVerticle {
         vertx.deployVerticle(LocalReceiver.class.getName());
     }
 
-    private void handleCoreCase(RoutingContext routingContext) {
+    private void handleEventbusCase(RoutingContext routingContext) {
         vertx.createHttpClient().getNow(8080, "localhost",
                 "/vertx-eventbus-3-scenario/case/executeTest",
                 it -> routingContext.response().setStatusCode(it.statusCode()).end());

test/plugin/scenarios/vertx-eventbus-3.x-scenario/support-version.list

@@ -14,6 +14,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+3.9.0
+3.8.5
+3.8.4
+3.8.3
+3.8.2
+3.8.1
+3.8.0
+3.7.1
 3.7.0
 3.6.3
 3.6.2

test/plugin/scenarios/vertx-web-3.x-scenario/config/expectedData.yaml

@@ -15,9 +15,50 @@
 # limitations under the License.
 segmentItems:
   - serviceName: vertx-web-3.x-scenario
-    segmentSize: 2
+    segmentSize: 4
     segments:
-      - segmentId: nq 0
+      - segmentId: not null
+        spans:
+          - operationName: /vertx-web-3-scenario/case/healthCheck
+            operationId: 0
+            parentSpanId: -1
+            spanId: 0
+            spanLayer: Http
+            startTime: nq 0
+            endTime: nq 0
+            componentId: 59
+            isError: false
+            spanType: Entry
+            peer: ''
+            skipAnalysis: false
+            tags:
+              - {key: http.method, value: HEAD}
+              - {key: url, value: /vertx-web-3-scenario/case/healthCheck}
+              - {key: status_code, value: '200'}
+      - segmentId: not null
+        spans:
+          - operationName: /vertx-web-3-scenario/case/healthCheck
+            operationId: 0
+            parentSpanId: -1
+            spanId: 0
+            spanLayer: Http
+            startTime: nq 0
+            endTime: nq 0
+            componentId: 59
+            isError: false
+            spanType: Entry
+            peer: ''
+            skipAnalysis: false
+            tags:
+              - {key: http.method, value: HEAD}
+              - {key: url, value: /vertx-web-3-scenario/case/healthCheck}
+              - {key: status_code, value: '200'}
+            refs:
+              - {parentEndpoint: /vertx-web-3-scenario/case/web-case, networkAddress: 'localhost:8080',
+                 refType: CrossProcess, parentSpanId: 1, parentTraceSegmentId: not null,
+                 parentServiceInstance: not null, parentService: vertx-web-3.x-scenario,
+                 traceId: not null}
+      - segmentId: not null
         spans:
           - operationName: /vertx-web-3-scenario/case/healthCheck
             operationId: 0
@@ -32,8 +73,9 @@ segmentItems:
             peer: localhost:8080
             skipAnalysis: false
             tags:
-              - {key: http.method, value: GET}
+              - {key: http.method, value: HEAD}
               - {key: url, value: /vertx-web-3-scenario/case/healthCheck}
+              - {key: status_code, value: '200'}
           - operationName: /vertx-web-3-scenario/case/web-case
             operationId: 0
             parentSpanId: -1
@@ -49,7 +91,8 @@ segmentItems:
             tags:
               - {key: http.method, value: GET}
               - {key: url, value: /vertx-web-3-scenario/case/web-case}
-      - segmentId: nq 0
+              - {key: status_code, value: '200'}
+      - segmentId: not null
         spans:
           - operationName: '#/vertx-web-3-scenario/case/healthCheck'
             operationId: 0
@@ -68,4 +111,3 @@ segmentItems:
                  refType: CrossThread, parentSpanId: 1, parentTraceSegmentId: not null,
                  parentServiceInstance: not null, parentService: vertx-web-3.x-scenario,
                  traceId: not null}
-

test/plugin/scenarios/vertx-web-3.x-scenario/pom.xml

@@ -30,7 +30,7 @@
     <properties>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
         <compiler.version>1.8</compiler.version>
-        <test.framework.version>3.7.0</test.framework.version>
+        <test.framework.version>3.9.0</test.framework.version>
     </properties>
 
     <name>skywalking-vertx-web-3.x-scenario</name>

test/plugin/scenarios/vertx-web-3.x-scenario/src/main/java/org/apache/skywalking/apm/testcase/vertxweb/controller/VertxWebController.java

@@ -32,7 +32,7 @@ public class VertxWebController extends AbstractVerticle {
     }
 
     private void handleCoreCase(RoutingContext routingContext) {
-        vertx.createHttpClient().getNow(8080, "localhost",
+        vertx.createHttpClient().headNow(8080, "localhost",
                 "/vertx-web-3-scenario/case/healthCheck",
                 it -> routingContext.response().setStatusCode(it.statusCode()).end());
     }

test/plugin/scenarios/vertx-web-3.x-scenario/support-version.list

@@ -14,6 +14,14 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+3.9.0
+3.8.5
+3.8.4
+3.8.3
+3.8.2
+3.8.1
+3.8.0
+3.7.1
 3.7.0
 3.6.3
 3.6.2