Repository: pinpoint-apm/pinpoint
Issue: HttpClient4 plugin's BasicFutureMethodInterceptor failed to record exception detatils when intercepting method `org.apache.http.concurrent.BasicFuture#failed`
Labels: bug
module:plugin
module:agent

 #### contribution info
* Contributor : @tankilo 

#### Prerequisites
Please check the [FAQ](http://pinpoint-apm.github.io/pinpoint/faq.html), and search existing [issues](https://github.com/pinpoint-apm/pinpoint/issues) for similar questions before creating a new issue.**YOU MAY DELETE THIS PREREQUISITES SECTION.**

- [x] I have checked the [FAQ](http://pinpoint-apm.github.io/pinpoint/faq.html), and [issues](https://github.com/pinpoint-apm/pinpoint/issues) and found no answer.

#### What version of pinpoint are you using?
master

#### Describe the bug
HttpClient4 plugin's BasicFutureMethodInterceptor failed to record exception detatils when intercepting method `org.apache.http.concurrent.BasicFuture#failed`

![image](https://user-images.githubusercontent.com/10413284/103470276-b36ddc80-4dab-11eb-96a9-ce5e16ce0fb0.png)

#### What did you do to trigger the bug?

Run the demo code with pinpoint agent.
Of course ,i didn't run any server on endpoint `http://127.0.0.1:9200`.

```
public class TestHttpClient {
    public static void main(String[] args){

        RequestConfig requestConfig = RequestConfig.custom()
            .setConnectTimeout(50000)
            .setSocketTimeout(50000)
            .setConnectionRequestTimeout(1000)
            .build();

        IOReactorConfig ioReactorConfig = IOReactorConfig.custom().
            setIoThreadCount(Runtime.getRuntime().availableProcessors())
            .setSoKeepAlive(true)
            .build();

        ConnectingIOReactor ioReactor=null;
        try {
            ioReactor = new DefaultConnectingIOReactor(ioReactorConfig);
        } catch (IOReactorException e) {
            e.printStackTrace();
        }
        PoolingNHttpClientConnectionManager connManager = new PoolingNHttpClientConnectionManager(ioReactor);
        connManager.setMaxTotal(100);
        connManager.setDefaultMaxPerRoute(100);


        final CloseableHttpAsyncClient client = HttpAsyncClients.custom().
            setConnectionManager(connManager)
            .setDefaultRequestConfig(requestConfig)
            .build();

        String url = "http://127.0.0.1:9200/_bulk";
        HttpPost httpPost = new HttpPost(url);
        StringEntity entity = null;
        try {
            String a = "{ \"index\": { \"_index\": \"test\", \"_type\": \"test\"} }\n" +
                "{\"name\": \"上海\",\"age\":33}\n";
            entity = new StringEntity(a);
        } catch (UnsupportedEncodingException e) {
            e.printStackTrace();
        }
        httpPost.setEntity(entity);

//start
        client.start();

//异步请求
        client.execute(httpPost, new Back());

        while(true){
            try {
                TimeUnit.SECONDS.sleep(1);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }

    static class Back implements FutureCallback<HttpResponse>{

        private long start = System.currentTimeMillis();
        Back(){
        }

        public void completed(HttpResponse httpResponse) {
            try {
                System.out.println("cost is:"+(System.currentTimeMillis()-start)+":"+EntityUtils.toString(httpResponse.getEntity()));
            } catch (IOException e) {
                e.printStackTrace();
            }
        }

        public void failed(Exception e) {
            System.err.println(" cost is:"+(System.currentTimeMillis()-start)+":"+e);
        }

        public void cancelled() {

        }
    }
}
```

#### Expected behavior
![image](https://user-images.githubusercontent.com/10413284/103470263-70136e00-4dab-11eb-8ccb-ffde5563c651.png)

Exception details should be recorded
#### Screenshots
If applicable, add screenshots to help explain your problem.

#### Logs
If applicable, please attach agent/collector/web DEBUG log that includes the code execution that led to the bug. In case of agents, including the start-up log may be of great help.

#### Additional context
Add any other context about the problem here, such as affected library for agents, how your collector/web/hbase is set up if applicable.


Issue comments:
I will post a PR to fix it.
@emeroad Do you think this is a bug?
@tankilo 
Thanks for your contribution.

I fully agree with your opinion.
I really like this PR. :+1: 
Thank you for your contribution

Pull request review comments:

Patch that resolved the issue:
diff --git a/plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/HttpClient4Plugin.java b/plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/HttpClient4Plugin.java
index 99a6dddd5f53..28e723a09b67 100644
--- a/plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/HttpClient4Plugin.java
+++ b/plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/HttpClient4Plugin.java
@@ -30,6 +30,7 @@
 import com.navercorp.pinpoint.bootstrap.logging.PLoggerFactory;
 import com.navercorp.pinpoint.bootstrap.plugin.ProfilerPlugin;
 import com.navercorp.pinpoint.bootstrap.plugin.ProfilerPluginSetupContext;
+import com.navercorp.pinpoint.plugin.httpclient4.interceptor.BasicFutureFailedMethodInterceptor;
 import com.navercorp.pinpoint.plugin.httpclient4.interceptor.BasicFutureMethodInterceptor;
 import com.navercorp.pinpoint.plugin.httpclient4.interceptor.DefaultClientExchangeHandlerImplStartMethodInterceptor;
 import com.navercorp.pinpoint.plugin.httpclient4.interceptor.DefaultHttpRequestRetryHandlerRetryRequestMethodInterceptor;
@@ -340,7 +341,7 @@ public byte[] doInTransform(Instrumentor instrumentor, ClassLoader loader, Strin
 
             InstrumentMethod failed = target.getDeclaredMethod("failed", "java.lang.Exception");
             if (failed != null) {
-                failed.addInterceptor(BasicFutureMethodInterceptor.class);
+                failed.addInterceptor(BasicFutureFailedMethodInterceptor.class);
 
             }
 
diff --git a/plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/BasicFutureFailedMethodInterceptor.java b/plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/BasicFutureFailedMethodInterceptor.java
new file mode 100644
index 000000000000..6f52de039449
--- /dev/null
+++ b/plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/BasicFutureFailedMethodInterceptor.java
@@ -0,0 +1,52 @@
+/*
+ * Copyright 2014 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.interceptor;
+
+import com.navercorp.pinpoint.bootstrap.context.AsyncContext;
+import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
+import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder;
+import com.navercorp.pinpoint.bootstrap.context.TraceContext;
+import com.navercorp.pinpoint.bootstrap.interceptor.AsyncContextSpanEventSimpleAroundInterceptor;
+import com.navercorp.pinpoint.plugin.httpclient4.HttpClient4Constants;
+
+/**
+ * 
+ * @author netspider
+ * @author jaehong.kim
+ * 
+ */
+public class BasicFutureFailedMethodInterceptor extends AsyncContextSpanEventSimpleAroundInterceptor {
+
+    public BasicFutureFailedMethodInterceptor(TraceContext traceContext, MethodDescriptor methodDescriptor) {
+        super(traceContext, methodDescriptor);
+    }
+
+    @Override
+    protected void doInBeforeTrace(SpanEventRecorder recorder, AsyncContext asyncContext, Object target, Object[] args) {
+        recorder.recordServiceType(HttpClient4Constants.HTTP_CLIENT_4_INTERNAL);
+    }
+
+    @Override
+    protected void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[] args, Object result, Throwable throwable) {
+        recorder.recordApi(methodDescriptor);
+        if (throwable!= null) {
+            recorder.recordException(throwable);
+        } else if (args.length == 1 && args[0] instanceof Exception) {
+            recorder.recordException((Exception) args[0]);
+        }
+    }
+}
\ No newline at end of file
diff --git a/plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/BasicFutureMethodInterceptor.java b/plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/BasicFutureMethodInterceptor.java
index 37f2d361c3e2..4e45dfe0ca83 100644
--- a/plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/BasicFutureMethodInterceptor.java
+++ b/plugins/httpclient4/src/main/java/com/navercorp/pinpoint/plugin/httpclient4/interceptor/BasicFutureMethodInterceptor.java
@@ -43,6 +43,10 @@ protected void doInBeforeTrace(SpanEventRecorder recorder, AsyncContext asyncCon
     @Override
     protected void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[] args, Object result, Throwable throwable) {
         recorder.recordApi(methodDescriptor);
-        recorder.recordException(throwable);
+        if ("failed".equals(methodDescriptor.getMethodName()) && args.length == 1 && args[0] instanceof Exception) {
+            recorder.recordException((Exception) args[0]);
+        } else {
+            recorder.recordException(throwable);
+        }
     }
 }
\ No newline at end of file


Test patch: