[#noissue] Refactor HttpClient4

plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/CommandContextFormatter.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright 2017 NAVER Corp.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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 com.navercorp.pinpoint.plugin.httpclient4;
+
+/**
+ * @author Woonduk Kang(emeroad)
+ */
+public class CommandContextFormatter {
+
+    public static String format(HttpCallContext commandContext) {
+        final StringBuilder sb = new StringBuilder();
+        sb.append("write=").append(commandContext.getWriteElapsedTime());
+        if (commandContext.isWriteFail()) {
+            sb.append("(fail)");
+        }
+        sb.append(", read=").append(commandContext.getReadElapsedTime());
+        if (commandContext.isReadFail()) {
+            sb.append("(fail)");
+        }
+        return sb.toString();
+    }
+}

plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/EndPointUtils.java

@@ -0,0 +1,58 @@
+/*
+ * Copyright 2017 NAVER Corp.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * 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 com.navercorp.pinpoint.plugin.httpclient4;
+
+import org.apache.http.HttpHost;
+import org.apache.http.conn.routing.HttpRoute;
+
+/**
+ * @author Woonduk Kang(emeroad)
+ */
+public class EndPointUtils {
+
+
+    public static String getHostAndPort(HttpRoute route) {
+        final HttpHost proxyHost = route.getProxyHost();
+        if (proxyHost != null) {
+            final String hostName = proxyHost.getHostName();
+            final int port = proxyHost.getPort();
+            if (port > 0) {
+                return hostAndPort(hostName, port);
+            }
+            return hostName;
+        } else {
+            final HttpHost targetHost = route.getTargetHost();
+            if (targetHost != null) {
+                final String hostName = targetHost.getHostName();
+                final int port = targetHost.getPort();
+                if (port > 0) {
+                    return hostAndPort(hostName, targetHost.getPort());
+                }
+                return hostName;
+            }
+        }
+        return "";
+    }
+
+    public static String hostAndPort(String host, int port) {
+        final StringBuilder sb = new StringBuilder();
+        sb.append(host);
+        sb.append(':');
+        sb.append(port);
+        return sb.toString();
+    }
+}

plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/AbstractHttpClientExecuteMethodInterceptor.java

@@ -41,9 +41,9 @@ public abstract class AbstractHttpClientExecuteMethodInterceptor implements Arou
     protected final boolean isDebug;
 
     private boolean isHasCallbackParam;
-    protected TraceContext traceContext;
-    protected MethodDescriptor descriptor;
-    protected InterceptorScope interceptorScope;
+    protected final TraceContext traceContext;
+    protected final MethodDescriptor descriptor;
+    protected final InterceptorScope interceptorScope;
 
     public AbstractHttpClientExecuteMethodInterceptor(Class<? extends AbstractHttpClientExecuteMethodInterceptor> childClazz, boolean isHasCallbackParam, TraceContext context, MethodDescriptor methodDescriptor, InterceptorScope interceptorScope) {
         this.logger = PLoggerFactory.getLogger(childClazz);

plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/DefaultClientExchangeHandlerImplStartMethodInterceptor.java

@@ -66,16 +66,16 @@ public class DefaultClientExchangeHandlerImplStartMethodInterceptor implements A
     private final PLogger logger = PLoggerFactory.getLogger(this.getClass());
     private final boolean isDebug = logger.isDebugEnabled();
 
-    private TraceContext traceContext;
-    private MethodDescriptor methodDescriptor;
+    private final TraceContext traceContext;
+    private final MethodDescriptor methodDescriptor;
 
-    private boolean param;
-    protected boolean cookie;
-    protected DumpType cookieDumpType;
+    private final boolean param;
+    protected final boolean cookie;
+    protected final DumpType cookieDumpType;
     protected SimpleSampler cookieSampler;
 
-    protected boolean entity;
-    protected DumpType entityDumpType;
+    protected final boolean entity;
+    protected final DumpType entityDumpType;
     protected SimpleSampler entitySampler;
 
     protected boolean statusCode;

plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/DefaultHttpRequestRetryHandlerRetryRequestMethodInterceptor.java

@@ -38,9 +38,15 @@ public class DefaultHttpRequestRetryHandlerRetryRequestMethodInterceptor impleme
 
     private final MethodDescriptor descriptor;
     private final TraceContext traceContext;
-    private ServiceType serviceType = HttpClient4Constants.HTTP_CLIENT_4_INTERNAL;
+    private final ServiceType serviceType = HttpClient4Constants.HTTP_CLIENT_4_INTERNAL;
 
     public DefaultHttpRequestRetryHandlerRetryRequestMethodInterceptor(TraceContext context, MethodDescriptor methodDescriptor) {
+        if (context == null) {
+            throw new NullPointerException("context must not be null");
+        }
+        if (methodDescriptor == null) {
+            throw new NullPointerException("methodDescriptor must not be null");
+        }
         this.traceContext = context;
         this.descriptor = methodDescriptor;
     }
@@ -51,8 +57,7 @@ public void before(Object target, Object[] args) {
             logger.beforeInterceptor(target, args);
         }
 
-        Trace trace = traceContext.currentTraceObject();
-
+        final Trace trace = traceContext.currentTraceObject();
         if (trace == null) {
             return;
         }
@@ -76,15 +81,9 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
             final SpanEventRecorder recorder = trace.currentSpanEventRecorder();
             recorder.recordApi(descriptor);
             recorder.recordException(throwable);
-            // arguments(final IOException exception, final int executionCount, final HttpContext context)
-            final StringBuilder sb = new StringBuilder();
-            if (args != null && args.length >= 1 && args[0] != null && args[0] instanceof Exception) {
-                sb.append(args[0].getClass().getName()).append(", ");
-            }
-            if (args != null && args.length >= 2 && args[1] != null && args[1] instanceof Integer) {
-                sb.append(args[1]);
-            }
-            recorder.recordAttribute(AnnotationKey.HTTP_INTERNAL_DISPLAY, sb.toString());
+
+            final String retryMessage = getRetryMessage(args);
+            recorder.recordAttribute(AnnotationKey.HTTP_INTERNAL_DISPLAY, retryMessage);
 
             if (result != null) {
                 recorder.recordAttribute(AnnotationKey.RETURN_DATA, result);
@@ -93,4 +92,19 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
             trace.traceBlockEnd();
         }
     }
+
+    private String getRetryMessage(Object[] args) {
+        if (args == null) {
+            return "";
+        }
+        // arguments(final IOException exception, final int executionCount, final HttpContext context)
+        final StringBuilder sb = new StringBuilder();
+        if (args.length >= 1 && args[0] instanceof Exception) {
+            sb.append(args[0].getClass().getName()).append(", ");
+        }
+        if (args.length >= 2 && args[1] instanceof Integer) {
+            sb.append(args[1]);
+        }
+        return sb.toString();
+    }
 }

plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/HttpClientConnectionManagerConnectMethodInterceptor.java

@@ -16,6 +16,7 @@
 
 package com.navercorp.pinpoint.plugin.httpclient4.interceptor;
 
+import com.navercorp.pinpoint.plugin.httpclient4.EndPointUtils;
 import org.apache.http.conn.routing.HttpRoute;
 
 import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
@@ -36,23 +37,10 @@ public HttpClientConnectionManagerConnectMethodInterceptor(TraceContext traceCon
 
     @Override
     protected void doInBeforeTrace(SpanEventRecorder recorder, Object target, Object[] args) {
-        if(args != null && args.length >= 2 && args[1] != null && args[1] instanceof HttpRoute) {
+        if (args != null && args.length >= 2 && args[1] instanceof HttpRoute) {
             final HttpRoute route = (HttpRoute) args[1];
-            final StringBuilder sb = new StringBuilder();
-            if(route.getProxyHost() != null) {
-                sb.append(route.getProxyHost().getHostName());
-                if(route.getProxyHost().getPort() > 0) {
-                    sb.append(":").append(route.getProxyHost().getPort());
-                }
-            } else {
-                if(route.getTargetHost() != null) {
-                    sb.append(route.getTargetHost().getHostName());
-                    if(route.getTargetHost().getPort() > 0) {
-                        sb.append(":").append(route.getTargetHost().getPort());
-                    }
-                }
-            }
-            recorder.recordAttribute(AnnotationKey.HTTP_INTERNAL_DISPLAY, sb.toString());
+            final String hostAndPort = EndPointUtils.getHostAndPort(route);
+            recorder.recordAttribute(AnnotationKey.HTTP_INTERNAL_DISPLAY, hostAndPort);
         }
         recorder.recordApi(methodDescriptor);
         recorder.recordServiceType(HttpClient4Constants.HTTP_CLIENT_4_INTERNAL);

plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/HttpClientExecuteMethodInternalInterceptor.java

@@ -67,14 +67,15 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
         }
 
         if (result != null && result instanceof HttpResponse) {
-            HttpResponse response = (HttpResponse) result;
+            final HttpResponse response = (HttpResponse) result;
             if (response.getStatusLine() != null) {
                 HttpCallContext context = new HttpCallContext();
                 final StatusLine statusLine = response.getStatusLine();
                 if (statusLine != null) {
                     context.setStatusCode(statusLine.getStatusCode());
-                    InterceptorScopeInvocation transaction = interceptorScope.getCurrentInvocation();
-                    if(transaction != null && transaction.getAttachment() == null) {
+                    final InterceptorScopeInvocation transaction = interceptorScope.getCurrentInvocation();
+                    final Object attachment = getAttachment(transaction);
+                    if (attachment == null) {
                         transaction.setAttachment(context);
                     }
                 }
@@ -97,11 +98,19 @@ private boolean needGetStatusCode() {
 //            return false;
 //        }
 
-        InterceptorScopeInvocation transaction = interceptorScope.getCurrentInvocation();
-        if (transaction != null && transaction.getAttachment() != null && transaction.getAttachment() instanceof HttpCallContext) {
+        final InterceptorScopeInvocation transaction = interceptorScope.getCurrentInvocation();
+        final Object attachment = getAttachment(transaction);
+        if (attachment instanceof HttpCallContext) {
             return false;
         }
 
         return true;
     }
+
+    private Object getAttachment(InterceptorScopeInvocation invocation) {
+        if (invocation == null) {
+            return null;
+        }
+        return invocation.getAttachment();
+    }
 }

plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/HttpRequestExecutorDoSendRequestAndDoReceiveResponseMethodInterceptor.java

@@ -57,9 +57,10 @@ public void before(Object target, Object[] args) {
         }
 
         final InterceptorScopeInvocation invocation = interceptorScope.getCurrentInvocation();
-        if(invocation != null && invocation.getAttachment() != null && invocation.getAttachment() instanceof HttpCallContext) {
-            HttpCallContext callContext = (HttpCallContext) invocation.getAttachment();
-            if(methodDescriptor.getMethodName().equals("doSendRequest")) {
+        final Object attachment = getAttachment(invocation);
+        if (attachment instanceof HttpCallContext) {
+            HttpCallContext callContext = (HttpCallContext) attachment;
+            if (methodDescriptor.getMethodName().equals("doSendRequest")) {
                 callContext.setWriteBeginTime(System.currentTimeMillis());
             } else {
                 callContext.setReadBeginTime(System.currentTimeMillis());
@@ -82,18 +83,28 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
         }
 
         final InterceptorScopeInvocation invocation = interceptorScope.getCurrentInvocation();
-        if(invocation != null && invocation.getAttachment() != null && invocation.getAttachment() instanceof HttpCallContext) {
-            HttpCallContext callContext = (HttpCallContext) invocation.getAttachment();
-            if(methodDescriptor.getMethodName().equals("doSendRequest")) {
+        final Object attachment = getAttachment(invocation);
+        if (attachment instanceof HttpCallContext) {
+            HttpCallContext callContext = (HttpCallContext) attachment;
+            if (methodDescriptor.getMethodName().equals("doSendRequest")) {
                 callContext.setWriteEndTime(System.currentTimeMillis());
                 callContext.setWriteFail(throwable != null);
             } else {
                 callContext.setReadEndTime(System.currentTimeMillis());
                 callContext.setReadFail(throwable != null);
             }
-            if(isDebug) {
+            if (isDebug) {
                 logger.debug("Set call context {}", callContext);
             }
         }
     }
+
+    private Object getAttachment(InterceptorScopeInvocation invocation) {
+        if (invocation == null) {
+            return null;
+        }
+        return invocation.getAttachment();
+    }
+
+
 }

plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/HttpRequestExecutorExecuteMethodInterceptor.java

@@ -22,6 +22,7 @@
 import com.navercorp.pinpoint.bootstrap.interceptor.scope.InterceptorScopeInvocation;
 import com.navercorp.pinpoint.common.Charsets;
 import com.navercorp.pinpoint.common.util.StringUtils;
+import com.navercorp.pinpoint.plugin.httpclient4.CommandContextFormatter;
 import com.navercorp.pinpoint.plugin.httpclient4.HttpCallContext;
 import com.navercorp.pinpoint.plugin.httpclient4.HttpCallContextFactory;
 import com.navercorp.pinpoint.plugin.httpclient4.HttpClient4PluginConfig;
@@ -169,8 +170,9 @@ private HttpRequest getHttpRequest(Object[] args) {
 
     private NameIntValuePair<String> getHost() {
         final InterceptorScopeInvocation transaction = interceptorScope.getCurrentInvocation();
-        if (transaction != null && transaction.getAttachment() != null && transaction.getAttachment() instanceof  HttpCallContext) {
-            HttpCallContext callContext = (HttpCallContext) transaction.getAttachment();
+        final Object attachment = getAttachment(transaction);
+        if (attachment instanceof  HttpCallContext) {
+            HttpCallContext callContext = (HttpCallContext) attachment;
             return new NameIntValuePair<String>(callContext.getHost(), callContext.getPort());
         }
 
@@ -217,20 +219,13 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
             recorder.recordException(throwable);
 
             final InterceptorScopeInvocation invocation = interceptorScope.getCurrentInvocation();
-            if (invocation != null && invocation.getAttachment() != null && invocation.getAttachment() instanceof  HttpCallContext) {
-                final HttpCallContext callContext = (HttpCallContext) invocation.getAttachment();
+            final Object attachment = getAttachment(invocation);
+            if (attachment instanceof HttpCallContext) {
+                final HttpCallContext callContext = (HttpCallContext) attachment;
                 logger.debug("Check call context {}", callContext);
                 if (io) {
-                    final StringBuilder sb = new StringBuilder();
-                    sb.append("write=").append(callContext.getWriteElapsedTime());
-                    if (callContext.isWriteFail()) {
-                        sb.append("(fail)");
-                    }
-                    sb.append(", read=").append(callContext.getReadElapsedTime());
-                    if (callContext.isReadFail()) {
-                        sb.append("(fail)");
-                    }
-                    recorder.recordAttribute(AnnotationKey.HTTP_IO, sb.toString());
+                    final String commandContextString = CommandContextFormatter.format(callContext);
+                    recorder.recordAttribute(AnnotationKey.HTTP_IO, commandContextString);
                 }
                 // clear
                 invocation.removeAttachment();
@@ -241,6 +236,13 @@ public void after(Object target, Object[] args, Object result, Throwable throwab
         }
     }
 
+    private Object getAttachment(InterceptorScopeInvocation invocation) {
+        if (invocation == null) {
+            return null;
+        }
+        return invocation.getAttachment();
+    }
+
     private Integer getStatusCode(Object result) {
         return getStatusCodeFromResponse(result);
     }