Repository: pinpoint-apm/pinpoint
Issue: Reduce trace scope for asynchronous calls made by the Thrift client
Labels: enhancement
module:plugin

The current implementation of the asynchronous tracing model cannot guarantee the closure of  _AsyncTrace_ unless it spans a single method.

For tracing asynchronous calls made by Thrift clients, the _AsyncTrace_ begins in `TAsyncMethodCall.start()` and ends in one of `TAsyncMethodCall.doWritingRequestBody()`, `TAsyncMethodCall.doReadingResponseBody()`, or `TAsyncMethodCall.onError()`.

If in future versions of Thrift adds a new code path that passes through `TAsyncMethodCall.start()` but not the one of ending methods, the _AsyncTrace_ will be pushed(start) but not popped(end).

To prevent these situations from every becoming a problem, it might be a good idea to reduce the scope of _AsyncTrace_ to the ending methods. The trace data might not be as detailed, but it would be safer this way.


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/ThriftConstants.java b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/ThriftConstants.java
index acdf4ea59573..6f4c7fdf3dd2 100644
--- a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/ThriftConstants.java
+++ b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/ThriftConstants.java
@@ -51,8 +51,6 @@ private ThriftConstants() {
     public static final Pattern CLIENT_PATTERN = Pattern.compile("\\$Client");
     public static final Pattern ASYNC_METHOD_CALL_PATTERN = Pattern.compile("\\$AsyncClient\\$");
 
-    public static final String ATTRIBUTE_CONFIG = "thriftPluginConfig";
-
     // field names
     public static final String T_ASYNC_METHOD_CALL_FIELD_TRANSPORT = "transport";
     public static final String FRAME_BUFFER_FIELD_TRANS_ = "trans_";
@@ -60,11 +58,7 @@ private ThriftConstants() {
 
     // custom field injector (accessor) FQCN
     private static final String FIELD_ACCESSOR_BASE = "com.navercorp.pinpoint.plugin.thrift.field.accessor.";
-    public static final String FIELD_ACCESSOR_ASYNC_CALL_END_FLAG = FIELD_ACCESSOR_BASE + "AsyncCallEndFlagFieldAccessor";
-    public static final String FIELD_ACCESSOR_ASYNC_CALL_REMOTE_ADDRESS = FIELD_ACCESSOR_BASE + "AsyncCallRemoteAddressFieldAccessor";
     public static final String FIELD_ACCESSOR_ASYNC_MARKER_FLAG = FIELD_ACCESSOR_BASE + "AsyncMarkerFlagFieldAccessor";
-    public static final String FIELD_ACCESSOR_ASYNC_NEXT_SPAN_ID = FIELD_ACCESSOR_BASE + "AsyncNextSpanIdFieldAccessor";
-    public static final String FIELD_ACCESSOR_ASYNC_TRACE_ID = FIELD_ACCESSOR_BASE + "AsyncTraceIdFieldAccessor";
     public static final String FIELD_ACCESSOR_SERVER_MARKER_FLAG = FIELD_ACCESSOR_BASE + "ServerMarkerFlagFieldAccessor";
     public static final String FIELD_ACCESSOR_SOCKET_ADDRESS = FIELD_ACCESSOR_BASE + "SocketAddressFieldAccessor";
     public static final String FIELD_ACCESSOR_SOCKET = FIELD_ACCESSOR_BASE + "SocketFieldAccessor";
diff --git a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/ThriftPlugin.java b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/ThriftPlugin.java
index 4e031a3ee026..47afb2694ec3 100644
--- a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/ThriftPlugin.java
+++ b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/ThriftPlugin.java
@@ -19,6 +19,7 @@
 import java.security.ProtectionDomain;
 import java.util.List;
 
+import com.navercorp.pinpoint.bootstrap.async.AsyncTraceIdAccessor;
 import com.navercorp.pinpoint.bootstrap.instrument.InstrumentClass;
 import com.navercorp.pinpoint.bootstrap.instrument.InstrumentException;
 import com.navercorp.pinpoint.bootstrap.instrument.InstrumentMethod;
@@ -146,12 +147,8 @@ public byte[] doInTransform(Instrumentor instrumentor, ClassLoader loader, Strin
                                         ProtectionDomain protectionDomain, byte[] classfileBuffer) throws InstrumentException {
 
                 final InstrumentClass target = instrumentor.getInstrumentClass(loader, className, classfileBuffer);
+                target.addField(AsyncTraceIdAccessor.class.getName());
                 target.addField(ThriftConstants.FIELD_ACCESSOR_SOCKET_ADDRESS);
-                target.addField(ThriftConstants.FIELD_ACCESSOR_ASYNC_MARKER_FLAG);
-                target.addField(ThriftConstants.FIELD_ACCESSOR_ASYNC_TRACE_ID);
-                target.addField(ThriftConstants.FIELD_ACCESSOR_ASYNC_NEXT_SPAN_ID);
-                target.addField(ThriftConstants.FIELD_ACCESSOR_ASYNC_CALL_END_FLAG);
-                target.addField(ThriftConstants.FIELD_ACCESSOR_ASYNC_CALL_REMOTE_ADDRESS);
                 target.addGetter(ThriftConstants.FIELD_GETTER_T_NON_BLOCKING_TRANSPORT, ThriftConstants.T_ASYNC_METHOD_CALL_FIELD_TRANSPORT);
 
                 // TAsyncMethodCall(TAsyncClient, TProtocolFactory, TNonblockingTransport, AsyncMethodCallback<T>, boolean)
@@ -163,48 +160,6 @@ public byte[] doInTransform(Instrumentor instrumentor, ClassLoader loader, Strin
                     constructor.addInterceptor(interceptor);
                 }
 
-                // TAsyncMethodCall.start(Selector)
-                final InstrumentMethod start = target.getDeclaredMethod("start", "java.nio.channels.Selector");
-                if (start != null) {
-                    String interceptor = "com.navercorp.pinpoint.plugin.thrift.interceptor.client.async.TAsyncMethodCallStartInterceptor";
-                    start.addInterceptor(interceptor);
-                }
-
-                // TAsyncMethodCall.doConnecting(SelectionKey)
-                final InstrumentMethod doConnecting = target.getDeclaredMethod("doConnecting", "java.nio.channels.SelectionKey");
-                if (doConnecting != null) {
-                    String interceptor = "com.navercorp.pinpoint.plugin.thrift.interceptor.client.async.TAsyncMethodCallInternalMethodInterceptor";
-                    doConnecting.addInterceptor(interceptor);
-                }
-
-                // TAsyncMethodCall.doWritingRequestSize()
-                final InstrumentMethod doWritingRequestSize = target.getDeclaredMethod("doWritingRequestSize");
-                if (doWritingRequestSize != null) {
-                    String interceptor = "com.navercorp.pinpoint.plugin.thrift.interceptor.client.async.TAsyncMethodCallInternalMethodInterceptor";
-                    doWritingRequestSize.addInterceptor(interceptor);
-                }
-
-                // TAsyncMethodCall.doWritingRequestBody(SelectionKey)
-                final InstrumentMethod doWritingRequestBody = target.getDeclaredMethod("doWritingRequestBody", "java.nio.channels.SelectionKey");
-                if (doWritingRequestBody != null) {
-                    String interceptor = "com.navercorp.pinpoint.plugin.thrift.interceptor.client.async.TAsyncMethodCallDoWritingRequestBodyInterceptor";
-                    doWritingRequestBody.addInterceptor(interceptor);
-                }
-
-                // TAsyncMethodCall.doReadingResponseSize()
-                final InstrumentMethod doReadingResponseSize = target.getDeclaredMethod("doReadingResponseSize");
-                if (doReadingResponseSize != null) {
-                    String interceptor = "com.navercorp.pinpoint.plugin.thrift.interceptor.client.async.TAsyncMethodCallInternalMethodInterceptor";
-                    doReadingResponseSize.addInterceptor(interceptor);
-                }
-
-                // TAsyncMethodCall.doReadingResponseBody(SelectionKey)
-                final InstrumentMethod doReadingResponseBody = target.getDeclaredMethod("doReadingResponseBody", "java.nio.channels.SelectionKey");
-                if (doReadingResponseBody != null) {
-                    String interceptor = "com.navercorp.pinpoint.plugin.thrift.interceptor.client.async.TAsyncMethodCallDoReadingResponseBodyInterceptor";
-                    doReadingResponseBody.addInterceptor(interceptor);
-                }
-
                 // TAsyncMethodCall.cleanUpAndFireCallback(SelectionKey)
                 final InstrumentMethod cleanUpAndFireCallback = target.getDeclaredMethod("cleanUpAndFireCallback", "java.nio.channels.SelectionKey");
                 if (cleanUpAndFireCallback != null) {
diff --git a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncClientManagerCallInterceptor.java b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncClientManagerCallInterceptor.java
index 0a0458466919..b6d1637ede9e 100644
--- a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncClientManagerCallInterceptor.java
+++ b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncClientManagerCallInterceptor.java
@@ -20,6 +20,7 @@
 
 import java.net.SocketAddress;
 
+import com.navercorp.pinpoint.bootstrap.async.AsyncTraceIdAccessor;
 import com.navercorp.pinpoint.bootstrap.context.AsyncTraceId;
 import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
 import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder;
@@ -37,10 +38,8 @@
 import com.navercorp.pinpoint.plugin.thrift.ThriftConstants;
 import com.navercorp.pinpoint.plugin.thrift.ThriftRequestProperty;
 import com.navercorp.pinpoint.plugin.thrift.ThriftUtils;
-import com.navercorp.pinpoint.plugin.thrift.field.accessor.AsyncCallRemoteAddressFieldAccessor;
-import com.navercorp.pinpoint.plugin.thrift.field.accessor.AsyncNextSpanIdFieldAccessor;
-import com.navercorp.pinpoint.plugin.thrift.field.accessor.AsyncTraceIdFieldAccessor;
 import com.navercorp.pinpoint.plugin.thrift.field.accessor.SocketAddressFieldAccessor;
+import org.apache.thrift.async.TAsyncMethodCall;
 
 /**
  * @author HyunGil Jeong
@@ -90,8 +89,6 @@ public void before(Object target, Object[] args) {
                 // inject async trace info to AsyncMethodCall object
                 final AsyncTraceId asyncTraceId = injectAsyncTraceId(asyncMethodCallObj, trace);
 
-                recorder.recordServiceType(ThriftConstants.THRIFT_CLIENT_INTERNAL);
-
                 // retrieve connection information
                 String remoteAddress = getRemoteAddress(asyncMethodCallObj);
 
@@ -109,8 +106,14 @@ public void before(Object target, Object[] args) {
                 parentTraceInfo.setParentApplicationType(this.traceContext.getServerTypeCode());
                 parentTraceInfo.setAcceptorHost(remoteAddress);
 
-                ((AsyncCallRemoteAddressFieldAccessor)asyncMethodCallObj)._$PINPOINT$_setAsyncCallRemoteAddress(remoteAddress);
-                ((AsyncNextSpanIdFieldAccessor)asyncMethodCallObj)._$PINPOINT$_setAsyncNextSpanId(nextSpanId);
+
+                recorder.recordServiceType(ThriftConstants.THRIFT_CLIENT);
+                recorder.recordNextSpanId(nextSpanId);
+                recorder.recordDestinationId(remoteAddress);
+
+                String methodUri = ThriftUtils.getAsyncMethodCallName((TAsyncMethodCall<?>) asyncMethodCallObj);
+                String thriftUrl = remoteAddress + "/" + methodUri;
+                recorder.recordAttribute(ThriftConstants.THRIFT_URL, thriftUrl);
             }
             InterceptorScopeInvocation currentTransaction = this.scope.getCurrentInvocation();
             currentTransaction.setAttachment(parentTraceInfo);
@@ -154,23 +157,9 @@ private boolean validate(final Object target, final Object[] args) {
             return false;
         }
 
-        if (!(asyncMethodCallObj instanceof AsyncTraceIdFieldAccessor)) {
-            if (isDebug) {
-                logger.debug("Invalid target object. Need field accessor({}).", AsyncTraceIdFieldAccessor.class.getName());
-            }
-            return false;
-        }
-
-        if (!(asyncMethodCallObj instanceof AsyncNextSpanIdFieldAccessor)) {
-            if (isDebug) {
-                logger.debug("Invalid target object. Need field accessor({}).", AsyncNextSpanIdFieldAccessor.class.getName());
-            }
-            return false;
-        }
-
-        if (!(asyncMethodCallObj instanceof AsyncCallRemoteAddressFieldAccessor)) {
+        if (!(asyncMethodCallObj instanceof AsyncTraceIdAccessor)) {
             if (isDebug) {
-                logger.debug("Invalid target object. Need field accessor({}).", AsyncCallRemoteAddressFieldAccessor.class.getName());
+                logger.debug("Invalid target object. Need field accessor({}).", AsyncTraceIdAccessor.class.getName());
             }
             return false;
         }
@@ -182,7 +171,7 @@ private AsyncTraceId injectAsyncTraceId(final Object asyncMethodCallObj, final T
         final AsyncTraceId asyncTraceId = trace.getAsyncTraceId();
         SpanEventRecorder recorder = trace.currentSpanEventRecorder();
         recorder.recordNextAsyncId(asyncTraceId.getAsyncId());
-        ((AsyncTraceIdFieldAccessor)asyncMethodCallObj)._$PINPOINT$_setAsyncTraceId(asyncTraceId);
+        ((AsyncTraceIdAccessor) asyncMethodCallObj)._$PINPOINT$_setAsyncTraceId(asyncTraceId);
         if (isDebug) {
             logger.debug("Set asyncTraceId metadata {}", asyncTraceId);
         }
@@ -194,7 +183,7 @@ private String getRemoteAddress(Object asyncMethodCallObj) {
             return ThriftConstants.UNKNOWN_ADDRESS;
         }
         SocketAddress socketAddress = ((SocketAddressFieldAccessor)asyncMethodCallObj)._$PINPOINT$_getSocketAddress();
-        return ThriftUtils.getHostPort((SocketAddress)socketAddress);
+        return ThriftUtils.getHostPort(socketAddress);
     }
 
 }
diff --git a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallCleanUpAndFireCallbackInterceptor.java b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallCleanUpAndFireCallbackInterceptor.java
index 4e02b6c62825..432ca2c38381 100644
--- a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallCleanUpAndFireCallbackInterceptor.java
+++ b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallCleanUpAndFireCallbackInterceptor.java
@@ -1,56 +1,47 @@
-/*
- * Copyright 2015 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.thrift.interceptor.client.async;
-
-import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
-import com.navercorp.pinpoint.bootstrap.context.TraceContext;
-import com.navercorp.pinpoint.plugin.thrift.field.accessor.AsyncCallEndFlagFieldAccessor;
-
-/**
- * @author HyunGil Jeong
- */
-public class TAsyncMethodCallCleanUpAndFireCallbackInterceptor extends TAsyncMethodCallInternalMethodInterceptor {
-    
-    public TAsyncMethodCallCleanUpAndFireCallbackInterceptor(
-            TraceContext traceContext,
-            MethodDescriptor methodDescriptor) {
-        super(traceContext, methodDescriptor);
-    }
-
-    @Override
-    public void after(Object target, Object[] args, Object result, Throwable throwable) {
-        super.after(target, args, result, throwable);
-
-        // Set a flag to end async trace block if this method completed successfully
-        if (throwable != null) {
-            return;
-        }
-        ((AsyncCallEndFlagFieldAccessor)target)._$PINPOINT$_setAsyncCallEndFlag(true);
-    }
-
-    @Override
-    protected boolean validate(Object target) {
-        if (!(target instanceof AsyncCallEndFlagFieldAccessor)) {
-            if (isDebug) {
-                logger.debug("Invalid target object. Need field accessor({}).", AsyncCallEndFlagFieldAccessor.class.getName());
-            }
-            return false;
-        }
-        return super.validate(target);
-    }
-    
-}
+/*
+ * Copyright 2016 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.thrift.interceptor.client.async;
+
+import com.navercorp.pinpoint.bootstrap.context.AsyncTraceId;
+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.SpanAsyncEventSimpleAroundInterceptor;
+import com.navercorp.pinpoint.plugin.thrift.ThriftConstants;
+
+/**
+ * @author HyunGil Jeong
+ */
+public class TAsyncMethodCallCleanUpAndFireCallbackInterceptor extends SpanAsyncEventSimpleAroundInterceptor {
+
+    public TAsyncMethodCallCleanUpAndFireCallbackInterceptor(TraceContext traceContext, MethodDescriptor methodDescriptor) {
+        super(traceContext, methodDescriptor);
+    }
+
+    @Override
+    protected void doInBeforeTrace(SpanEventRecorder recorder, AsyncTraceId asyncTraceId, Object target, Object[] args) {
+        // Do nothing
+    }
+
+    @Override
+    protected void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[] args, Object result, Throwable throwable) {
+        recorder.recordServiceType(ThriftConstants.THRIFT_CLIENT_INTERNAL);
+        recorder.recordApi(methodDescriptor);
+        recorder.recordException(throwable);
+    }
+
+}
diff --git a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallDoReadingResponseBodyInterceptor.java b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallDoReadingResponseBodyInterceptor.java
deleted file mode 100644
index db9a6810a235..000000000000
--- a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallDoReadingResponseBodyInterceptor.java
+++ /dev/null
@@ -1,68 +0,0 @@
-/*
- * Copyright 2015 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.thrift.interceptor.client.async;
-
-import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
-import com.navercorp.pinpoint.bootstrap.context.Trace;
-import com.navercorp.pinpoint.bootstrap.context.TraceContext;
-import com.navercorp.pinpoint.plugin.thrift.field.accessor.AsyncCallEndFlagFieldAccessor;
-
-/**
- * @author HyunGil Jeong
- */
-public class TAsyncMethodCallDoReadingResponseBodyInterceptor extends TAsyncMethodCallInternalMethodInterceptor {
-
-    public TAsyncMethodCallDoReadingResponseBodyInterceptor(TraceContext traceContext, MethodDescriptor methodDescriptor) {
-        super(traceContext, methodDescriptor);
-    }
-
-    @Override
-    public void after(Object target, Object[] args, Object result, Throwable throwable) {
-        super.after(target, args, result, throwable);
-
-        // End async trace block if TAsyncMethodCall.cleanUpAndFireCallback(...) call completed successfully
-        // if there was an exception, TAsyncMethodCall.onError(...) will be called and the async trace block will be ended there
-        if (throwable != null) {
-            return;
-        }
-        boolean endAsyncBlock = ((AsyncCallEndFlagFieldAccessor)target)._$PINPOINT$_getAsyncCallEndFlag();
-        if (endAsyncBlock) {
-            final Trace trace = super.traceContext.currentTraceObject();
-            // shouldn't be null
-            if (trace == null) {
-                return;
-            }
-
-            if (trace.isAsync() && trace.isRootStack()) {
-                trace.close();
-                super.traceContext.removeTraceObject();
-            }
-        }
-    }
-
-    @Override
-    protected boolean validate(Object target) {
-        if (!(target instanceof AsyncCallEndFlagFieldAccessor)) {
-            if (isDebug) {
-                logger.debug("Invalid target object. Need field accessor({}).", AsyncCallEndFlagFieldAccessor.class.getName());
-            }
-            return false;
-        }
-        return super.validate(target);
-    }
-
-}
diff --git a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallDoWritingRequestBodyInterceptor.java b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallDoWritingRequestBodyInterceptor.java
deleted file mode 100644
index ac2bc37fcd71..000000000000
--- a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallDoWritingRequestBodyInterceptor.java
+++ /dev/null
@@ -1,107 +0,0 @@
-/*
- * Copyright 2015 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.thrift.interceptor.client.async;
-
-import com.navercorp.pinpoint.plugin.thrift.ThriftConstants;
-import org.apache.thrift.async.TAsyncMethodCall;
-
-import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
-import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder;
-import com.navercorp.pinpoint.bootstrap.context.Trace;
-import com.navercorp.pinpoint.bootstrap.context.TraceContext;
-import com.navercorp.pinpoint.common.trace.ServiceType;
-import com.navercorp.pinpoint.plugin.thrift.ThriftUtils;
-import com.navercorp.pinpoint.plugin.thrift.field.accessor.AsyncCallEndFlagFieldAccessor;
-import com.navercorp.pinpoint.plugin.thrift.field.accessor.AsyncCallRemoteAddressFieldAccessor;
-import com.navercorp.pinpoint.plugin.thrift.field.accessor.AsyncNextSpanIdFieldAccessor;
-
-/**
- * @author HyunGil Jeong
- */
-public class TAsyncMethodCallDoWritingRequestBodyInterceptor extends TAsyncMethodCallInternalMethodInterceptor {
-
-    public TAsyncMethodCallDoWritingRequestBodyInterceptor(TraceContext traceContext, MethodDescriptor methodDescriptor) {
-        super(traceContext, methodDescriptor);
-    }
-
-    @Override
-    protected void doInBeforeTrace(SpanEventRecorder recorder, Object target, Object[] args) {
-        super.doInBeforeTrace(recorder, target, args);
-
-        Long nextSpanId = ((AsyncNextSpanIdFieldAccessor)target)._$PINPOINT$_getAsyncNextSpanId();
-        recorder.recordNextSpanId(nextSpanId);
-
-        String remoteAddress = ((AsyncCallRemoteAddressFieldAccessor)target)._$PINPOINT$_getAsyncCallRemoteAddress();
-        recorder.recordDestinationId(remoteAddress);
-
-        String methodUri = ThriftUtils.getAsyncMethodCallName((TAsyncMethodCall<?>)target);
-        String thriftUrl = remoteAddress + "/" + methodUri;
-        recorder.recordAttribute(ThriftConstants.THRIFT_URL, thriftUrl);
-    }
-
-    @Override
-    public void after(Object target, Object[] args, Object result, Throwable throwable) {
-        super.after(target, args, result, throwable);
-
-        // End async trace block if TAsyncMethodCall.cleanUpAndFireCallback(...) call completed successfully
-        // if there was an exception, TAsyncMethodCall.onError(...) will be called and the async trace block will be ended there
-        if (throwable != null) {
-            return;
-        }
-        boolean endAsyncBlock = ((AsyncCallEndFlagFieldAccessor)target)._$PINPOINT$_getAsyncCallEndFlag();
-        if (endAsyncBlock) {
-            final Trace trace = super.traceContext.currentTraceObject();
-            // shouldn't be null
-            if (trace == null) {
-                return;
-            }
-
-            if (trace.isAsync() && trace.isRootStack()) {
-                trace.close();
-                super.traceContext.removeTraceObject();
-            }
-        }
-    }
-
-    @Override
-    protected boolean validate(Object target) {
-        if (!(target instanceof AsyncNextSpanIdFieldAccessor)) {
-            if (isDebug) {
-                logger.debug("Invalid target object. Need field accessor({}).", AsyncNextSpanIdFieldAccessor.class.getName());
-            }
-            return false;
-        }
-        if (!(target instanceof AsyncCallRemoteAddressFieldAccessor)) {
-            if (isDebug) {
-                logger.debug("Invalid target object. Need field accessor({}).", AsyncCallRemoteAddressFieldAccessor.class.getName());
-            }
-            return false;
-        }
-        if (!(target instanceof AsyncCallEndFlagFieldAccessor)) {
-            if (isDebug) {
-                logger.debug("Invalid target object. Need field accessor({}).", AsyncCallEndFlagFieldAccessor.class.getName());
-            }
-            return false;
-        }
-        return super.validate(target);
-    }
-
-    @Override
-    protected ServiceType getServiceType() {
-        return ThriftConstants.THRIFT_CLIENT;
-    }
-}
\ No newline at end of file
diff --git a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallInternalMethodInterceptor.java b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallInternalMethodInterceptor.java
deleted file mode 100644
index 34b5a189a3a6..000000000000
--- a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallInternalMethodInterceptor.java
+++ /dev/null
@@ -1,129 +0,0 @@
-/*
- * Copyright 2015 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.thrift.interceptor.client.async;
-
-import org.apache.thrift.async.TAsyncMethodCall;
-
-import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
-import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder;
-import com.navercorp.pinpoint.bootstrap.context.Trace;
-import com.navercorp.pinpoint.bootstrap.context.TraceContext;
-import com.navercorp.pinpoint.bootstrap.interceptor.AroundInterceptor;
-import com.navercorp.pinpoint.bootstrap.logging.PLogger;
-import com.navercorp.pinpoint.bootstrap.logging.PLoggerFactory;
-import com.navercorp.pinpoint.common.trace.ServiceType;
-import com.navercorp.pinpoint.plugin.thrift.ThriftConstants;
-import com.navercorp.pinpoint.plugin.thrift.field.accessor.AsyncMarkerFlagFieldAccessor;
-
-/**
- * @author HyunGil Jeong
- */
-public class TAsyncMethodCallInternalMethodInterceptor implements AroundInterceptor {
-    protected final PLogger logger = PLoggerFactory.getLogger(getClass());
-    protected final boolean isDebug = logger.isDebugEnabled();
-
-    protected final TraceContext traceContext;
-    protected final MethodDescriptor methodDescriptor;
-
-    public TAsyncMethodCallInternalMethodInterceptor(TraceContext traceContext, MethodDescriptor methodDescriptor) {
-        this.traceContext = traceContext;
-        this.methodDescriptor = methodDescriptor;
-    }
-
-    @Override
-    public void before(Object target, Object[] args) {
-        if (isDebug) {
-            logger.beforeInterceptor(target, args);
-        }
-
-        if (!validate(target)) {
-            return;
-        }
-
-        final Trace trace = traceContext.currentTraceObject();
-        if (trace == null) {
-            return;
-        }
-        try {
-            trace.traceBlockBegin();
-            SpanEventRecorder recorder = trace.currentSpanEventRecorder();
-            recorder.recordServiceType(getServiceType());
-            doInBeforeTrace(recorder, target, args);
-        } catch (Throwable th) {
-            if (logger.isWarnEnabled()) {
-                logger.warn("BEFORE. Caused:{}", th.getMessage(), th);
-            }
-        }
-    }
-
-    protected void doInBeforeTrace(SpanEventRecorder recorder, final Object target, final Object[] args) {
-    }
-
-    @Override
-    public void after(Object target, Object[] args, Object result, Throwable throwable) {
-        if (isDebug) {
-            logger.afterInterceptor(target, args, result, throwable);
-        }
-
-        if (!validate0(target)) {
-            return;
-        }
-
-        final Trace trace = traceContext.currentTraceObject();
-        if (trace == null) {
-            return;
-        }
-
-        try {
-            SpanEventRecorder recorder = trace.currentSpanEventRecorder();
-            recorder.recordApi(this.methodDescriptor);
-            recorder.recordException(throwable);
-            doInAfterTrace(recorder, target, args, result, throwable);
-        } catch (Throwable th) {
-            if (logger.isWarnEnabled()) {
-                logger.warn("AFTER error. Caused:{}", th.getMessage(), th);
-            }
-        } finally {
-            trace.traceBlockEnd();
-        }
-    }
-
-    protected void doInAfterTrace(SpanEventRecorder recorder, final Object target, final Object[] args, final Object result, Throwable throwable) {
-    }
-
-    private boolean validate0(Object target) {
-        if (!(target instanceof TAsyncMethodCall)) {
-            return false;
-        }
-        if (!(target instanceof AsyncMarkerFlagFieldAccessor)) {
-            if (isDebug) {
-                logger.debug("Invalid target object. Need field accessor({}).", AsyncMarkerFlagFieldAccessor.class.getName());
-            }
-            return false;
-        }
-        return validate(target);
-    }
-
-    protected boolean validate(Object target) {
-        return true;
-    }
-
-    protected ServiceType getServiceType() {
-        return ThriftConstants.THRIFT_CLIENT_INTERNAL;
-    }
-
-}
diff --git a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallOnErrorInterceptor.java b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallOnErrorInterceptor.java
index 655dc72ff4a7..d1d1a9185149 100644
--- a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallOnErrorInterceptor.java
+++ b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallOnErrorInterceptor.java
@@ -1,58 +1,51 @@
-/*
- * Copyright 2015 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.thrift.interceptor.client.async;
-
-import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
-import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder;
-import com.navercorp.pinpoint.bootstrap.context.Trace;
-import com.navercorp.pinpoint.bootstrap.context.TraceContext;
-import com.navercorp.pinpoint.common.trace.AnnotationKey;
-
-/**
- * @author HyunGil Jeong
- */
-public class TAsyncMethodCallOnErrorInterceptor extends TAsyncMethodCallInternalMethodInterceptor {
-
-    public TAsyncMethodCallOnErrorInterceptor(TraceContext traceContext, MethodDescriptor methodDescriptor) {
-        super(traceContext, methodDescriptor);
-    }
-
-    @Override
-    public void after(Object target, Object[] args, Object result, Throwable throwable) {
-        super.after(target, args, result, throwable);
-
-        // End async trace block
-        final Trace trace = super.traceContext.currentTraceObject();
-        // shouldn't be null
-        if (trace == null) {
-            return;
-        }
-
-        if (trace.isAsync() && trace.isRootStack()) {
-            trace.close();
-            super.traceContext.removeTraceObject();
-        }
-    }
-
-    @Override
-    public void doInBeforeTrace(SpanEventRecorder recorder, final Object target, final Object[] args) {
-        if (args.length == 1) {
-            recorder.recordAttribute(AnnotationKey.ARGS0, args[0]);
-        }
-    }
-
-}
+/*
+ * Copyright 2016 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.thrift.interceptor.client.async;
+
+import com.navercorp.pinpoint.bootstrap.context.AsyncTraceId;
+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.SpanAsyncEventSimpleAroundInterceptor;
+import com.navercorp.pinpoint.common.trace.AnnotationKey;
+import com.navercorp.pinpoint.plugin.thrift.ThriftConstants;
+
+/**
+ * @author HyunGil Jeong
+ */
+public class TAsyncMethodCallOnErrorInterceptor extends SpanAsyncEventSimpleAroundInterceptor {
+
+    public TAsyncMethodCallOnErrorInterceptor(TraceContext traceContext, MethodDescriptor methodDescriptor) {
+        super(traceContext, methodDescriptor);
+    }
+
+    @Override
+    protected void doInBeforeTrace(SpanEventRecorder recorder, AsyncTraceId asyncTraceId, Object target, Object[] args) {
+        // Do nothing
+    }
+
+    @Override
+    protected void doInAfterTrace(SpanEventRecorder recorder, Object target, Object[] args, Object result, Throwable throwable) {
+        if (args != null && args.length == 1) {
+            recorder.recordAttribute(AnnotationKey.ARGS0, args[0]);
+        }
+        recorder.recordServiceType(ThriftConstants.THRIFT_CLIENT_INTERNAL);
+        recorder.recordApi(methodDescriptor);
+        recorder.recordException(throwable);
+    }
+
+}
diff --git a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallStartInterceptor.java b/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallStartInterceptor.java
deleted file mode 100644
index 1405f5e4764a..000000000000
--- a/plugins/thrift/src/main/java/com/navercorp/pinpoint/plugin/thrift/interceptor/client/async/TAsyncMethodCallStartInterceptor.java
+++ /dev/null
@@ -1,82 +0,0 @@
-/*
- * Copyright 2015 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.thrift.interceptor.client.async;
-
-import com.navercorp.pinpoint.bootstrap.context.AsyncTraceId;
-import com.navercorp.pinpoint.bootstrap.context.MethodDescriptor;
-import com.navercorp.pinpoint.bootstrap.context.SpanEventRecorder;
-import com.navercorp.pinpoint.bootstrap.context.Trace;
-import com.navercorp.pinpoint.bootstrap.context.TraceContext;
-import com.navercorp.pinpoint.common.trace.ServiceType;
-import com.navercorp.pinpoint.plugin.thrift.descriptor.ThriftAsyncClientMethodDescriptor;
-import com.navercorp.pinpoint.plugin.thrift.field.accessor.AsyncMarkerFlagFieldAccessor;
-import com.navercorp.pinpoint.plugin.thrift.field.accessor.AsyncTraceIdFieldAccessor;
-
-/**
- * @author HyunGil Jeong
- */
-public class TAsyncMethodCallStartInterceptor extends TAsyncMethodCallInternalMethodInterceptor {
-
-    private final ThriftAsyncClientMethodDescriptor thriftAsyncClientMethodDescriptor = new ThriftAsyncClientMethodDescriptor();
-
-    public TAsyncMethodCallStartInterceptor(TraceContext traceContext, MethodDescriptor methodDescriptor) {
-        super(traceContext, methodDescriptor);
-        this.traceContext.cacheApi(this.thriftAsyncClientMethodDescriptor);
-    }
-
-    @Override
-    public void before(Object target, Object[] args) {
-        if (!validate0(target)) {
-            return;
-        }
-
-        // Retrieve asyncTraceId
-        final AsyncTraceId asyncTraceId = ((AsyncTraceIdFieldAccessor)target)._$PINPOINT$_getAsyncTraceId();
-        Trace trace = traceContext.currentTraceObject();
-        // Check if the method was invoked by the same thread or not, probabaly safe not to check but just in case.
-        if (trace == null) {
-            // another thread has invoked the method
-            trace = traceContext.continueAsyncTraceObject(asyncTraceId, asyncTraceId.getAsyncId(), asyncTraceId.getSpanStartTime());
-            if (trace == null) {
-                logger.warn("Failed to continue async trace. 'result is null'");
-                return;
-            }
-            SpanEventRecorder recorder = trace.currentSpanEventRecorder();
-            recorder.recordServiceType(ServiceType.ASYNC);
-            recorder.recordApi(this.thriftAsyncClientMethodDescriptor);
-            ((AsyncMarkerFlagFieldAccessor)target)._$PINPOINT$_setAsyncMarkerFlag(Boolean.TRUE);
-        }
-        super.before(target, args);
-    }
-
-    private boolean validate0(Object target) {
-        if (!(target instanceof AsyncTraceIdFieldAccessor)) {
-            if (isDebug) {
-                logger.debug("Invalid target object. Need field accessor({}).", AsyncTraceIdFieldAccessor.class.getName());
-            }
-            return false;
-        }
-        if (!(target instanceof AsyncMarkerFlagFieldAccessor)) {
-            if (isDebug) {
-                logger.debug("Invalid target object. Need field accessor({}).", AsyncMarkerFlagFieldAccessor.class.getName());
-            }
-            return false;
-        }
-        return true;
-    }
-
-}


Test patch:
diff --git a/agent/src/test/java/com/navercorp/pinpoint/plugin/thrift/common/client/AsyncEchoTestClient.java b/agent/src/test/java/com/navercorp/pinpoint/plugin/thrift/common/client/AsyncEchoTestClient.java
index 56a3fd9442b0..6eb3bb1c20aa 100644
--- a/agent/src/test/java/com/navercorp/pinpoint/plugin/thrift/common/client/AsyncEchoTestClient.java
+++ b/agent/src/test/java/com/navercorp/pinpoint/plugin/thrift/common/client/AsyncEchoTestClient.java
@@ -22,7 +22,6 @@
 import java.lang.reflect.Method;
 import java.net.InetSocketAddress;
 import java.nio.channels.SelectionKey;
-import java.nio.channels.Selector;
 import java.util.concurrent.CountDownLatch;
 
 import org.apache.thrift.TBase;
@@ -85,42 +84,8 @@ public String echo(String message) throws TException {
     public void verifyTraces(PluginTestVerifier verifier, String expectedMessage) throws Exception {
         final InetSocketAddress actualServerAddress = this.environment.getServerAddress();
         // ********** Asynchronous Traces
-        // SpanEvent - Thrift Asynchronous Client Invocation
-        ExpectedTrace asyncClientInvocationTrace = event("ASYNC", "Thrift Asynchronous Client Invocation");
-
-        // SpanEvent - TAsyncMethodCall.start
-        Method start = TAsyncMethodCall.class.getDeclaredMethod("start", Selector.class);
-        ExpectedTrace startTrace = event("THRIFT_CLIENT_INTERNAL", start);
-
-        // SpanEvent - TAsyncMethodCall.doConnecting
-        Method doConnecting = TAsyncMethodCall.class.getDeclaredMethod("doConnecting", SelectionKey.class);
-        ExpectedTrace doConnectingTrace = event("THRIFT_CLIENT_INTERNAL", doConnecting);
-
-        // SpanEvent - TAsyncMethodCall.doWritingRequestSize
-        Method doWritingRequestSize = TAsyncMethodCall.class.getDeclaredMethod("doWritingRequestSize");
-        ExpectedTrace doWritingRequestSizeTrace = event("THRIFT_CLIENT_INTERNAL", doWritingRequestSize);
-
-        // SpanEvent - TAsyncMethodCall.doWritingRequestBody
-        Method doWritingRequestBody = TAsyncMethodCall.class.getDeclaredMethod("doWritingRequestBody",
-                SelectionKey.class);
-        ExpectedAnnotation thriftUrl = Expectations.annotation("thrift.url", actualServerAddress.getHostName() + ":"
-                + actualServerAddress.getPort() + "/com/navercorp/pinpoint/plugin/thrift/dto/EchoService/echo_call");
-        ExpectedTrace doWritingRequestBodyTrace = event("THRIFT_CLIENT", // ServiceType
-                doWritingRequestBody, // Method
-                null, // rpc
-                null, // endPoint
-                actualServerAddress.getHostName() + ":" + actualServerAddress.getPort(), // destinationId
-                thriftUrl // Annotation("thrift.url")
-        );
-
-        // SpanEvent - TAsyncMethodCall.doReadingResponseSize
-        Method doReadingResponseSize = TAsyncMethodCall.class.getDeclaredMethod("doReadingResponseSize");
-        ExpectedTrace doReadingResponseSizeTrace = event("THRIFT_CLIENT_INTERNAL", doReadingResponseSize);
-
-        // SpanEvent - TAsyncMethodCall.doReadingResponseBody
-        Method doReadingResponseBody = TAsyncMethodCall.class.getDeclaredMethod("doReadingResponseBody",
-                SelectionKey.class);
-        ExpectedTrace doReadingResponseBodyTrace = event("THRIFT_CLIENT_INTERNAL", doReadingResponseBody);
+        // SpanEvent - Asynchronous Invocation
+        ExpectedTrace asyncInvocationTrace = event("ASYNC", "Asynchronous Invocation");
 
         // SpanEvent - TAsyncMethodCall.cleanUpAndFireCallback
         Method cleanUpAndFireCallback = TAsyncMethodCall.class.getDeclaredMethod("cleanUpAndFireCallback",
@@ -139,9 +104,16 @@ public void verifyTraces(PluginTestVerifier verifier, String expectedMessage) th
         // ********** Root trace for Asynchronous traces
         // SpanEvent - TAsyncClientManager.call
         Method call = TAsyncClientManager.class.getDeclaredMethod("call", TAsyncMethodCall.class);
-        verifier.verifyTrace(async(Expectations.event("THRIFT_CLIENT_INTERNAL", call), asyncClientInvocationTrace,
-                startTrace, doConnectingTrace, doWritingRequestSizeTrace, doWritingRequestBodyTrace,
-                doReadingResponseSizeTrace, doReadingResponseBodyTrace, cleanUpAndFireCallbackTrace, receiveBaseTrace));
+        ExpectedAnnotation thriftUrl = Expectations.annotation("thrift.url", actualServerAddress.getHostName() + ":"
+                + actualServerAddress.getPort() + "/com/navercorp/pinpoint/plugin/thrift/dto/EchoService/echo_call");
+        ExpectedTrace callTrace = event("THRIFT_CLIENT", // ServiceType
+                call, // Method
+                null, // rpc
+                null, // endPoint
+                actualServerAddress.getHostName() + ":" + actualServerAddress.getPort(), // destinationId
+                thriftUrl // Annotation("thrift.url")
+        );
+        verifier.verifyTrace(async(callTrace, asyncInvocationTrace, cleanUpAndFireCallbackTrace, receiveBaseTrace));
     }
 
     private static class AsyncEchoResultHolder {