-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Support URL exclusion using UrlCleaner in Spring WebFlux adapter #1049
Support URL exclusion using UrlCleaner in Spring WebFlux adapter #1049
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1049 +/- ##
============================================
- Coverage 42.64% 42.61% -0.03%
Complexity 1472 1472
============================================
Files 317 317
Lines 9286 9290 +4
Branches 1267 1269 +2
============================================
- Hits 3960 3959 -1
- Misses 4833 4836 +3
- Partials 493 495 +2
Continue to review full report at Codecov.
|
@@ -55,7 +55,7 @@ For REST APIs, you have to clean the URL resource (e.g. `/foo/1` and `/foo/2` -> | |||
the amount of context and resources will exceed the threshold. | |||
|
|||
If you need to exclude some URLs (that should not be recorded as Sentinel resources), you could also | |||
leverage the `UrlCleaner` interface. You may unify the unwanted URLs to the empty string `""`, | |||
leverage the `UrlCleaner` interface. You may unify the unwanted URLs to the empty string `""` or null, |
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.
This PR is for Spring WebFlux adapter only, so it might be better to make this change in other PRs.
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.
Ok, I will try to change this in another PR.
...ter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webflux/SentinelWebFluxFilter.java
Outdated
Show resolved
Hide resolved
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.
LGTM
Thanks for contributing! |
Describe what this PR does / why we need it
Does this pull request fix one issue?
Resolves #1020
Describe how you did it
Describe how to verify it
Special notes for reviews