-
Notifications
You must be signed in to change notification settings - Fork 26.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[3.0] dubbo-spring-boot-actuator compatible with Spring Boot Actuator 2.6.x #9426
Conversation
@@ -45,41 +46,47 @@ | |||
|
|||
@Bean | |||
@ConditionalOnMissingBean | |||
@ConditionalOnAvailableEndpoint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ConditionalOnAvailableEndpoint
is available sine Spring Boot 2.2.0, see https://docs.spring.io/spring-boot/docs/current/api/org/springframework/boot/actuate/autoconfigure/endpoint/condition/ConditionalOnAvailableEndpoint.html
If we reference @ConditionalOnAvailableEndpoint
directly, it may cause loading failed on Spring Boot version < 2.2.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems missing annotation class will not cause ClassNotFoundException, please check it on Spring Boot version < 2.2.0.
.map(Condition.class::cast) // Cast the instance to be Condition one | ||
.orElse(NegativeCondition.INSTANCE); // Or else get a negative condition | ||
|
||
return condition.matches(context, metadata); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please try fix error of condition.matches(context, metadata)
of OnAvailableEndpointCondition
by add required attributes or re-construct the metadata, to compatible with spring boot 2.2.0+ and 2.6.1+.
//OnAvailableEndpointCondition of Spring Boot 2.6.1
@Override
public ConditionOutcome getMatchOutcome(ConditionContext context, AnnotatedTypeMetadata metadata) {
Environment environment = context.getEnvironment();
MergedAnnotation<ConditionalOnAvailableEndpoint> conditionAnnotation = metadata.getAnnotations()
.get(ConditionalOnAvailableEndpoint.class);
Class<?> target = getTarget(context, metadata, conditionAnnotation);
MergedAnnotation<Endpoint> endpointAnnotation = getEndpointAnnotation(target);
return getMatchOutcome(environment, conditionAnnotation, endpointAnnotation);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gitchenjh If it can work in Spring Boot <2.2.0, it is more compatible to use ConditionalOnAvailableEndpoint directly. The only problem is that the code is a bit difficult to understand.
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { | ||
ClassLoader classLoader = context.getClassLoader(); | ||
|
||
Condition condition = Stream.of(CONDITION_CLASS_NAMES) // Iterate class names | ||
.filter(className -> ClassUtils.isPresent(className, classLoader)) // Search class existing or not by name | ||
.findFirst() // Find the first candidate | ||
.map(className -> ClassUtils.resolveClassName(className, classLoader)) // Resolve class name to Class | ||
.filter(Condition.class::isAssignableFrom) // Accept the Condition implementation | ||
.map(BeanUtils::instantiateClass) // Instantiate Class to be instance | ||
.map(Condition.class::cast) // Cast the instance to be Condition one | ||
.orElse(NegativeCondition.INSTANCE); // Or else get a negative condition | ||
|
||
return condition.matches(context, metadata); | ||
} | ||
|
||
private static class NegativeCondition implements Condition { | ||
|
||
static final NegativeCondition INSTANCE = new NegativeCondition(); | ||
|
||
@Override | ||
public boolean matches(ConditionContext context, AnnotatedTypeMetadata metadata) { | ||
return false; | ||
if (ClassUtils.isPresent(CONDITION_CLASS_NAME_OLD, classLoader)) { | ||
Class<?> cls = ClassUtils.resolveClassName(CONDITION_CLASS_NAME_OLD, classLoader); | ||
if (Condition.class.isAssignableFrom(cls)) { | ||
Condition condition = Condition.class.cast(BeanUtils.instantiateClass(cls)); | ||
return condition.matches(context, metadata); | ||
} | ||
} | ||
// Check by org.springframework.boot.actuate.autoconfigure.endpoint.condition.ConditionalOnAvailableEndpoint | ||
if (ClassUtils.isPresent(CONDITION_CLASS_NAME_NEW, classLoader)) { | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good. The CompatibleOnEnabledEndpointCondition
only delegate OnEnabledEndpointCondition
, and do nothing with OnAvailableEndpointCondition
. In this case, it looks clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is CompatibleOnEnabledEndpointCondition#matches(context, metadata)
return true if OnEnabledEndpointCondition
class were not found.
But I'm not sure is there a possibility that both OnEnabledEndpointCondition
and OnAvailableEndpointCondition
class were not found(eg. Spring Boot 1) . So I let this method return false by default, if OnAvailableEndpointCondition class were found, then return true, and @ConditionalOnAvailableEndpoint
should work
Codecov Report
@@ Coverage Diff @@
## 3.0 #9426 +/- ##
============================================
+ Coverage 65.21% 65.26% +0.04%
- Complexity 341 342 +1
============================================
Files 1202 1202
Lines 51950 51950
Branches 7739 7739
============================================
+ Hits 33880 33903 +23
+ Misses 14434 14415 -19
+ Partials 3636 3632 -4
Continue to review full report at Codecov.
|
What is the purpose of the change
fix #9394
Brief changelog
CompatibleOnEnabledEndpointCondition only handle
org.springframework.boot.actuate.autoconfigure.endpoint.condition.OnEnabledEndpointCondition
(Spring Boot 2.0.0 ~ 2.2.x)Endpint beans add @ConditionalOnAvailableEndpoint derectly (Spring Boot 2.2x +)
Verifying this change
you can verify by this case: https://github.com/gitchenjh/dubbo-test
Checklist