Avoid BlockHound NoSuchTypeException when context propagation dependency is unavailable #3337
Conversation
…on dependencies is unavailable
|
@reactor/core-team , can you please take a look ? thanks. |
What is really needed is to avoid using a lambda when initializing the contextCaptureFunction from the static initializer.
|
@reactor/core-team , I have simplified the impact of the PR, and the key point is to avoid using a lambda in this part of the static initializer. please take a look. |
|
@chemicL , thank you for your review ! |
|
I wonder which label should be used for this issue, because there is no bug fixed per se, it's a work around to avoid the logging issue from raphw/byte-buddy#780 Should I set the label to |
|
I picked |
|
After the recent necessary changes made in the ContextPropagation (latest commit done in 3.5.3 version), the exception is resurfacing. |
When the
context-propagationjar is unavailable (which can happen since the dependency is optional), then BlockHound reports the following ERROR log:This log does not break anything, it is only about logging, but it can be avoided using the proposed patch.
A user has also reported this issue here: reactor/BlockHound#311
Can be also reproduced when building reactor-netty under the netty5 branch, when running this test:
This problem is actually related to the following old issues:
reactor/BlockHound#71
raphw/byte-buddy#780
In a nutshell, Byte Buddy requires all type information for a transformed type to be available, else the above error log may be displayed. The log may happen occasionally with very rare byte code combinations
when the class is being defined (initial classloading). Please check #780 for the whole details.
So, what happens is that when the reactor-core ContextPropagation static initializer is instrumented, all bytes code from the static init is instrumented, including the part which is accessing to the context-propagation API, especially this problematic part which involves a lambda; so when the context-propagation is not there, byte buddy is reporting the missing io.micrometer.context.ContextRegistry type.
To avoid this problem, the proposed patch replaces the ContextPropagation static initializer with a basic
Class.forName("io.micrometer.context.ContextRegistry")and all the init code which is manipulating the context propagation API is moved to the lazyHolderinner class which will be lazily invoked only if the context propagation is available.If this PR is rejected, it is possible to enhance the BlockHound in order to not log the missing type. However, we can't ignore such missing types by default because in cases where the missing type is not optional, then that would be dangerous to just hide the log. So what can be done is to enhance the BlockHound builder API so user can specify a callback in order to ignore (or log) a missing type. And in this case, the Reactor Core BlockHound integration plugin will have to be modified in order to just ignore the missing type for context propagation using the new BlockHound missing type callback.