feat: support Sentinel flow control for RestClient - #4273
Conversation
- Add SentinelRestClientInterceptor to intercept RestClient requests - Add SentinelRestClientBeanPostProcessor to auto-inject interceptor - Add SentinelRestClientHttpResponse for blocked request responses - Support flow control (QPS limiting) and circuit breaking (degrade) - Support custom blockHandler and fallback handlers - Support URL cleaner for RESTful path normalization Change-Id: Iedbe1dbe377eb5e1a6531c86c9c9850adcad12fb Co-developed-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
uuuyuqi
changed the title
RestClientRestClient
…er for each RestClient Change-Id: Id9c65c5e44e97ffbbee804265db2f0bcf59d7ce0 Co-developed-by: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
uuuyuqi
changed the title
RestClientRestClient
| DegradeRule degradeRule2 = new DegradeRule("GET:https://httpbin.org/delay/3"); | ||
| degradeRule2.setGrade(RuleConstant.DEGRADE_GRADE_RT); | ||
| degradeRule2.setCount(2000); | ||
| degradeRule1.setStatIntervalMs(10 * 1000); |
| */ | ||
| @AutoConfiguration | ||
| @ConditionalOnClass(RestClient.class) | ||
| @ConditionalOnProperty(prefix = "spring.cloud.sentinel", name = "enabled", |
There was a problem hiding this comment.
是否需要考虑 sentinel.restclient.enabled
|
This PR has conflicts with the base branch and cannot be merged. Please rebase or merge the base branch into your branch and resolve the conflicts: git fetch origin
git checkout uuuyuqi/2025.1.x-sentinel-restclient-support
git rebase origin/2025.1.x
# resolve conflicts, then:
git push --force-with-leaseThis is a one-time reminder. Feel free to @mention me for a re-review after conflicts are resolved. Automated notification by github-manager-bot |
oss-sentinel-ai
left a comment
oss-sentinel-ai
left a comment
There was a problem hiding this comment.
Summary
This PR adds Sentinel flow control support for Spring 6 RestClient, creating two-level resources (host + path) with configurable block/fallback/urlCleaner handlers. The implementation follows the existing SentinelRestTemplate pattern closely. The code structure is clean and the entry/exit lifecycle is correct. However, the auto-configuration is too aggressive (applies to all RestClient beans by default) and there is no working test coverage.
Note: This PR has merge conflicts with the base branch that need to be resolved before it can be merged.
Suggestions
- Consider making the blanket interceptor opt-in rather than opt-out to avoid surprising existing users
- Add working unit tests (the current test is
@Disabled) - Resolve merge conflicts with
2025.1.x
Automated review by github-manager-bot
| @ConditionalOnClass(RestClient.class) | ||
| @ConditionalOnProperty(prefix = "spring.cloud.sentinel", name = "enabled", | ||
| havingValue = "true", matchIfMissing = true) | ||
| public class SentinelRestClientAutoConfiguration { |
There was a problem hiding this comment.
[Warning] This auto-configuration registers a SentinelRestClientBeanPostProcessor that adds the Sentinel interceptor to every RestClient.Builder bean in the application (when spring.cloud.sentinel.enabled is true, which is the default). This is a broad behavioral change — users who add the Sentinel starter will silently get flow control on all their RestClient calls, even without the @SentinelRestClient annotation.
Consider either:
- Making the auto-configuration opt-in (e.g.
matchIfMissing = false), or - Only applying the interceptor to builders annotated with
@SentinelRestClient(via the BeanPostProcessor), and removing the blanketRestClientBuilderCustomizerbean.
The existing SentinelRestTemplate integration requires explicit @SentinelRestTemplate annotation — this should follow the same pattern for consistency.
| * @author QHT | ||
| */ | ||
| @Disabled("For debugging") | ||
| public class RestClientApplicationTest { |
There was a problem hiding this comment.
[Warning] The only test in this module is @Disabled("For debugging"), providing zero test coverage for the new Sentinel + RestClient integration. Consider adding at least:
- A unit test for
SentinelRestClientInterceptor.intercept()that verifies entry/exit lifecycle and block handling - A test verifying the
@SentinelRestClientannotation correctly wires the interceptor
The existing sentinel-resttemplate-example has working tests that can serve as a template.
|
|
||
| String interceptorBeanName = interceptorBeanNamePrefix + "@" | ||
| + bean.toString(); | ||
| registerBean(interceptorBeanName, sentinelRestClient); |
There was a problem hiding this comment.
[Info] The interceptor bean name includes bean.toString() which may produce unstable or verbose names depending on the RestClient.Builder implementation. Consider using a hash code or a simpler identifier (e.g., the bean name from the @Bean method) to keep the bean registry clean.
4 participants
Describe what this PR does / why we need it
Support Sentinel flow control for
RestClientDoes this pull request fix one issue?
None
This PR builds upon the original work by @QHtttttt . Thank you for the contribution! I have made a few minor fixes on top of it.