[GitHub] [ozone] elek edited a comment on pull request #2330: HDDS-5335. Method not found: allocateBlock - when tracing is enabled

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

elek edited a comment 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:
   
   ```java
     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:
   
   ```java
   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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org