[GitHub] [skywalking] arugal commented on a change in pull request #3854: Intercept feign URL without parameters

[GitBox <gi...@apache.org>]

arugal commented on a change in pull request #3854: Intercept feign URL without parameters
URL: https://github.com/apache/skywalking/pull/3854#discussion_r347091004
 
 

 ##########
 File path: apm-sniffer/apm-sdk-plugin/feign-default-http-9.x-plugin/src/main/java/org/apache/skywalking/apm/plugin/feign/http/v9/DefaultHttpClientInterceptor.java
 ##########
 @@ -48,26 +49,34 @@
  */
 public class DefaultHttpClientInterceptor implements InstanceMethodsAroundInterceptor {
 
-    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}.
      *
-     * @param method
+     * @param method intercept method
      * @param result change this result, if you want to truncate the method.
-     * @throws Throwable
+     * @throws Throwable NoSuchFieldException or IllegalArgumentException
      */
-    @Override public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
-        Class<?>[] argumentsTypes, MethodInterceptResult result) throws Throwable {
-        Request request = (Request)allArguments[0];
-
+    @Override
+    public void beforeMethod(EnhancedInstance objInst, Method method, Object[] allArguments,
+                             Class<?>[] argumentsTypes, MethodInterceptResult result) throws Throwable {
+        Request request = (Request) allArguments[0];
         URL url = new URL(request.url());
         ContextCarrier contextCarrier = new ContextCarrier();
         int port = url.getPort() == -1 ? 80 : url.getPort();
         String remotePeer = url.getHost() + ":" + port;
         String operationName = url.getPath();
+        FeignResolvedURL feignResolvedURL = PathVarInterceptor.URL_CONTEXT.get();
 
 Review comment:
   Hi, it seems better this way. right?
   ```java
           FeignResolvedURL feignResolvedURL = PathVarInterceptor.URL_CONTEXT.get();
           if (feignResolvedURL != null) {
               try {
                   operationName = operationName.replace(feignResolvedURL.getUrl(), feignResolvedURL.getOriginUrl());
               } finally {
                   PathVarInterceptor.URL_CONTEXT.remove();
               }
            }
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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