You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@skywalking.apache.org by GitBox <gi...@apache.org> on 2017/12/31 04:35:00 UTC

[GitHub] wu-sheng closed pull request #716: [Agent] Make operation name of fegin plugin more scenes

wu-sheng closed pull request #716: [Agent] Make operation name of fegin plugin more scenes
URL: https://github.com/apache/incubator-skywalking/pull/716
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptor.java b/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptor.java
index b49b1fd2c..eef05b7ef 100644
--- a/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptor.java
+++ b/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/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 @@
     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 @@
 
         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 @@
 
     /**
      * 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/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptorTest.java b/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptorTest.java
index 1c5d30a1a..20f2ccfe9 100644
--- a/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptorTest.java
+++ b/apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/test/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptorTest.java
@@ -87,7 +87,7 @@
     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));
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services