elek commented on pull request #2330:
URL: https://github.com/apache/ozone/pull/2330#issuecomment-861396282


   Ok, I checked the implementation and it looks like a bug in `TraceAllMethod` 
where we need using  `getMethods` instead of `getDeclaredMethods`
   
   I think `getDeclaredMethods` was used to avoid tracing the `Object` methods 
(`toString`,`equals`) but those can be excluded:
   
   The fixed version looks like:
   ```
     public TraceAllMethod(T delegate, String name) {
       this.delegate = delegate;
       this.name = name;
       for (Method method : delegate.getClass().getMethods()) {
         if (!methods.containsKey(method.getName())) {
           methods.put(method.getName(), new HashMap<>());
         }
         if (!method.getDeclaringClass().equals(Object.class)) {
           methods.get(method.getName()).put(method.getParameterTypes(), 
method);
         }
       }
     }
   ``` 
   
   Tested with this unit test:
   
   ```
   package org.apache.hadoop.hdds.tracing;
   
   import org.junit.Assert;
   import org.junit.Test;
   
   public class TestTraceAllMethod {
   
     @Test
     public void tracing() throws Throwable {
       final TraceAllMethod tracer = new TraceAllMethod(new ServiceImpl(), 
"test");
   
       Assert.assertEquals("Hello noname", tracer.invoke(null, 
ServiceImpl.class.getMethod("hello"), new Object[]{}));
       Assert.assertEquals("Hello asd", tracer.invoke(null, 
ServiceImpl.class.getMethod("hello", String.class), new Object[]{"asd"}));
     }
   
   
     public interface Service {
       default String hello() {
         return hello("noname");
       }
   
       String hello(String name);
     }
   
     public static class ServiceImpl implements Service {
   
       @Override
       public String hello(String name) {
         return "Hello " + name;
       }
     }
   }
   ```
   
   I think it would be greate to have this more resilient fix, but I am fine 
with both including it here or submitting it in a new PR.


-- 
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to