Repository: pinpoint-apm/pinpoint
Issue: Fixed a side effect of Arcus interceptor
Labels: bug

Side effect : faild to failover.
Root cause : Unnecessary reverse DNS lookup
Bug fix : Avoid DNS lookup API.
- ApiInterceptor.java
- FutureGetInterceptor.java

``` java
@Override
public void after(Object target, Object[] args, Object result, Throwable throwable) {
...
            final Operation op = (Operation) getOperation.invoke(target);
            if (op != null) {
                MemcachedNode handlingNode = op.getHandlingNode();
                if (handlingNode != null) {
                    SocketAddress socketAddress = handlingNode.getSocketAddress();
                    if (socketAddress instanceof InetSocketAddress) {
                        InetSocketAddress address = (InetSocketAddress) socketAddress;
                       // ROOT Cause : Unnecessary DNS look ->  InetSocketAddress.getHostName() 
                        trace.recordEndPoint(address.getHostName() + ":" + address.getPort());
                    }
                } else {
                    logger.info("no handling node");
                }
            } else {
                logger.info("operation not found");
            }
}
```


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/arcus/interceptor/ApiInterceptor.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/arcus/interceptor/ApiInterceptor.java
index e9d33e86a0ee..d90c717676c6 100644
--- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/arcus/interceptor/ApiInterceptor.java
+++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/arcus/interceptor/ApiInterceptor.java
@@ -16,6 +16,7 @@
 
 package com.navercorp.pinpoint.profiler.modifier.arcus.interceptor;
 
+import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.concurrent.Future;
@@ -61,11 +62,12 @@ public void doInAfterTrace(RecordableTrace trace, Object target, Object[] args,
         if (result instanceof Future && !(result instanceof FrontCacheGetFuture)) {
             final Operation op = getOperation(result);
             if (op != null) {
-                MemcachedNode handlingNode = op.getHandlingNode();
-                SocketAddress socketAddress = handlingNode.getSocketAddress();
-                if (socketAddress instanceof InetSocketAddress) {
-                    InetSocketAddress address = (InetSocketAddress) socketAddress;
-                    trace.recordEndPoint(address.getHostName() + ":" + address.getPort());
+                final MemcachedNode handlingNode = op.getHandlingNode();
+                if (handlingNode != null) {
+                    final String endPoint = getEndPoint(handlingNode);
+                    if (endPoint != null) {
+                        trace.recordEndPoint(endPoint);
+                    }
                 }
             } else {
                 logger.info("operation not found");
@@ -85,6 +87,40 @@ public void doInAfterTrace(RecordableTrace trace, Object target, Object[] args,
         trace.markAfterTime();
     }
 
+    private String getEndPoint(MemcachedNode handlingNode) {
+        // TODO duplicated code : ApiInterceptor, FutureGetInterceptor
+        final SocketAddress socketAddress = handlingNode.getSocketAddress();
+        if (socketAddress instanceof InetSocketAddress) {
+            final InetSocketAddress inetSocketAddress = (InetSocketAddress) socketAddress;
+            final String hostAddress = getHostAddress(inetSocketAddress);
+            if (hostAddress == null) {
+                // TODO return "Unknown Host"; ?
+                logger.debug("hostAddress is null");
+                return null;
+            }
+            return hostAddress + ":" + inetSocketAddress.getPort();
+
+        } else {
+            if (logger.isDebugEnabled()) {
+                logger.debug("invalid socketAddress:{}", socketAddress);
+            }
+            return null;
+        }
+    }
+
+    private String getHostAddress(InetSocketAddress inetSocketAddress) {
+        if (inetSocketAddress == null) {
+            return null;
+        }
+        // TODO JDK 1.7 InetSocketAddress.getHostString();
+        // Warning : Avoid unnecessary DNS lookup  (warning:InetSocketAddress.getHostName())
+        final InetAddress inetAddress = inetSocketAddress.getAddress();
+        if (inetAddress == null) {
+            return null;
+        }
+        return inetAddress.getHostAddress();
+    }
+
     private Operation getOperation(Object result) {
 //        __operation -> ObjectTraceValue1.class
         final Object operationObject = ObjectTraceValue1Utils.__getTraceObject1(result, null);
diff --git a/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/arcus/interceptor/FutureGetInterceptor.java b/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/arcus/interceptor/FutureGetInterceptor.java
index 6e5579a9aef0..1d0ca160a8d0 100644
--- a/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/arcus/interceptor/FutureGetInterceptor.java
+++ b/profiler/src/main/java/com/navercorp/pinpoint/profiler/modifier/arcus/interceptor/FutureGetInterceptor.java
@@ -16,6 +16,7 @@
 
 package com.navercorp.pinpoint.profiler.modifier.arcus.interceptor;
 
+import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 
@@ -77,12 +78,11 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
             // find the target node
             final Operation op = getOperation(target);
             if (op != null) {
-                MemcachedNode handlingNode = op.getHandlingNode();
+                final MemcachedNode handlingNode = op.getHandlingNode();
                 if (handlingNode != null) {
-                    SocketAddress socketAddress = handlingNode.getSocketAddress();
-                    if (socketAddress instanceof InetSocketAddress) {
-                        InetSocketAddress address = (InetSocketAddress) socketAddress;
-                        trace.recordEndPoint(address.getHostName() + ":" + address.getPort());
+                    final String endPoint = getEndPoint(handlingNode);
+                    if (endPoint != null) {
+                        trace.recordEndPoint(endPoint);
                     }
                     trace.recordException(op.getException());
                 } else {
@@ -113,6 +113,40 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
         }
     }
 
+    private String getEndPoint(MemcachedNode handlingNode) {
+        // TODO duplicated code : ApiInterceptor, FutureGetInterceptor
+        final SocketAddress socketAddress = handlingNode.getSocketAddress();
+        if (socketAddress instanceof InetSocketAddress) {
+            final InetSocketAddress inetSocketAddress = (InetSocketAddress) socketAddress;
+            final String hostAddress = getHostAddress(inetSocketAddress);
+            if (hostAddress == null) {
+                // TODO return "Unknown Host";
+                logger.debug("hostAddress is null");
+                return null;
+            }
+            return hostAddress + ":" + inetSocketAddress.getPort();
+
+        } else {
+            if (logger.isDebugEnabled()) {
+                logger.debug("invalid socketAddress:{}", socketAddress);
+            }
+            return null;
+        }
+    }
+
+    private String getHostAddress(InetSocketAddress inetSocketAddress) {
+        if (inetSocketAddress == null) {
+            return null;
+        }
+        // TODO JDK 1.7 InetSocketAddress.getHostString();
+        // Warning : Avoid unnecessary DNS lookup  (warning:InetSocketAddress.getHostName())
+        final InetAddress inetAddress = inetSocketAddress.getAddress();
+        if (inetAddress == null) {
+            return null;
+        }
+        return inetAddress.getHostAddress();
+    }
+
 //    __operation -> ObjectTraceValue1.class
     private Operation getOperation(Object target) {
         final Object operationObject = ObjectTraceValue1Utils.__getTraceObject1(target, null);


Test patch: