You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by wu...@apache.org on 2024/03/12 15:12:40 UTC
(skywalking-java) branch main updated: Fix not tracing in HttpClient v5 when HttpHost(arg[0]) is null but `RoutingSupport#determineHost` works. (#674)
This is an automated email from the ASF dual-hosted git repository.
wusheng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/skywalking-java.git
The following commit(s) were added to refs/heads/main by this push:
new 466f173f98 Fix not tracing in HttpClient v5 when HttpHost(arg[0]) is null but `RoutingSupport#determineHost` works. (#674)
466f173f98 is described below
commit 466f173f988b728551b2134f6ed409b97baac26c
Author: cylx3126 <49...@users.noreply.github.com>
AuthorDate: Tue Mar 12 23:12:34 2024 +0800
Fix not tracing in HttpClient v5 when HttpHost(arg[0]) is null but `RoutingSupport#determineHost` works. (#674)
Skywalking hc5 plugin worked the same as hc4 plugin: if the arg[0] is null, skip creating the exitSpan. this will cause a bug in hc5: when the HttpHost is null but InternalHttpClient determines the host from ClassicHttpRequest, InternalHttpClient will send the request but Skywalking will not record it.
---
CHANGES.md | 1 +
.../v5/HttpClientDoExecuteInterceptor.java | 19 ++++-
.../v5/InternalClientDoExecuteInterceptor.java | 44 +++++++++++
.../v5/MinimalClientDoExecuteInterceptor.java | 33 ++++++++
...java => InternalHttpClientInstrumentation.java} | 11 ++-
....java => MinimalHttpClientInstrumentation.java} | 9 +--
.../src/main/resources/skywalking-plugin.def | 3 +-
... InternalHttpClientExecuteInterceptorTest.java} | 91 +++++++++++++++++++---
...> MinimalHttpClientExecuteInterceptorTest.java} | 4 +-
9 files changed, 186 insertions(+), 29 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index 5ea8c5da7b..11dcc86166 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -17,6 +17,7 @@ Release Notes.
* Support for ActiveMQ-Artemis messaging tracing.
* Archive the expired plugins `impala-jdbc-2.6.x-plugin`.
* Fix a bug in Spring Cloud Gateway if HttpClientFinalizer#send does not invoke, the span created at NettyRoutingFilterInterceptor can not stop.
+* Fix not tracing in HttpClient v5 when HttpHost(arg[0]) is null but `RoutingSupport#determineHost` works.
#### Documentation
* Update docs to describe `expired-plugins`.
diff --git a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpClientDoExecuteInterceptor.java b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpClientDoExecuteInterceptor.java
index e31b5cae1e..d53eb7a246 100644
--- a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpClientDoExecuteInterceptor.java
+++ b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpClientDoExecuteInterceptor.java
@@ -38,7 +38,7 @@ import java.lang.reflect.Method;
import java.net.MalformedURLException;
import java.net.URL;
-public class HttpClientDoExecuteInterceptor implements InstanceMethodsAroundInterceptor {
+public abstract class HttpClientDoExecuteInterceptor implements InstanceMethodsAroundInterceptor {
private static final String ERROR_URI = "/_blank";
private static final ILog LOGGER = LogManager.getLogger(HttpClientDoExecuteInterceptor.class);
@@ -46,11 +46,11 @@ public class HttpClientDoExecuteInterceptor implements InstanceMethodsAroundInte
@Override
public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
MethodInterceptResult result) throws Throwable {
- if (allArguments[0] == null || allArguments[1] == null) {
+ if (skipIntercept(objInst, method, allArguments, argumentsTypes)) {
// illegal args, can't trace. ignore.
return;
}
- final HttpHost httpHost = (HttpHost) allArguments[0];
+ final HttpHost httpHost = getHttpHost(objInst, method, allArguments, argumentsTypes);
ClassicHttpRequest httpRequest = (ClassicHttpRequest) allArguments[1];
final ContextCarrier contextCarrier = new ContextCarrier();
@@ -75,10 +75,18 @@ public class HttpClientDoExecuteInterceptor implements InstanceMethodsAroundInte
}
}
+ protected boolean skipIntercept(EnhancedInstance objInst, Method method, Object[] allArguments,
+ Class<?>[] argumentsTypes) {
+ return allArguments[1] == null || getHttpHost(objInst, method, allArguments, argumentsTypes) == null;
+ }
+
+ protected abstract HttpHost getHttpHost(EnhancedInstance objInst, Method method, Object[] allArguments,
+ Class<?>[] argumentsTypes) ;
+
@Override
public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments, Class<?>[] argumentsTypes,
Object ret) throws Throwable {
- if (allArguments[0] == null || allArguments[1] == null) {
+ if (skipIntercept(objInst, method, allArguments, argumentsTypes)) {
return ret;
}
@@ -100,6 +108,9 @@ public class HttpClientDoExecuteInterceptor implements InstanceMethodsAroundInte
@Override
public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
Class<?>[] argumentsTypes, Throwable t) {
+ if (skipIntercept(objInst, method, allArguments, argumentsTypes)) {
+ return;
+ }
AbstractSpan activeSpan = ContextManager.activeSpan();
activeSpan.log(t);
}
diff --git a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/InternalClientDoExecuteInterceptor.java b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/InternalClientDoExecuteInterceptor.java
new file mode 100644
index 0000000000..c487452473
--- /dev/null
+++ b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/InternalClientDoExecuteInterceptor.java
@@ -0,0 +1,44 @@
+/*
+ * 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.httpclient.v5;
+
+import org.apache.hc.client5.http.routing.RoutingSupport;
+import org.apache.hc.core5.http.HttpHost;
+import org.apache.hc.core5.http.HttpRequest;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+
+import java.lang.reflect.Method;
+
+public class InternalClientDoExecuteInterceptor extends HttpClientDoExecuteInterceptor {
+
+ @Override
+ protected HttpHost getHttpHost(EnhancedInstance objInst, Method method, Object[] allArguments,
+ Class<?>[] argumentsTypes) {
+ HttpHost httpHost = (HttpHost) allArguments[0];
+ if (httpHost != null) {
+ return httpHost;
+ }
+ try {
+ return RoutingSupport.determineHost((HttpRequest) allArguments[1]);
+ } catch (Exception ignore) {
+ // ignore
+ return null;
+ }
+ }
+}
diff --git a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/MinimalClientDoExecuteInterceptor.java b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/MinimalClientDoExecuteInterceptor.java
new file mode 100644
index 0000000000..4c1dc38c4b
--- /dev/null
+++ b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/MinimalClientDoExecuteInterceptor.java
@@ -0,0 +1,33 @@
+/*
+ * 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.httpclient.v5;
+
+import org.apache.hc.core5.http.HttpHost;
+import org.apache.skywalking.apm.agent.core.plugin.interceptor.enhance.EnhancedInstance;
+
+import java.lang.reflect.Method;
+
+public class MinimalClientDoExecuteInterceptor extends HttpClientDoExecuteInterceptor {
+
+ @Override
+ protected HttpHost getHttpHost(EnhancedInstance objInst, Method method, Object[] allArguments,
+ Class<?>[] argumentsTypes) {
+ return (HttpHost) allArguments[0];
+ }
+}
diff --git a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/HttpClientInstrumentation.java b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/InternalHttpClientInstrumentation.java
similarity index 87%
copy from apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/HttpClientInstrumentation.java
copy to apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/InternalHttpClientInstrumentation.java
index 79593f1ada..431a4c1225 100644
--- a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/HttpClientInstrumentation.java
+++ b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/InternalHttpClientInstrumentation.java
@@ -28,21 +28,20 @@ import org.apache.skywalking.apm.agent.core.plugin.match.MultiClassNameMatch;
import static net.bytebuddy.matcher.ElementMatchers.named;
-public class HttpClientInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+public class InternalHttpClientInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
- private static final String ENHANCE_CLASS_MINIMAL = "org.apache.hc.client5.http.impl.classic.MinimalHttpClient";
- private static final String ENHANCE_CLASS_INTERNAL = "org.apache.hc.client5.http.impl.classic.InternalHttpClient";
+ private static final String ENHANCE_CLASS_MINIMAL = "org.apache.hc.client5.http.impl.classic.InternalHttpClient";
private static final String METHOD_NAME = "doExecute";
- private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.httpclient.v5.HttpClientDoExecuteInterceptor";
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.httpclient.v5.InternalClientDoExecuteInterceptor";
@Override
public ClassMatch enhanceClass() {
- return MultiClassNameMatch.byMultiClassMatch(ENHANCE_CLASS_MINIMAL, ENHANCE_CLASS_INTERNAL);
+ return MultiClassNameMatch.byMultiClassMatch(ENHANCE_CLASS_MINIMAL);
}
@Override
public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
- return null;
+ return new ConstructorInterceptPoint[0];
}
@Override
diff --git a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/HttpClientInstrumentation.java b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/MinimalHttpClientInstrumentation.java
similarity index 88%
rename from apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/HttpClientInstrumentation.java
rename to apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/MinimalHttpClientInstrumentation.java
index 79593f1ada..f078cc04e8 100644
--- a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/HttpClientInstrumentation.java
+++ b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpclient/v5/define/MinimalHttpClientInstrumentation.java
@@ -28,21 +28,20 @@ import org.apache.skywalking.apm.agent.core.plugin.match.MultiClassNameMatch;
import static net.bytebuddy.matcher.ElementMatchers.named;
-public class HttpClientInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
+public class MinimalHttpClientInstrumentation extends ClassInstanceMethodsEnhancePluginDefine {
private static final String ENHANCE_CLASS_MINIMAL = "org.apache.hc.client5.http.impl.classic.MinimalHttpClient";
- private static final String ENHANCE_CLASS_INTERNAL = "org.apache.hc.client5.http.impl.classic.InternalHttpClient";
private static final String METHOD_NAME = "doExecute";
- private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.httpclient.v5.HttpClientDoExecuteInterceptor";
+ private static final String INTERCEPT_CLASS = "org.apache.skywalking.apm.plugin.httpclient.v5.MinimalClientDoExecuteInterceptor";
@Override
public ClassMatch enhanceClass() {
- return MultiClassNameMatch.byMultiClassMatch(ENHANCE_CLASS_MINIMAL, ENHANCE_CLASS_INTERNAL);
+ return MultiClassNameMatch.byMultiClassMatch(ENHANCE_CLASS_MINIMAL);
}
@Override
public ConstructorInterceptPoint[] getConstructorsInterceptPoints() {
- return null;
+ return new ConstructorInterceptPoint[0];
}
@Override
diff --git a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/resources/skywalking-plugin.def b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/resources/skywalking-plugin.def
index 7f04889ca2..dc6622a88f 100644
--- a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/resources/skywalking-plugin.def
+++ b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/main/resources/skywalking-plugin.def
@@ -14,6 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-httpclient-5.x=org.apache.skywalking.apm.plugin.httpclient.v5.define.HttpClientInstrumentation
+httpclient-5.x=org.apache.skywalking.apm.plugin.httpclient.v5.define.MinimalHttpClientInstrumentation
+httpclient-5.x=org.apache.skywalking.apm.plugin.httpclient.v5.define.InternalHttpClientInstrumentation
httpclient-5.x=org.apache.skywalking.apm.plugin.httpclient.v5.define.HttpAsyncClientInstrumentation
httpclient-5.x=org.apache.skywalking.apm.plugin.httpclient.v5.define.IOSessionImplInstrumentation
diff --git a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpClientExecuteInterceptorTest.java b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/InternalHttpClientExecuteInterceptorTest.java
similarity index 69%
copy from apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpClientExecuteInterceptorTest.java
copy to apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/InternalHttpClientExecuteInterceptorTest.java
index b4c1972d1e..f20bf6254a 100644
--- a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpClientExecuteInterceptorTest.java
+++ b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/InternalHttpClientExecuteInterceptorTest.java
@@ -18,15 +18,6 @@
package org.apache.skywalking.apm.plugin.httpclient.v5;
-import static junit.framework.TestCase.assertNotNull;
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.mockito.ArgumentMatchers.anyString;
-import static org.mockito.Mockito.times;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-import java.net.URI;
-import java.util.List;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.HttpHost;
@@ -52,8 +43,19 @@ import org.mockito.Mock;
import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
+import java.net.URI;
+import java.util.List;
+
+import static junit.framework.TestCase.assertNotNull;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
@RunWith(TracingSegmentRunner.class)
-public class HttpClientExecuteInterceptorTest {
+public class InternalHttpClientExecuteInterceptorTest {
@SegmentStoragePoint
private SegmentStorage segmentStorage;
@@ -73,6 +75,7 @@ public class HttpClientExecuteInterceptorTest {
private ClassicHttpResponse httpResponse;
private Object[] allArguments;
+ private Object[] allArgumentsWithNullHttpHost;
private Class[] argumentsType;
@Mock
@@ -82,7 +85,7 @@ public class HttpClientExecuteInterceptorTest {
public void setUp() throws Exception {
ServiceManager.INSTANCE.boot();
- httpClientDoExecuteInterceptor = new HttpClientDoExecuteInterceptor();
+ httpClientDoExecuteInterceptor = new InternalClientDoExecuteInterceptor();
when(httpResponse.getCode()).thenReturn(200);
when(httpHost.getHostName()).thenReturn("127.0.0.1");
@@ -95,6 +98,10 @@ public class HttpClientExecuteInterceptorTest {
httpHost,
request
};
+ allArgumentsWithNullHttpHost = new Object[]{
+ null,
+ request
+ };
argumentsType = new Class[]{
httpHost.getClass(),
request.getClass()
@@ -109,6 +116,20 @@ public class HttpClientExecuteInterceptorTest {
Assert.assertThat(segmentStorage.getTraceSegments().size(), is(1));
TraceSegment traceSegment = segmentStorage.getTraceSegments().get(0);
+ List<AbstractTracingSpan> spans = SegmentHelper.getSpans(traceSegment);
+ assertHttpSpan(spans.get(0));
+ verify(request, times(3)).setHeader(anyString(), anyString());
+
+ }
+
+ @Test
+ public void testNullHttpHostHttpClient() throws Throwable {
+ httpClientDoExecuteInterceptor.beforeMethod(enhancedInstance, null, allArgumentsWithNullHttpHost, argumentsType, null);
+ httpClientDoExecuteInterceptor.afterMethod(enhancedInstance, null, allArgumentsWithNullHttpHost, argumentsType, httpResponse);
+
+ Assert.assertThat(segmentStorage.getTraceSegments().size(), is(1));
+ TraceSegment traceSegment = segmentStorage.getTraceSegments().get(0);
+
List<AbstractTracingSpan> spans = SegmentHelper.getSpans(traceSegment);
assertHttpSpan(spans.get(0));
verify(request, times(3)).setHeader(anyString(), anyString());
@@ -135,6 +156,27 @@ public class HttpClientExecuteInterceptorTest {
verify(request, times(3)).setHeader(anyString(), anyString());
}
+ @Test
+ public void testNullHttpHostStatusCodeNotEquals200() throws Throwable {
+ when(httpResponse.getCode()).thenReturn(500);
+ httpClientDoExecuteInterceptor.beforeMethod(enhancedInstance, null, allArgumentsWithNullHttpHost, argumentsType, null);
+ httpClientDoExecuteInterceptor.afterMethod(enhancedInstance, null, allArgumentsWithNullHttpHost, argumentsType, httpResponse);
+
+ Assert.assertThat(segmentStorage.getTraceSegments().size(), is(1));
+ TraceSegment traceSegment = segmentStorage.getTraceSegments().get(0);
+ List<AbstractTracingSpan> spans = SegmentHelper.getSpans(traceSegment);
+
+ assertThat(spans.size(), is(1));
+
+ List<TagValuePair> tags = SpanHelper.getTags(spans.get(0));
+ assertThat(tags.size(), is(3));
+ assertThat(tags.get(2).getValue(), is("500"));
+
+ assertHttpSpan(spans.get(0));
+ assertThat(SpanHelper.getErrorOccurred(spans.get(0)), is(true));
+ verify(request, times(3)).setHeader(anyString(), anyString());
+ }
+
@Test
public void testHttpClientWithException() throws Throwable {
httpClientDoExecuteInterceptor.beforeMethod(enhancedInstance, null, allArguments, argumentsType, null);
@@ -152,7 +194,25 @@ public class HttpClientExecuteInterceptorTest {
assertThat(SpanHelper.getErrorOccurred(span), is(true));
assertHttpSpanErrorLog(SpanHelper.getLogs(span));
verify(request, times(3)).setHeader(anyString(), anyString());
+ }
+ @Test
+ public void testNullHttpHostHttpClientWithException() throws Throwable {
+ httpClientDoExecuteInterceptor.beforeMethod(enhancedInstance, null, allArgumentsWithNullHttpHost, argumentsType, null);
+ httpClientDoExecuteInterceptor.handleMethodException(enhancedInstance, null, allArgumentsWithNullHttpHost, argumentsType,
+ new RuntimeException("testException"));
+ httpClientDoExecuteInterceptor.afterMethod(enhancedInstance, null, allArgumentsWithNullHttpHost, argumentsType, httpResponse);
+
+ Assert.assertThat(segmentStorage.getTraceSegments().size(), is(1));
+ TraceSegment traceSegment = segmentStorage.getTraceSegments().get(0);
+ List<AbstractTracingSpan> spans = SegmentHelper.getSpans(traceSegment);
+
+ assertThat(spans.size(), is(1));
+ AbstractTracingSpan span = spans.get(0);
+ assertHttpSpan(span);
+ assertThat(SpanHelper.getErrorOccurred(span), is(true));
+ assertHttpSpanErrorLog(SpanHelper.getLogs(span));
+ verify(request, times(3)).setHeader(anyString(), anyString());
}
@Test
@@ -169,6 +229,15 @@ public class HttpClientExecuteInterceptorTest {
verify(request, times(3)).setHeader(anyString(), anyString());
}
+ @Test
+ public void testNullHttpHostUriNotProtocol() throws Throwable {
+ when(request.getUri()).thenReturn(new URI("/test-web/test"));
+ httpClientDoExecuteInterceptor.beforeMethod(enhancedInstance, null, allArgumentsWithNullHttpHost, argumentsType, null);
+ httpClientDoExecuteInterceptor.afterMethod(enhancedInstance, null, allArgumentsWithNullHttpHost, argumentsType, httpResponse);
+
+ Assert.assertThat(segmentStorage.getTraceSegments().size(), is(0));
+ }
+
private void assertHttpSpanErrorLog(List<LogDataEntity> logs) {
assertThat(logs.size(), is(1));
LogDataEntity logData = logs.get(0);
diff --git a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpClientExecuteInterceptorTest.java b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/MinimalHttpClientExecuteInterceptorTest.java
similarity index 98%
rename from apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpClientExecuteInterceptorTest.java
rename to apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/MinimalHttpClientExecuteInterceptorTest.java
index b4c1972d1e..57b9d90553 100644
--- a/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/HttpClientExecuteInterceptorTest.java
+++ b/apm-sniffer/apm-sdk-plugin/httpclient-5.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpclient/v5/MinimalHttpClientExecuteInterceptorTest.java
@@ -53,7 +53,7 @@ import org.mockito.junit.MockitoJUnit;
import org.mockito.junit.MockitoRule;
@RunWith(TracingSegmentRunner.class)
-public class HttpClientExecuteInterceptorTest {
+public class MinimalHttpClientExecuteInterceptorTest {
@SegmentStoragePoint
private SegmentStorage segmentStorage;
@@ -82,7 +82,7 @@ public class HttpClientExecuteInterceptorTest {
public void setUp() throws Exception {
ServiceManager.INSTANCE.boot();
- httpClientDoExecuteInterceptor = new HttpClientDoExecuteInterceptor();
+ httpClientDoExecuteInterceptor = new MinimalClientDoExecuteInterceptor();
when(httpResponse.getCode()).thenReturn(200);
when(httpHost.getHostName()).thenReturn("127.0.0.1");