You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2019/10/29 10:28:51 UTC

[skywalking] branch master updated: fix deficient code, "/socket.io/" http segment declare (#3717)

This is an automated email from the ASF dual-hosted git repository.

wusheng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking.git


The following commit(s) were added to refs/heads/master by this push:
     new 7ebb310  fix deficient code, "/socket.io/" http segment declare (#3717)
7ebb310 is described below

commit 7ebb3107bf0dd338af59456a3a2118b48dac5958
Author: mrproliu <74...@qq.com>
AuthorDate: Tue Oct 29 18:28:41 2019 +0800

    fix deficient code, "/socket.io/" http segment declare (#3717)
    
    * fix deficient code, "/socket.io/" http segment declare
    
    * add agent test and fix expectedData.yaml file
    
    * add listener on tomcat start, work for reduce socket io server start time
    change "/socket.io/" operation "http.method", cause get or post will invoke both
    
    * resolve style check error
    
    * change socket io server and client start to health check servlet
    
    * if startClientAndWaitConnect invoke on multi-thread, it will only check client ref, no check connect state, fix it.
---
 Jenkinsfile-Agent-Test                             |  6 +++
 .../config/expectedData.yaml                       | 32 +++++++++++----
 .../apm/testcase/netty/socketio/CaseServlet.java   | 11 ------
 ...ealthCheckServlet.java => ContextListener.java} | 31 +++++++++------
 .../netty/socketio/HealthCheckServlet.java         |  7 ++++
 .../testcase/netty/socketio/SocketIOStarter.java   | 46 ++++++++--------------
 .../src/main/webapp/WEB-INF/web.xml                |  1 -
 7 files changed, 72 insertions(+), 62 deletions(-)

diff --git a/Jenkinsfile-Agent-Test b/Jenkinsfile-Agent-Test
index cb62f10..cc9093c 100755
--- a/Jenkinsfile-Agent-Test
+++ b/Jenkinsfile-Agent-Test
@@ -104,6 +104,12 @@ pipeline {
                                 sh 'bash test/plugin/run.sh --build_id=wl1_${BUILD_ID} spring-async-scenario'
                             }
                         }
+
+                        stage('netty-socketio 1.x (4)') {
+                            steps {
+                                sh 'bash test/plugin/run.sh --build_id=wl1_${BUILD_ID} netty-socketio-scenario'
+                            }
+                        }
                     }
                 }
                 stage('Group2') {
diff --git a/test/plugin/scenarios/netty-socketio-scenario/config/expectedData.yaml b/test/plugin/scenarios/netty-socketio-scenario/config/expectedData.yaml
index 5f328fd..e8ea27c 100644
--- a/test/plugin/scenarios/netty-socketio-scenario/config/expectedData.yaml
+++ b/test/plugin/scenarios/netty-socketio-scenario/config/expectedData.yaml
@@ -29,39 +29,39 @@ segmentItems:
     segments:
       - segmentId: not null
         spans:
-          - operationName: SocketIO/onConnect
+          - operationName: /netty-socketio-scenario/case/netty-socketio
             operationId: 0
             parentSpanId: -1
             spanId: 0
             spanLayer: Http
             startTime: nq 0
             endTime: nq 0
-            componentId: 76
+            componentId: 1
             componentName: ''
             isError: false
             spanType: Entry
             peer: ''
             peerId: 0
             tags:
-              - {key: from, value: not null}
+              - {key: url, value: 'http://localhost:8080/netty-socketio-scenario/case/netty-socketio'}
+              - {key: http.method, value: GET}
       - segmentId: not null
         spans:
-          - operationName: /netty-socketio-scenario/case/netty-socketio
+          - operationName: SocketIO/onConnect
             operationId: 0
             parentSpanId: -1
             spanId: 0
             spanLayer: Http
             startTime: nq 0
             endTime: nq 0
-            componentId: 1
+            componentId: 76
             componentName: ''
             isError: false
             spanType: Entry
             peer: ''
             peerId: 0
             tags:
-              - {key: url, value: 'http://localhost:8080/netty-socketio-scenario/case/netty-socketio'}
-              - {key: http.method, value: GET}
+              - {key: from, value: not null}
       - segmentId: not null
         spans:
           - operationName: SocketIO/send_data/receive
@@ -77,3 +77,21 @@ segmentItems:
             spanType: Entry
             peer: ''
             peerId: 0
+      - segmentId: not null
+        spans:
+          - operationName: /socket.io/
+            operationId: 0
+            parentSpanId: -1
+            spanId: 0
+            spanLayer: Http
+            startTime: nq 0
+            endTime: nq 0
+            componentId: 12
+            componentName: ''
+            isError: false
+            spanType: Exit
+            peer: not null
+            peerId: 0
+            tags:
+              - {key: http.method, value: not null}
+              - {key: url, value: not null}
\ No newline at end of file
diff --git a/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/CaseServlet.java b/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/CaseServlet.java
index 6ad02af..33cfab9 100644
--- a/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/CaseServlet.java
+++ b/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/CaseServlet.java
@@ -29,17 +29,6 @@ import java.io.PrintWriter;
 
 public class CaseServlet extends HttpServlet {
 
-    static {
-        // start socket io server
-        SocketIOStarter.startServer();
-
-        // start client
-        try {
-            SocketIOStarter.startClientAndWaitConnect();
-        } catch (Exception e) {
-        }
-    }
-
     @Override
     protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
         // create socket io client and send data
diff --git a/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/HealthCheckServlet.java b/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/ContextListener.java
similarity index 58%
copy from test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/HealthCheckServlet.java
copy to test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/ContextListener.java
index ac81af3..5c99e63 100644
--- a/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/HealthCheckServlet.java
+++ b/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/ContextListener.java
@@ -13,28 +13,33 @@
  * 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.testcase.netty.socketio;
 
-import javax.servlet.ServletException;
-import javax.servlet.http.HttpServlet;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import java.io.IOException;
-import java.io.PrintWriter;
+import javax.servlet.ServletContextEvent;
+import javax.servlet.ServletContextListener;
 
-public class HealthCheckServlet extends HttpServlet {
+/**
+ * @author MrPro
+ */
+public class ContextListener implements ServletContextListener {
 
     @Override
-    protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
-        PrintWriter writer = resp.getWriter();
-        writer.write("Success");
-        writer.flush();
+    public void contextInitialized(ServletContextEvent servletContextEvent) {
+        // start socket io server on tomcat start
+        SocketIOStarter.startServer();
+
+        // start client
+        try {
+            SocketIOStarter.startClientAndWaitConnect();
+        } catch (Exception e) {
+        }
     }
 
     @Override
-    protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
-        doGet(req, resp);
+    public void contextDestroyed(ServletContextEvent servletContextEvent) {
+        SocketIOStarter.server.stop();
     }
 }
diff --git a/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/HealthCheckServlet.java b/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/HealthCheckServlet.java
index ac81af3..ad4a73c 100644
--- a/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/HealthCheckServlet.java
+++ b/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/HealthCheckServlet.java
@@ -28,6 +28,13 @@ public class HealthCheckServlet extends HttpServlet {
 
     @Override
     protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
+        // start socket io server and client on heath check
+        SocketIOStarter.startServer();
+        try {
+            SocketIOStarter.startClientAndWaitConnect();
+        } catch (Exception e) {
+        }
+
         PrintWriter writer = resp.getWriter();
         writer.write("Success");
         writer.flush();
diff --git a/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/SocketIOStarter.java b/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/SocketIOStarter.java
index 1c26fa8..ff2b266 100644
--- a/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/SocketIOStarter.java
+++ b/test/plugin/scenarios/netty-socketio-scenario/src/main/java/org/apache/skywalking/apm/testcase/netty/socketio/SocketIOStarter.java
@@ -17,17 +17,14 @@
 
 package org.apache.skywalking.apm.testcase.netty.socketio;
 
-import com.corundumstudio.socketio.AckRequest;
 import com.corundumstudio.socketio.Configuration;
-import com.corundumstudio.socketio.SocketIOClient;
 import com.corundumstudio.socketio.SocketIOServer;
-import com.corundumstudio.socketio.listener.ConnectListener;
-import com.corundumstudio.socketio.listener.DataListener;
 import io.socket.client.IO;
 import io.socket.client.Socket;
 import io.socket.emitter.Emitter;
 
 import java.net.URISyntaxException;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.TimeUnit;
 
@@ -43,53 +40,42 @@ public class SocketIOStarter {
     public static SocketIOServer server;
     public static Socket client;
 
+    private static CountDownLatch connectedCountDownLatch = new CountDownLatch(1);
+
     public static void startServer() {
+        if (server != null) {
+            return;
+        }
         Configuration config = new Configuration();
         config.setHostname("localhost");
         config.setPort(SERVER_PORT);
+        config.setBossThreads(1);
+        config.setWorkerThreads(1);
 
         server = new SocketIOServer(config);
-        server.addEventListener(LISTEN_EVENT_NAME, String.class, new DataListener<String>() {
-            @Override
-            public void onData(SocketIOClient client, String data, AckRequest ackRequest) {
-                // get message
-            }
-        });
-
-        server.addConnectListener(new ConnectClientListener());
 
         server.start();
-
-        // close server on kill signal
-        Runtime.getRuntime().addShutdownHook(new Thread() {
-            @Override
-            public void run() {
-                server.stop();
-            }
-        });
-    }
-
-    private static class ConnectClientListener implements ConnectListener {
-
-        @Override
-        public void onConnect(SocketIOClient client) {
-            // connect client
-        }
     }
 
     public static void startClientAndWaitConnect() throws URISyntaxException, InterruptedException {
+        if (client != null) {
+            // check client is connected again
+            // if this method invoke on multi thread, client will return but not connected
+            connectedCountDownLatch.await(5, TimeUnit.SECONDS);
+            return;
+        }
         client = IO.socket("http://localhost:" + SERVER_PORT);
         LinkedBlockingQueue<Boolean> connected = new LinkedBlockingQueue<>(1);
         client.on(Socket.EVENT_CONNECT, new Emitter.Listener() {
             @Override
             public void call(Object... objects) {
-                connected.add(true);
+                connectedCountDownLatch.countDown();
             }
         });
         client.connect();
 
         // wait connect to server
-        connected.poll(5, TimeUnit.SECONDS);
+        connectedCountDownLatch.await(5, TimeUnit.SECONDS);
     }
 
 }
diff --git a/test/plugin/scenarios/netty-socketio-scenario/src/main/webapp/WEB-INF/web.xml b/test/plugin/scenarios/netty-socketio-scenario/src/main/webapp/WEB-INF/web.xml
index 655eb65..d790f44 100644
--- a/test/plugin/scenarios/netty-socketio-scenario/src/main/webapp/WEB-INF/web.xml
+++ b/test/plugin/scenarios/netty-socketio-scenario/src/main/webapp/WEB-INF/web.xml
@@ -41,5 +41,4 @@
     </servlet-mapping>
 
 
-
 </web-app>