Merge pull request apache#716 from ascrutae/fix/feign-plugin-operatio…

…nName [Agent] Make operation name of fegin plugin more scenes
scwanglijun · Dec 31, 2017 · 8e510af · 8e510af
2 parents 1441493 + f30837b
commit 8e510af
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 13 deletions.
diff --git a/...ain/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptor.java b/...ain/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptor.java
@@ -16,7 +16,6 @@
  *
  */
 
-
 package org.apache.skywalking.apm.plugin.feign.http.v9;
 
 import feign.Request;
@@ -52,9 +51,9 @@ public class DefaultHttpClientInterceptor implements InstanceMethodsAroundInterc
     private static final String COMPONENT_NAME = "FeignDefaultHttp";
 
     /**
-     * Get the {@link feign.Request} from {@link EnhancedInstance}, then create {@link AbstractSpan} and set host,
-     * port, kind, component, url from {@link feign.Request}.
-     * Through the reflection of the way, set the http header of context data into {@link feign.Request#headers}.
+     * Get the {@link feign.Request} from {@link EnhancedInstance}, then create {@link AbstractSpan} and set host, port,
+     * kind, component, url from {@link feign.Request}. Through the reflection of the way, set the http header of
+     * context data into {@link feign.Request#headers}.
      *
      * @param method
      * @param result change this result, if you want to truncate the method.
@@ -66,11 +65,16 @@ public class DefaultHttpClientInterceptor implements InstanceMethodsAroundInterc
 
         URL url = new URL(request.url());
         ContextCarrier contextCarrier = new ContextCarrier();
-        String remotePeer = url.getHost() + ":" + url.getPort();
-        AbstractSpan span = ContextManager.createExitSpan(request.url(), contextCarrier, remotePeer);
+        int port = url.getPort() == -1 ? 80 : url.getPort();
+        String remotePeer = url.getHost() + ":" + port;
+        String operationName = url.getPath();
+        if (operationName == null || operationName.length() == 0) {
+            operationName = "/";
+        }
+        AbstractSpan span = ContextManager.createExitSpan(operationName, contextCarrier, remotePeer);
         span.setComponent(ComponentsDefine.FEIGN);
         Tags.HTTP.METHOD.set(span, request.method());
-        Tags.URL.set(span, url.getPath());
+        Tags.URL.set(span, request.url());
         SpanLayer.asHttp(span);
 
         Field headersField = Request.class.getDeclaredField("headers");
@@ -94,8 +98,7 @@ public class DefaultHttpClientInterceptor implements InstanceMethodsAroundInterc
 
     /**
      * Get the status code from {@link Response}, when status code greater than 400, it means there was some errors in
-     * the server.
-     * Finish the {@link AbstractSpan}.
+     * the server. Finish the {@link AbstractSpan}.
      *
      * @param method
      * @param ret the method's original return value.

diff --git a/...java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptorTest.java b/...java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptorTest.java
@@ -87,7 +87,7 @@ public class DefaultHttpClientInterceptorTest {
     public void setUp() throws Exception {
 
         Map<String, Collection<String>> headers = new LinkedHashMap<String, Collection<String>>();
-        request = Request.create("GET", "http://skywalking.org", headers, "Test".getBytes(), Charset.forName("UTF-8"));
+        request = Request.create("GET", "http://skywalking.org/", headers, "Test".getBytes(), Charset.forName("UTF-8"));
         Request.Options options = new Request.Options();
         allArguments = new Object[] {request, options};
         argumentTypes = new Class[] {request.getClass(), options.getClass()};
@@ -112,7 +112,7 @@ public void testMethodsAround() throws Throwable {
         List<KeyValuePair> tags = SpanHelper.getTags(finishedSpan);
         assertThat(tags.size(), is(2));
         assertThat(tags.get(0).getValue(), is("GET"));
-        assertThat(tags.get(1).getValue(), is(""));
+        assertThat(tags.get(1).getValue(), is("http://skywalking.org/"));
 
         Assert.assertEquals(false, SpanHelper.getErrorOccurred(finishedSpan));
     }
@@ -135,7 +135,7 @@ public void testMethodsAroundError() throws Throwable {
         List<KeyValuePair> tags = SpanHelper.getTags(finishedSpan);
         assertThat(tags.size(), is(3));
         assertThat(tags.get(0).getValue(), is("GET"));
-        assertThat(tags.get(1).getValue(), is(""));
+        assertThat(tags.get(1).getValue(), is("http://skywalking.org/"));
         assertThat(tags.get(2).getValue(), is("404"));
 
         Assert.assertEquals(true, SpanHelper.getErrorOccurred(finishedSpan));
@@ -166,7 +166,7 @@ public void testException() throws Throwable {
         List<KeyValuePair> tags = SpanHelper.getTags(finishedSpan);
         assertThat(tags.size(), is(2));
         assertThat(tags.get(0).getValue(), is("GET"));
-        assertThat(tags.get(1).getValue(), is(""));
+        assertThat(tags.get(1).getValue(), is("http://skywalking.org/"));
 
         Assert.assertEquals(true, SpanHelper.getErrorOccurred(finishedSpan));