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/plugins/arcus/src/main/java/com/navercorp/pinpoint/plugin/arcus/interceptor/ApiInterceptor.java b/plugins/arcus/src/main/java/com/navercorp/pinpoint/plugin/arcus/interceptor/ApiInterceptor.java
index 0a1d7f07315a..f1567e99d221 100644
--- a/plugins/arcus/src/main/java/com/navercorp/pinpoint/plugin/arcus/interceptor/ApiInterceptor.java
+++ b/plugins/arcus/src/main/java/com/navercorp/pinpoint/plugin/arcus/interceptor/ApiInterceptor.java
@@ -14,6 +14,7 @@
  */
 package com.navercorp.pinpoint.plugin.arcus.interceptor;
 
+import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.util.concurrent.Future;
@@ -126,15 +127,14 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
 
             // find the target node
             if (result instanceof Future && result instanceof OperationAccessor) {
-                Operation op = ((OperationAccessor)result)._$PINPOINT$_getOperation();
-
+                final Operation op = ((OperationAccessor)result)._$PINPOINT$_getOperation();
                 if (op != null) {
-                    MemcachedNode handlingNode = op.getHandlingNode();
-                    SocketAddress socketAddress = handlingNode.getSocketAddress();
-
-                    if (socketAddress instanceof InetSocketAddress) {
-                        InetSocketAddress address = (InetSocketAddress) socketAddress;
-                        recorder.recordEndPoint(address.getHostName() + ":" + address.getPort());
+                    final MemcachedNode handlingNode = op.getHandlingNode();
+                    if (handlingNode != null) {
+                        final String endPoint = getEndPoint(handlingNode);
+                        if (endPoint != null) {
+                            recorder.recordEndPoint(endPoint);
+                        }
                     }
                 } else {
                     logger.info("operation not found");
@@ -179,6 +179,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();
+    }
+
     private boolean isAsynchronousInvocation(final Object target, final Object[] args, Object result, Throwable throwable) {
         if (throwable != null || result == null) {
             return false;
diff --git a/plugins/arcus/src/main/java/com/navercorp/pinpoint/plugin/arcus/interceptor/FutureGetInterceptor.java b/plugins/arcus/src/main/java/com/navercorp/pinpoint/plugin/arcus/interceptor/FutureGetInterceptor.java
index 57569a165c77..3ba470e5a60d 100644
--- a/plugins/arcus/src/main/java/com/navercorp/pinpoint/plugin/arcus/interceptor/FutureGetInterceptor.java
+++ b/plugins/arcus/src/main/java/com/navercorp/pinpoint/plugin/arcus/interceptor/FutureGetInterceptor.java
@@ -14,6 +14,7 @@
  */
 package com.navercorp.pinpoint.plugin.arcus.interceptor;
 
+import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 
@@ -64,13 +65,14 @@ protected void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[
         }
 
         recorder.recordException(op.getException());
-        MemcachedNode handlingNode = op.getHandlingNode();
+
+        final MemcachedNode handlingNode = op.getHandlingNode();
         if (handlingNode != null) {
-            SocketAddress socketAddress = handlingNode.getSocketAddress();
-            if (socketAddress instanceof InetSocketAddress) {
-                InetSocketAddress address = (InetSocketAddress) socketAddress;
-                recorder.recordEndPoint(address.getHostName() + ":" + address.getPort());
+            final String endPoint = getEndPoint(handlingNode);
+            if (endPoint != null) {
+                recorder.recordEndPoint(endPoint);
             }
+            recorder.recordException(op.getException());
         } else {
             logger.info("no handling node");
         }
@@ -84,4 +86,38 @@ protected void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[
             }
         }
     }
+
+    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();
+    }
 }
\ No newline at end of file


Test patch: