ExecutionServiceMetricsBinder strips unrelated instrumentations from io.netty classes

[mitchnull]

Expected Behavior

ExecutionServiceMetricsBinder does not remove any previously added instrumentations.

Actual Behaviour

The fix in #62 introduced a bug where any previous instrumentations on io.netty objects (like EventLoopGroup) get removed.

As far as I can see here you should return the original bean (executorService) instead of the unwrapped netty bean (in ExecutionServiceMetricsBinder.onCreate):

        if (unwrapped.getClass().getName().startsWith("io.netty")) {
            return unwrapped; // this should be: return executionService;
        }

With the current code, if we try to instrument a netty EventLoopGroup then we run into 2 failure scenarios:

1. the above code strips our own instrumentation if we implement InstrumentedExecutionService
2. re-introduce the original bug if we do not implement InstrumentedExecutionService as the special (and rather ugly) hack for io.netty package match fails.

Steps To Reproduce

Add a BeanCreatedEventListener<ExecutorService> that instruments EventLoopGroup

1. if it implements InstrumentedExecutionService then this will be removed if the ExecutionServiceMetricsBinder runs after our BeanCreatedEventListener (this is impossible to avoid as the ExecutionServiceMetricsBinder has default (lowest) order, so it's ordered last, and no other property is used for ordering lowest precedence listeners)
2. if it does not implement InstrumentedExecutionService then the original bug Micronaut 2.0.0 doesn't work with metrics #62 will resurface, as the logic to avoid instrumenting io.netty packages will not trigger

If we create our listener for something other than ExecutorService, then we still face the same issue as ordering cannot be guaranteed.

Environment Information

environment: linux, jdk-11.

Example Application

No response

Version

3.5.0

[graemerocher]

The solution would be to create an InstrumentedEventLoopGroup type and add a BeanCreatedEventListener< EventLoopGroup>. The problem with the previous code is it alters the bean type resulting on other downstream problems.