Fixing reliability issue with JedisMethodVisitor. (#423)

* Jedis Visitor Changes

* Hooking up Jedis visitor

* remove printstack

* Update changelog

* fixed typo

CHANGELOG.md

@@ -1,6 +1,7 @@
 # CHANGELOG
 
 ## Version 1.0.10
+- Fixed reliability issue with Jedis client dependency collector
 - Fixed Request Telemetry Sending bug with new schema
 - Schema updated to the latest version. Changes in internal namespace `core/src/main/java/com/microsoft/applicationinsights/internal/schemav2`.
 - Class `SendableData` in internal namespace deleted.

agent/src/main/java/com/microsoft/applicationinsights/agent/internal/agent/DefaultMethodVisitor.java

@@ -86,11 +86,11 @@ public DefaultMethodVisitor(MethodInstrumentationDecision decision,
     protected void byteCodeForMethodExit(int opcode) {
 
         Object[] args = null;
-        String methodSignature = FINISH_METHOD_DEFAULT_SIGNATURE;
+        String methodSignature = getOnExitMethodDefaultSignature();
         switch (translateExitCode(opcode)) {
             case EXIT_WITH_EXCEPTION:
                 args = new Object[] { getMethodName(), duplicateTopStackToTempVariable(Type.getType(Throwable.class)) };
-                methodSignature = FINISH_METHOD_EXCEPTION_SIGNATURE;
+                methodSignature = getOnExitMethodExceptionSignature();
                 break;
 
             case EXIT_WITH_RETURN_VALUE:
@@ -103,7 +103,7 @@ protected void byteCodeForMethodExit(int opcode) {
         }
 
         if (args != null) {
-            activateEnumMethod(ImplementationsCoordinator.class, FINISH_DETECT_METHOD_NAME, methodSignature, args);
+            activateEnumMethod(ImplementationsCoordinator.class, getOnExitMethodName(), methodSignature, args);
         }
     }
 
@@ -143,8 +143,28 @@ protected void onMethodEnter() {
 
         activateEnumMethod(
                 ImplementationsCoordinator.class,
-                START_DETECT_METHOD_NAME,
-                START_DETECT_METHOD_SIGNATURE,
+                getOnEnterMethodName(),
+                getOnEnterMethodSignature(),
                 getMethodName());
     }
+
+    protected String getOnEnterMethodName() {
+        return START_DETECT_METHOD_NAME;
+    } 
+
+    protected String getOnEnterMethodSignature() {
+        return START_DETECT_METHOD_SIGNATURE;
+    }
+
+    protected String getOnExitMethodName() {
+        return FINISH_DETECT_METHOD_NAME;
+    }
+
+    protected String getOnExitMethodDefaultSignature() {
+        return FINISH_METHOD_DEFAULT_SIGNATURE;
+    }
+
+    protected String getOnExitMethodExceptionSignature() {
+        return FINISH_METHOD_EXCEPTION_SIGNATURE;
+    }
 }

agent/src/main/java/com/microsoft/applicationinsights/agent/internal/agent/redis/JedisClassDataProvider.java

@@ -46,13 +46,13 @@ public JedisClassDataProvider(Map<String, ClassInstrumentationData> classesToIns
     public void add() {
         try {
             ClassInstrumentationData data =
-                    new ClassInstrumentationData(JEDIS_CLASS_NAME, InstrumentedClassType.OTHER)
+                    new ClassInstrumentationData(JEDIS_CLASS_NAME, InstrumentedClassType.Redis)
                             .setReportCaughtExceptions(false)
                             .setReportExecutionTime(true);
             MethodVisitorFactory methodVisitorFactory = new MethodVisitorFactory() {
                 @Override
                 public MethodVisitor create(MethodInstrumentationDecision decision, int access, String desc, String owner, String methodName, MethodVisitor methodVisitor, ClassToMethodTransformationData additionalData) {
-                    return new JedisMethodVisitor(access, desc, JEDIS_CLASS_NAME, methodName, methodVisitor, additionalData);
+                    return new JedisMethodVisitorV2(access, desc, JEDIS_CLASS_NAME, methodName, methodVisitor, additionalData);
                 }
             };
             data.addAllMethods(false, true, methodVisitorFactory);

agent/src/main/java/com/microsoft/applicationinsights/agent/internal/agent/redis/JedisMethodVisitor.java

@@ -30,9 +30,12 @@
 import org.objectweb.asm.Opcodes;
 import org.objectweb.asm.Type;
 
+
 /**
  * Created by gupele on 8/6/2015.
+ * @deprecated Replaced with JedisMethodVisitorV2
  */
+@Deprecated 
 final class JedisMethodVisitor extends DefaultMethodVisitor {
     private final static String FINISH_DETECT_METHOD_NAME = "methodFinished";
     private final static String FINISH_METHOD_DEFAULT_SIGNATURE = "(Ljava/lang/String;J[Ljava/lang/Object;Ljava/lang/Throwable;)V";

agent/src/main/java/com/microsoft/applicationinsights/agent/internal/agent/redis/JedisMethodVisitorV2.java

@@ -0,0 +1,55 @@
+/*
+ * ApplicationInsights-Java
+ * Copyright (c) Microsoft Corporation
+ * All rights reserved.
+ *
+ * MIT License
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of this
+ * software and associated documentation files (the ""Software""), to deal in the Software
+ * without restriction, including without limitation the rights to use, copy, modify, merge,
+ * publish, distribute, sublicense, and/or sell copies of the Software, and to permit
+ * persons to whom the Software is furnished to do so, subject to the following conditions:
+ * The above copyright notice and this permission notice shall be included in all copies or
+ * substantial portions of the Software.
+ * THE SOFTWARE IS PROVIDED *AS IS*, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+ * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
+ * PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
+ * FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+ * OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+package com.microsoft.applicationinsights.agent.internal.agent.redis;
+
+import com.microsoft.applicationinsights.agent.internal.agent.DefaultMethodVisitor;
+import com.microsoft.applicationinsights.agent.internal.agent.ClassToMethodTransformationData;
+
+import org.objectweb.asm.MethodVisitor;
+
+/**
+ * The class is responsible for instrumenting Jedis client methods.
+ */
+final class JedisMethodVisitorV2 extends DefaultMethodVisitor {
+    private final static String ON_ENTER_METHOD_NAME = "jedisMethodStarted";
+    private final static String ON_ENTER_METHOD_SIGNATURE = "(Ljava/lang/String;)V";
+
+    public JedisMethodVisitorV2(int access,
+                                  String desc,
+                                  String owner,
+                                  String methodName,
+                                  MethodVisitor methodVisitor,
+                                  ClassToMethodTransformationData additionalData) {
+        super(false, true, 0, access, desc, owner, methodName, methodVisitor, additionalData);
+    }
+
+    @Override
+    protected String getOnEnterMethodName() {
+        return ON_ENTER_METHOD_NAME;
+    }
+
+    @Override
+    protected String getOnEnterMethodSignature() {
+        return ON_ENTER_METHOD_SIGNATURE;
+    }
+
+}

agent/src/main/java/com/microsoft/applicationinsights/agent/internal/coresync/AgentNotificationsHandler.java

@@ -97,6 +97,12 @@ public interface AgentNotificationsHandler {
      */
     void preparedStatementExecuteBatchMethodStarted(String classAndMethodNames, PreparedStatement statement, String sqlStatement, int batchCounter);
 
+     /**
+     * Called before methods in the Jedis client class are executed.
+     * @param classAndMethodNames The name of the class and method separated by '.'
+     */
+    void jedisMethodStarted(String classAndMethodNames);
+
     /**
      * A 'regular' method enter. Non HTTP/SQL method
      * @param classAndMethodNames The name of the class and method separated by '.'

agent/src/main/java/com/microsoft/applicationinsights/agent/internal/coresync/InstrumentedClassType.java

@@ -27,5 +27,6 @@
 public enum InstrumentedClassType {
     SQL,
     HTTP,
-    OTHER
+    OTHER,
+    Redis
 }

agent/src/main/java/com/microsoft/applicationinsights/agent/internal/coresync/impl/ImplementationsCoordinator.java

@@ -175,6 +175,17 @@ public void sqlStatementMethodStarted(String name, Statement statement, String s
         }
     }
 
+    @Override
+    public void jedisMethodStarted(String name) {
+        try {
+            AgentNotificationsHandler implementation = getImplementation();
+            if (implementation != null) {
+                implementation.jedisMethodStarted(name);
+            }
+        } catch (Throwable t) {
+        }
+    }
+
     @Override
     public void methodStarted(String name) {
         try {

core/src/main/java/com/microsoft/applicationinsights/internal/agent/CoreAgentNotificationsHandler.java

@@ -155,6 +155,16 @@ public void httpMethodFinished(String identifier, String method, String uri, int
         telemetryClient.track(telemetry);
     }
 
+    @Override
+    public void jedisMethodStarted(String name) {
+        int index = name.lastIndexOf('#');
+        if (index != -1) {            
+            name = name.substring(0, index);
+        }
+
+        startMethod(InstrumentedClassType.Redis.toString(), name, new String[]{});
+    }
+
     @Override
     public void methodStarted(String name) {
         int index = name.lastIndexOf('#');