Remove the use of a FactoryBean when auto-configuring VirtualThreadMetrics #43957
Conversation
spring-projects-issues
added
the
status: waiting-for-triage
There was a problem hiding this comment.
Wow! That was quick, @nosan. Thanks very much. I've left one comment for your consideration when you have a minute.
| private ClassLoader beanClassLoader; | ||
|
|
||
| @Override | ||
| public void setBeanClassLoader(ClassLoader classLoader) { | ||
| this.beanClassLoader = classLoader; | ||
| } | ||
|
|
There was a problem hiding this comment.
I think we should simplify this further and use the class loader that loaded JvmMetricsAutoConfiguration. Using the bean class loader theoretically allows JvmMetricsAutoConfiguration to be loaded by one class loader and VirtualThreadMetrics to be loaded by another. I don't think we need to support that arrangement as it wouldn't be supported if this were a straightforward @Bean method with VirtualThreadMetrics as its return type.
There was a problem hiding this comment.
Thanks, @wilkinsona
I used BeanClassLoader instead of getClass().getClassLoader() because, to my knowledge, it is used in OnClassCondition. I am concerned that if the @ConditionalOnClass check passes but the class is not available in the JvmMetricsAutoConfiguration class loader, it could cause an issue.
But this is probably an impossible situation since @AutoConfiguration classes are loaded using the BeanClassLoader.
PR has been updated.
…trics See spring-projectsgh-43956 Signed-off-by: Dmytro Nosan <[email protected]>
|
Now, this implementation looks much better than it was before! Thanks very much, @wilkinsona |
mhalbritter
added
type: task
…trics See gh-43957 Signed-off-by: Dmytro Nosan <[email protected]>
|
Thanks @nosan ! |
|
Thanks, @mhalbritter |
…trics See spring-projectsgh-43957 Signed-off-by: Dmytro Nosan <[email protected]> Signed-off-by: arefbehboudi <[email protected]>
See gh-43956