pinpoint-apm#1171 Fixed a side effect of Arcus interceptor

- modify dns lookup api InetSocketAddress.getHostName() -> InetAddress.getHostAddress()
emeroad · Nov 11, 2015 · a79a27b · a79a27b
1 parent cd18294
commit a79a27b
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 10 deletions.
diff --git a/.../main/java/com/navercorp/pinpoint/profiler/modifier/arcus/interceptor/ApiInterceptor.java b/.../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/...java/com/navercorp/pinpoint/profiler/modifier/arcus/interceptor/FutureGetInterceptor.java b/...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);