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 2019/02/01 15:28:28 UTC
[incubator-skywalking] branch master updated: Fixed an error that took a non-protocol uri string as a java.net.URL … (#2231)
This is an automated email from the ASF dual-hosted git repository.
wusheng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-skywalking.git
The following commit(s) were added to refs/heads/master by this push:
new 4657ce7 Fixed an error that took a non-protocol uri string as a java.net.URL … (#2231)
4657ce7 is described below
commit 4657ce7f0c0dbc31caeb1734d4977cb0819396cb
Author: dongbin <do...@qq.com>
AuthorDate: Fri Feb 1 23:28:21 2019 +0800
Fixed an error that took a non-protocol uri string as a java.net.URL … (#2231)
* Fixed an error that took a non-protocol uri string as a java.net.URL construct parameter.
* Fixed an error that took a non-protocol uri string as a java.net.URL construct parameter.
* Fixed checkstyle error.
---
.../v4/HttpClientExecuteInterceptor.java | 46 +++++++++++++++++-----
.../v4/HttpClientExecuteInterceptorTest.java | 29 ++++++++++++++
2 files changed, 66 insertions(+), 9 deletions(-)
diff --git a/apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientExecuteInterceptor.java b/apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientExecuteInterceptor.java
index af077b9..8707f79 100644
--- a/apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientExecuteInterceptor.java
+++ b/apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientExecuteInterceptor.java
@@ -39,7 +39,7 @@ import org.apache.skywalking.apm.network.trace.component.ComponentsDefine;
public class HttpClientExecuteInterceptor implements InstanceMethodsAroundInterceptor {
@Override public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
- Class<?>[] argumentsTypes, MethodInterceptResult result) throws Throwable {
+ Class<?>[] argumentsTypes, MethodInterceptResult result) throws Throwable {
if (allArguments[0] == null || allArguments[1] == null) {
// illegal args, can't trace. ignore.
return;
@@ -48,16 +48,15 @@ public class HttpClientExecuteInterceptor implements InstanceMethodsAroundInterc
HttpRequest httpRequest = (HttpRequest)allArguments[1];
final ContextCarrier contextCarrier = new ContextCarrier();
- String remotePeer = httpHost.getHostName() + ":" + (httpHost.getPort() > 0 ? httpHost.getPort() :
- "https".equals(httpHost.getSchemeName().toLowerCase()) ? 443 : 80);
+ String remotePeer = httpHost.getHostName() + ":" + port(httpHost);
String uri = httpRequest.getRequestLine().getUri();
String requestURI = getRequestURI(uri);
- String operationName = uri.startsWith("http") ? requestURI : uri;
+ String operationName = requestURI;
AbstractSpan span = ContextManager.createExitSpan(operationName, contextCarrier, remotePeer);
span.setComponent(ComponentsDefine.HTTPCLIENT);
- Tags.URL.set(span, uri);
+ Tags.URL.set(span, buildSpanValue(httpHost,uri));
Tags.HTTP.METHOD.set(span, httpRequest.getRequestLine().getMethod());
SpanLayer.asHttp(span);
@@ -69,7 +68,7 @@ public class HttpClientExecuteInterceptor implements InstanceMethodsAroundInterc
}
@Override public Object afterMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
- Class<?>[] argumentsTypes, Object ret) throws Throwable {
+ Class<?>[] argumentsTypes, Object ret) throws Throwable {
if (allArguments[0] == null || allArguments[1] == null) {
return ret;
}
@@ -92,14 +91,43 @@ public class HttpClientExecuteInterceptor implements InstanceMethodsAroundInterc
}
@Override public void handleMethodException(EnhancedInstance objInst, Method method, Object[] allArguments,
- Class<?>[] argumentsTypes, Throwable t) {
+ Class<?>[] argumentsTypes, Throwable t) {
AbstractSpan activeSpan = ContextManager.activeSpan();
activeSpan.errorOccurred();
activeSpan.log(t);
}
private String getRequestURI(String uri) throws MalformedURLException {
- String requestPath = new URL(uri).getPath();
- return requestPath != null && requestPath.length() > 0 ? requestPath : "/";
+ if (isUrl(uri)) {
+ String requestPath = new URL(uri).getPath();
+ return requestPath != null && requestPath.length() > 0 ? requestPath : "/";
+ } else {
+ return uri;
+ }
+ }
+
+ private boolean isUrl(String uri) {
+ String lowerUrl = uri.toLowerCase();
+ return lowerUrl.startsWith("http") || lowerUrl.startsWith("https");
+ }
+
+ private String buildSpanValue(HttpHost httpHost, String uri) {
+ if (isUrl(uri)) {
+ return uri;
+ } else {
+ StringBuilder buff = new StringBuilder();
+ buff.append(httpHost.getSchemeName().toLowerCase());
+ buff.append("://");
+ buff.append(httpHost.getHostName());
+ buff.append(":");
+ buff.append(port(httpHost));
+ buff.append(uri);
+ return buff.toString();
+ }
+ }
+
+ private int port(HttpHost httpHost) {
+ int port = httpHost.getPort();
+ return port > 0 ? port : "https".equals(httpHost.getSchemeName().toLowerCase()) ? 443 : 80;
}
}
diff --git a/apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientExecuteInterceptorTest.java b/apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientExecuteInterceptorTest.java
index 2c2106d..89fbf30 100644
--- a/apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientExecuteInterceptorTest.java
+++ b/apm-sniffer/apm-sdk-plugin/httpClient-4.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/httpClient/v4/HttpClientExecuteInterceptorTest.java
@@ -171,6 +171,35 @@ public class HttpClientExecuteInterceptorTest {
}
+ @Test
+ public void testUriNotProtocol() throws Throwable {
+ when(request.getRequestLine()).thenReturn(new RequestLine() {
+ @Override
+ public String getMethod() {
+ return "GET";
+ }
+
+ @Override
+ public ProtocolVersion getProtocolVersion() {
+ return new ProtocolVersion("http", 1, 1);
+ }
+
+ @Override
+ public String getUri() {
+ return "/test-web/test";
+ }
+ });
+ httpClientExecuteInterceptor.beforeMethod(enhancedInstance, null, allArguments, argumentsType, null);
+ httpClientExecuteInterceptor.afterMethod(enhancedInstance, null, allArguments, 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(1)).setHeader(anyString(), anyString());
+ }
+
private void assertHttpSpanErrorLog(List<LogDataEntity> logs) {
assertThat(logs.size(), is(1));
LogDataEntity logData = logs.get(0);