From 4657ce7f0c0dbc31caeb1734d4977cb0819396cb Mon Sep 17 00:00:00 2001 From: dongbin Date: Fri, 1 Feb 2019 23:28:21 +0800 Subject: [PATCH] =?UTF-8?q?Fixed=20an=20error=20that=20took=20a=20non-prot?= =?UTF-8?q?ocol=20uri=20string=20as=20a=20java.net.URL=20=E2=80=A6=20(#223?= =?UTF-8?q?1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 af077b9dc3..8707f79633 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 2c2106de33..89fbf30bd3 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 spans = SegmentHelper.getSpans(traceSegment); + assertHttpSpan(spans.get(0)); + verify(request, times(1)).setHeader(anyString(), anyString()); + } + private void assertHttpSpanErrorLog(List logs) { assertThat(logs.size(), is(1)); LogDataEntity logData = logs.get(0); -- GitLab