diff --git a/CHANGES.md b/CHANGES.md index 49c9f6fd6..666235544 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -32,6 +32,7 @@ Release Notes. * Add plugin to support Apache HttpClient 5. * Format SpringMVC & Tomcat EntrySpan operation name to `METHOD:URI`. * Make `HTTP method` in the operation name according to runtime, rather than previous code-level definition, which used to have possibilities including multiple HTTP methods. +* Fix the bug that httpasyncclient-4.x-plugin does not take effect every time. #### Documentation diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/Constants.java b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/Constants.java new file mode 100644 index 000000000..b77057f2f --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/Constants.java @@ -0,0 +1,28 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.skywalking.apm.plugin.httpasyncclient.v4; + +import org.apache.http.protocol.HttpContext; + +public class Constants { + + public final static ThreadLocal HTTP_CONTEXT_LOCAL = new ThreadLocal<>(); + + public final static String SKYWALKING_CONTEXT_SNAPSHOT = "skywalking-context-snapshot"; + public final static String SKYWALKING_HTTP_CONTEXT = "skywalking-http-context"; +} diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncClientInterceptor.java b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncClientInterceptor.java index 5e5e5be1a..04d06ec2a 100644 --- a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncClientInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncClientInterceptor.java @@ -28,8 +28,6 @@ import org.apache.skywalking.apm.plugin.httpasyncclient.v4.wrapper.HttpAsyncResp import java.lang.reflect.Method; -import static org.apache.skywalking.apm.plugin.httpasyncclient.v4.SessionRequestCompleteInterceptor.CONTEXT_LOCAL; - /** * in main thread,hold the context in thread local so we can read in the same thread. */ @@ -43,7 +41,7 @@ public class HttpAsyncClientInterceptor implements InstanceMethodsAroundIntercep FutureCallback callback = (FutureCallback) allArguments[3]; allArguments[1] = new HttpAsyncResponseConsumerWrapper(consumer); allArguments[3] = new FutureCallbackWrapper(callback); - CONTEXT_LOCAL.set(context); + Constants.HTTP_CONTEXT_LOCAL.set(context); } @Override diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncRequestExecutorInterceptor.java b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncRequestExecutorInterceptor.java index f89409eac..15457634e 100644 --- a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncRequestExecutorInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncRequestExecutorInterceptor.java @@ -21,10 +21,12 @@ import org.apache.http.HttpHost; import org.apache.http.RequestLine; import org.apache.http.client.methods.HttpRequestWrapper; import org.apache.http.client.protocol.HttpClientContext; +import org.apache.http.nio.NHttpClientConnection; import org.apache.http.protocol.HttpContext; import org.apache.skywalking.apm.agent.core.context.CarrierItem; import org.apache.skywalking.apm.agent.core.context.ContextCarrier; import org.apache.skywalking.apm.agent.core.context.ContextManager; +import org.apache.skywalking.apm.agent.core.context.ContextSnapshot; import org.apache.skywalking.apm.agent.core.context.tag.Tags; import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer; @@ -38,8 +40,7 @@ import org.apache.skywalking.apm.util.StringUtil; import java.lang.reflect.Method; import java.net.URI; import java.net.URL; - -import static org.apache.skywalking.apm.plugin.httpasyncclient.v4.SessionRequestCompleteInterceptor.CONTEXT_LOCAL; +import java.util.Objects; /** * the actual point request begin fetch the request from thread local . @@ -49,9 +50,8 @@ public class HttpAsyncRequestExecutorInterceptor implements InstanceMethodsAroun @Override public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, MethodInterceptResult result) throws Throwable { - HttpContext context = CONTEXT_LOCAL.get(); - CONTEXT_LOCAL.remove(); - if (context == null) { + HttpContext context = findHttpContext(allArguments); + if (Objects.isNull(context)) { return; } final ContextCarrier contextCarrier = new ContextCarrier(); @@ -101,4 +101,31 @@ public class HttpAsyncRequestExecutorInterceptor implements InstanceMethodsAroun Tags.HTTP.PARAMS.set(span, tagValue); } } + + private HttpContext findHttpContext(Object[] allArguments) { + HttpContext context = Constants.HTTP_CONTEXT_LOCAL.get(); + Constants.HTTP_CONTEXT_LOCAL.remove(); + if (Objects.nonNull(context)) { + return context; + } + NHttpClientConnection conn = (NHttpClientConnection) allArguments[0]; + HttpContext contextInConn = conn.getContext(); + if (Objects.isNull(contextInConn)) { + return null; + } + context = (HttpContext) contextInConn.getAttribute(Constants.SKYWALKING_HTTP_CONTEXT); + conn.getContext().removeAttribute(Constants.SKYWALKING_HTTP_CONTEXT); + if (Objects.isNull(context)) { + return null; + } + ContextSnapshot snapshot = (ContextSnapshot) contextInConn.getAttribute(Constants.SKYWALKING_CONTEXT_SNAPSHOT); + conn.getContext().removeAttribute(Constants.SKYWALKING_CONTEXT_SNAPSHOT); + if (snapshot != null) { + AbstractSpan localSpan = ContextManager.createLocalSpan("HttpAsyncClient/local"); + localSpan.setComponent(ComponentsDefine.HTTP_ASYNC_CLIENT); + localSpan.setLayer(SpanLayer.HTTP); + ContextManager.continued(snapshot); + } + return context; + } } diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/LeaseRequestCompletedInterceptor.java b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/LeaseRequestCompletedInterceptor.java new file mode 100644 index 000000000..8b4bdc80c --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/LeaseRequestCompletedInterceptor.java @@ -0,0 +1,53 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.skywalking.apm.plugin.httpasyncclient.v4; + +import org.apache.http.nio.NHttpConnection; +import org.apache.http.pool.PoolEntry; +import org.apache.skywalking.apm.agent.core.context.ContextManager; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult; + +import java.lang.reflect.Method; + +public class LeaseRequestCompletedInterceptor implements InstanceMethodsAroundInterceptor { + + @Override + public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, + MethodInterceptResult result) throws Throwable { + PoolEntry entry = (PoolEntry) allArguments[0]; + NHttpConnection conn = (NHttpConnection) entry.getConnection(); + if (ContextManager.isActive()) { + conn.getContext().setAttribute(Constants.SKYWALKING_CONTEXT_SNAPSHOT, ContextManager.capture()); + conn.getContext().setAttribute(Constants.SKYWALKING_HTTP_CONTEXT, Constants.HTTP_CONTEXT_LOCAL.get()); + } + } + + @Override + public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, + Object ret) throws Throwable { + return ret; + } + + @Override + public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments, + Class[] argumentsTypes, Throwable t) { + + } +} diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestCompleteInterceptor.java b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestCompleteInterceptor.java index 31f503025..a74c0b7d8 100644 --- a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestCompleteInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestCompleteInterceptor.java @@ -20,9 +20,12 @@ package org.apache.skywalking.apm.plugin.httpasyncclient.v4; import org.apache.http.protocol.HttpContext; import org.apache.skywalking.apm.agent.core.context.ContextManager; import org.apache.skywalking.apm.agent.core.context.ContextSnapshot; +import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; +import org.apache.skywalking.apm.agent.core.context.trace.SpanLayer; import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance; import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceMethodsAroundInterceptor; import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInterceptResult; +import org.apache.skywalking.apm.network.trace.component.ComponentsDefine; import java.lang.reflect.Method; @@ -31,8 +34,6 @@ import java.lang.reflect.Method; */ public class SessionRequestCompleteInterceptor implements InstanceMethodsAroundInterceptor { - public static ThreadLocal CONTEXT_LOCAL = new ThreadLocal(); - @Override public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, MethodInterceptResult result) throws Throwable { @@ -41,11 +42,13 @@ public class SessionRequestCompleteInterceptor implements InstanceMethodsAroundI return; } ContextSnapshot snapshot = (ContextSnapshot) array[0]; - ContextManager.createLocalSpan("httpasyncclient/local"); + AbstractSpan localSpan = ContextManager.createLocalSpan("httpasyncclient/local"); + localSpan.setComponent(ComponentsDefine.HTTP_ASYNC_CLIENT); + localSpan.setLayer(SpanLayer.HTTP); if (snapshot != null) { ContextManager.continued(snapshot); } - CONTEXT_LOCAL.set((HttpContext) array[1]); + Constants.HTTP_CONTEXT_LOCAL.set((HttpContext) array[1]); } diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestConstructorInterceptor.java b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestConstructorInterceptor.java index 2cd33f60e..5abf5d97d 100644 --- a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestConstructorInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestConstructorInterceptor.java @@ -22,8 +22,6 @@ import org.apache.skywalking.apm.agent.core.context.ContextSnapshot; import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance; import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.InstanceConstructorInterceptor; -import static org.apache.skywalking.apm.plugin.httpasyncclient.v4.SessionRequestCompleteInterceptor.CONTEXT_LOCAL; - /** * hold the snapshot in SkyWalkingDynamicField */ @@ -32,15 +30,15 @@ public class SessionRequestConstructorInterceptor implements InstanceConstructor public void onConstruct(EnhancedInstance objInst, Object[] allArguments) { if (ContextManager.isActive()) { if (ContextManager.activeSpan().isExit()) { - CONTEXT_LOCAL.remove(); + Constants.HTTP_CONTEXT_LOCAL.remove(); return; } ContextSnapshot snapshot = ContextManager.capture(); objInst.setSkyWalkingDynamicField(new Object[] { snapshot, - CONTEXT_LOCAL.get() + Constants.HTTP_CONTEXT_LOCAL.get() }); } - CONTEXT_LOCAL.remove(); + Constants.HTTP_CONTEXT_LOCAL.remove(); } } diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestFailInterceptor.java b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestFailInterceptor.java index 085f4c04d..d80c86b06 100644 --- a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestFailInterceptor.java +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/SessionRequestFailInterceptor.java @@ -23,8 +23,6 @@ import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.MethodInt import java.lang.reflect.Method; -import static org.apache.skywalking.apm.plugin.httpasyncclient.v4.SessionRequestCompleteInterceptor.CONTEXT_LOCAL; - /** * when request fail to ready we should remove thread local in case of memory leak; */ @@ -34,7 +32,7 @@ public class SessionRequestFailInterceptor implements InstanceMethodsAroundInter public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class[] argumentsTypes, MethodInterceptResult result) throws Throwable { //this means actual request will not started. so the span has not been created,we cannot log the status. - CONTEXT_LOCAL.remove(); + Constants.HTTP_CONTEXT_LOCAL.remove(); objInst.setSkyWalkingDynamicField(null); } diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/define/LeaseRequestInstrumentation.java b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/define/LeaseRequestInstrumentation.java new file mode 100644 index 000000000..3c1e9db72 --- /dev/null +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/define/LeaseRequestInstrumentation.java @@ -0,0 +1,67 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 org.apache.skywalking.apm.plugin.httpasyncclient.v4.define; + +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.matcher.ElementMatcher; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.ConstructorInterceptPoint; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.InstanceMethodsInterceptPoint; +import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.ClassInstanceMethodsEnhancePluginDefine; +import org.apache.skywalking.apm.agent.core.plugin.match.ClassMatch; + +import static net.bytebuddy.matcher.ElementMatchers.named; +import static org.apache.skywalking.apm.agent.core.plugin.match.NameMatch.byName; + +public class LeaseRequestInstrumentation extends ClassInstanceMethodsEnhancePluginDefine { + + private static final String ENHANCE_CLASS = "org.apache.http.nio.pool.LeaseRequest"; + private static final String METHOD = "completed"; + private static final String INTERCEPTOR_CLASS = "org.apache.skywalking.apm.plugin.httpasyncclient.v4.LeaseRequestCompletedInterceptor"; + + @Override + protected ClassMatch enhanceClass() { + return byName(ENHANCE_CLASS); + } + + @Override + public ConstructorInterceptPoint[] getConstructorsInterceptPoints() { + return new ConstructorInterceptPoint[0]; + } + + @Override + public InstanceMethodsInterceptPoint[] getInstanceMethodsInterceptPoints() { + return new InstanceMethodsInterceptPoint[]{ + new InstanceMethodsInterceptPoint() { + @Override + public ElementMatcher getMethodsMatcher() { + return named(METHOD); + } + + @Override + public String getMethodsInterceptor() { + return INTERCEPTOR_CLASS; + } + + @Override + public boolean isOverrideArgs() { + return true; + } + } + }; + } +} diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/wrapper/FutureCallbackWrapper.java b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/wrapper/FutureCallbackWrapper.java index 28f4eccb8..b87c2a1dd 100644 --- a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/wrapper/FutureCallbackWrapper.java +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/wrapper/FutureCallbackWrapper.java @@ -19,8 +19,7 @@ package org.apache.skywalking.apm.plugin.httpasyncclient.v4.wrapper; import org.apache.http.concurrent.FutureCallback; import org.apache.skywalking.apm.agent.core.context.ContextManager; - -import static org.apache.skywalking.apm.plugin.httpasyncclient.v4.SessionRequestCompleteInterceptor.CONTEXT_LOCAL; +import org.apache.skywalking.apm.plugin.httpasyncclient.v4.Constants; /** * a wrapper for {@link FutureCallback} so we can be notified when the hold response (when one or more request fails the @@ -47,7 +46,7 @@ public class FutureCallbackWrapper implements FutureCallback { @Override public void failed(Exception e) { - CONTEXT_LOCAL.remove(); + Constants.HTTP_CONTEXT_LOCAL.remove(); if (ContextManager.isActive()) { ContextManager.activeSpan().log(e); ContextManager.stopSpan(); @@ -59,7 +58,7 @@ public class FutureCallbackWrapper implements FutureCallback { @Override public void cancelled() { - CONTEXT_LOCAL.remove(); + Constants.HTTP_CONTEXT_LOCAL.remove(); if (ContextManager.isActive()) { ContextManager.activeSpan().errorOccurred(); ContextManager.stopSpan(); diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/wrapper/HttpAsyncResponseConsumerWrapper.java b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/wrapper/HttpAsyncResponseConsumerWrapper.java index 147fe72e3..8ecf8f0f8 100644 --- a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/wrapper/HttpAsyncResponseConsumerWrapper.java +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/wrapper/HttpAsyncResponseConsumerWrapper.java @@ -26,11 +26,10 @@ import org.apache.http.protocol.HttpContext; import org.apache.skywalking.apm.agent.core.context.ContextManager; import org.apache.skywalking.apm.agent.core.context.tag.Tags; import org.apache.skywalking.apm.agent.core.context.trace.AbstractSpan; +import org.apache.skywalking.apm.plugin.httpasyncclient.v4.Constants; import java.io.IOException; -import static org.apache.skywalking.apm.plugin.httpasyncclient.v4.SessionRequestCompleteInterceptor.CONTEXT_LOCAL; - /** * a wrapper for {@link HttpAsyncResponseConsumer} so we can be notified when the current response(every response will * callback the wrapper) received maybe completed or canceled,or failed. @@ -68,7 +67,7 @@ public class HttpAsyncResponseConsumerWrapper implements HttpAsyncResponseCon @Override public void failed(Exception ex) { - CONTEXT_LOCAL.remove(); + Constants.HTTP_CONTEXT_LOCAL.remove(); if (ContextManager.isActive()) { ContextManager.activeSpan().log(ex); ContextManager.stopSpan(); @@ -99,7 +98,7 @@ public class HttpAsyncResponseConsumerWrapper implements HttpAsyncResponseCon @Override public boolean cancel() { - CONTEXT_LOCAL.remove(); + Constants.HTTP_CONTEXT_LOCAL.remove(); if (ContextManager.isActive()) { ContextManager.activeSpan().errorOccurred(); ContextManager.stopSpan(); diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/resources/skywalking-plugin.def b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/resources/skywalking-plugin.def index d3153bc64..6cb1bf1dd 100644 --- a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/resources/skywalking-plugin.def +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/main/resources/skywalking-plugin.def @@ -17,5 +17,6 @@ httpasyncclient-4.x=org.apache.skywalking.apm.plugin.httpasyncclient.v4.define.HttpAsyncClientInstrumentation httpasyncclient-4.x=org.apache.skywalking.apm.plugin.httpasyncclient.v4.define.SessionRequestInstrumentation httpasyncclient-4.x=org.apache.skywalking.apm.plugin.httpasyncclient.v4.define.HttpAsyncRequestExecutorInstrumentation +httpasyncclient-4.x=org.apache.skywalking.apm.plugin.httpasyncclient.v4.define.LeaseRequestInstrumentation diff --git a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncClientInterceptorTest.java b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncClientInterceptorTest.java index ed306300b..22695420d 100644 --- a/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncClientInterceptorTest.java +++ b/apm-sniffer/apm-sdk-plugin/httpasyncclient-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpasyncclient/v4/HttpAsyncClientInterceptorTest.java @@ -28,8 +28,10 @@ import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpRequestWrapper; import org.apache.http.client.protocol.HttpClientContext; import org.apache.http.concurrent.FutureCallback; +import org.apache.http.impl.nio.DefaultNHttpClientConnection; import org.apache.http.nio.protocol.HttpAsyncRequestProducer; import org.apache.http.nio.protocol.HttpAsyncResponseConsumer; +import org.apache.http.pool.PoolEntry; import org.apache.http.protocol.BasicHttpContext; import org.apache.http.protocol.HttpContext; import org.apache.skywalking.apm.agent.core.boot.ServiceManager; @@ -59,7 +61,6 @@ import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; import org.powermock.modules.junit4.PowerMockRunnerDelegate; -import static org.apache.skywalking.apm.plugin.httpasyncclient.v4.SessionRequestCompleteInterceptor.CONTEXT_LOCAL; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.Matchers.anyString; @@ -88,6 +89,8 @@ public class HttpAsyncClientInterceptorTest { private SessionRequestCompleteInterceptor completeInterceptor; + private LeaseRequestCompletedInterceptor leaseRequestInterceptor; + @Mock private HttpAsyncRequestProducer producer; @@ -109,6 +112,15 @@ public class HttpAsyncClientInterceptorTest { @Mock private HttpResponse response; + @Mock + private DefaultNHttpClientConnection nonContextConnection; + + @Mock + private PoolEntry poolEntry; + + @Mock + private DefaultNHttpClientConnection connection; + @Before public void setUp() throws Exception { ServiceManager.INSTANCE.boot(); @@ -116,11 +128,12 @@ public class HttpAsyncClientInterceptorTest { requestExecutorInterceptor = new HttpAsyncRequestExecutorInterceptor(); sessionRequestConstructorInterceptor = new SessionRequestConstructorInterceptor(); completeInterceptor = new SessionRequestCompleteInterceptor(); + leaseRequestInterceptor = new LeaseRequestCompletedInterceptor(); httpContext = new BasicHttpContext(); httpContext.setAttribute(HttpClientContext.HTTP_REQUEST, requestWrapper); httpContext.setAttribute(HttpClientContext.HTTP_TARGET_HOST, httpHost); - CONTEXT_LOCAL.set(httpContext); + Constants.HTTP_CONTEXT_LOCAL.set(httpContext); HttpClientPluginConfig.Plugin.HttpClient.COLLECT_HTTP_PARAMS = true; when(httpHost.getHostName()).thenReturn("127.0.0.1"); when(httpHost.getSchemeName()).thenReturn("http"); @@ -178,6 +191,9 @@ public class HttpAsyncClientInterceptorTest { this.object = value; } }; + + when(poolEntry.getConnection()).thenReturn(connection); + when(connection.getContext()).thenReturn(httpContext); } @Test @@ -199,6 +215,25 @@ public class HttpAsyncClientInterceptorTest { } + @Test + public void testPoolSuccess() throws Throwable { + + //mock active span; + ContextManager.createEntrySpan("mock-test", new ContextCarrier()); + + Thread thread = basePoolTest(); + + ContextManager.stopSpan(); + + thread.join(); + Assert.assertThat(segmentStorage.getTraceSegments().size(), is(2)); + + List spans = SegmentHelper.getSpans(findNeedSegemnt()); + assertHttpSpan(spans.get(0)); + verify(requestWrapper, times(3)).setHeader(anyString(), anyString()); + + } + @Test public void testNoContext() throws Throwable { @@ -225,7 +260,7 @@ public class HttpAsyncClientInterceptorTest { FutureCallback.class }; httpAsyncClientInterceptor.beforeMethod(enhancedInstance, null, allArguments, types, null); - Assert.assertEquals(CONTEXT_LOCAL.get(), httpContext); + Assert.assertEquals(Constants.HTTP_CONTEXT_LOCAL.get(), httpContext); Assert.assertTrue(allArguments[1] instanceof HttpAsyncResponseConsumerWrapper); Assert.assertTrue(allArguments[3] instanceof FutureCallbackWrapper); @@ -238,7 +273,51 @@ public class HttpAsyncClientInterceptorTest { //start local completeInterceptor.beforeMethod(enhancedInstance, null, null, null, null); //start request - requestExecutorInterceptor.beforeMethod(enhancedInstance, null, null, null, null); + requestExecutorInterceptor.beforeMethod(enhancedInstance, null, + new Object[]{nonContextConnection}, null, null); + + HttpAsyncResponseConsumerWrapper consumerWrapper = new HttpAsyncResponseConsumerWrapper(consumer); + + consumerWrapper.responseReceived(response); + + new FutureCallbackWrapper(callback).completed(null); + + } catch (Throwable throwable) { + throwable.printStackTrace(); + } + } + }); + thread.start(); + return thread; + } + + private Thread basePoolTest() throws Throwable { + Object[] allArguments = new Object[] { + producer, + consumer, + httpContext, + callback + }; + Class[] types = new Class[] { + HttpAsyncRequestProducer.class, + HttpAsyncResponseConsumer.class, + HttpContext.class, + FutureCallback.class + }; + httpAsyncClientInterceptor.beforeMethod(enhancedInstance, null, allArguments, types, null); + Assert.assertEquals(Constants.HTTP_CONTEXT_LOCAL.get(), httpContext); + Assert.assertTrue(allArguments[1] instanceof HttpAsyncResponseConsumerWrapper); + Assert.assertTrue(allArguments[3] instanceof FutureCallbackWrapper); + + leaseRequestInterceptor.beforeMethod(null, null, new Object[]{poolEntry}, null, null); + + Thread thread = new Thread(new Runnable() { + @Override + public void run() { + try { + //start request + requestExecutorInterceptor.beforeMethod(enhancedInstance, null, + new Object[]{connection}, null, null); HttpAsyncResponseConsumerWrapper consumerWrapper = new HttpAsyncResponseConsumerWrapper(consumer); @@ -286,4 +365,4 @@ public class HttpAsyncClientInterceptorTest { Object result = instanceMethodsAroundInterceptor.afterMethod(enhancedInstance, null, null, null, ret); Assert.assertEquals(ret, result); } -} \ No newline at end of file +} diff --git a/test/plugin/scenarios/httpasyncclient-scenario/config/expectedData.yaml b/test/plugin/scenarios/httpasyncclient-scenario/config/expectedData.yaml index 0131c4023..16ea12a56 100644 --- a/test/plugin/scenarios/httpasyncclient-scenario/config/expectedData.yaml +++ b/test/plugin/scenarios/httpasyncclient-scenario/config/expectedData.yaml @@ -71,7 +71,7 @@ segmentItems: spanLayer: not null startTime: nq 0 endTime: nq 0 - componentId: 0 + componentId: 26 isError: false spanType: Local peer: ''