-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
Add support for managing gateway flow rules and customized API group in Sentinel dashboard #869
Conversation
👍 |
Codecov Report
@@ Coverage Diff @@
## master #869 +/- ##
===========================================
- Coverage 42.57% 42.48% -0.1%
+ Complexity 1437 1436 -1
===========================================
Files 310 310
Lines 8974 8987 +13
Branches 1217 1219 +2
===========================================
- Hits 3821 3818 -3
- Misses 4685 4699 +14
- Partials 468 470 +2
Continue to review full report at Codecov.
|
...mmon/src/main/java/com/alibaba/csp/sentinel/adapter/gateway/common/rule/GatewayFlowRule.java
Outdated
Show resolved
Hide resolved
...mmon/src/main/java/com/alibaba/csp/sentinel/adapter/gateway/common/rule/GatewayFlowRule.java
Outdated
Show resolved
Hide resolved
...n/src/main/java/com/alibaba/csp/sentinel/adapter/gateway/common/rule/GatewayRuleManager.java
Outdated
Show resolved
Hide resolved
<div> | ||
<label class="col-sm-2 control-label">间隔</label> | ||
<div class="col-sm-3"> | ||
<input type='number' min="1" class="form-control highlight-border" ng-model='currentRule.interval' placeholder="单位(秒)" /> |
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.
Maybe we could update the placeholder here?
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.
Meticulous! Yes, it should be updated when user change different interval unit.
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.
Oh, as the select after the input displayed the interval unit, how about the placeholder here just "请输入"?
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.
Fixed, with tip text 统计窗口时间长度
in this placeholder.
<label class="col-sm-5 control-label"> | ||
<div class="form-control highlight-border" align="center"> | ||
<input type="radio" value="0" checked ng-model="currentRule.paramItem.matchStrategy" title="精确" /> 精确 | ||
<input type="radio" value="1" ng-model="currentRule.paramItem.matchStrategy" title="前缀" /> 前缀  |
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.
matchStrategy 中前缀模式不需要,我们只支持这几种就可以:
- 精确:0
- 子串:3
- 正则:2
对应的值可参考 SentinelGatewayConstants
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.
matchStrategy中的前缀已去掉,目前3种matchStrategy 精确:0 子串:1 正则:2
对应的界面和SentinelGatewayConstants
类都已修改。
}; | ||
|
||
function saveRule(rule, edit) { | ||
GatewayFlowService.saveRule(rule).success(function (data) { |
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.
Maybe we need to check whether the rule is valid here?
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.
In 115 line, before calling addNewRule
and saveRule
, call GatewayFlowService.checkRuleValid($scope.currentRule)
to check, and the check method has been improved.
|
||
private String resource; | ||
private Integer resourceMode; | ||
|
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.
Missing grade
here?
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.
Yes, grade
propertity has been added.
sentinel-dashboard/src/main/webapp/resources/app/scripts/controllers/gateway/flow.js
Outdated
Show resolved
Hide resolved
Fabulous! Just one more advice: we need to keep the customized API name unique. We could add validation in the "new API definition" dialog to check whether the current API list contains the API group name. |
Good idea, the customized API name should be unique. |
The unique validation for customized API name has been added, both front and back ends. Since the api name can't be modified when editing, only validate when adding new customized API . |
.../src/main/java/com/alibaba/csp/sentinel/adapter/gateway/common/SentinelGatewayConstants.java
Outdated
Show resolved
Hide resolved
...control/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/param/ParamFlowRuleUtil.java
Outdated
Show resolved
Hide resolved
@@ -111,7 +111,8 @@ gulp.task('clean', function () { | |||
}); | |||
|
|||
// 总任务 | |||
gulp.task('build', ['clean', 'jshint', 'lib', 'js', 'css']); | |||
// gulp.task('build', ['clean', 'jshint', 'lib', 'js', 'css']); | |||
gulp.task('build', ['clean', 'lib', 'js', 'css']); |
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.
Why is the jshint
task removed?
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.
Oh I'm sorry for commit local modify file, I'll fix it soon.
Nice work! Could you please squash and separate your commits according to the corresponding module? For the commit related to the dashboard, it can be prefixed with |
… of dashboard in awesome-sentinel.md
…eGatewayApiDefinitionGroupCommandHandler
b80fedb
to
a2fce6f
Compare
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 your awesome contribution! |
Signed-off-by: Eric Zhao <[email protected]>
Signed-off-by: Eric Zhao <[email protected]> (cherry picked from commit e50be35)
[RIP-10]fix-bug: ScheduleMessageServiceTest
Describe what this PR does / why we need it
Support managing gateway flow rules and customized API group definitions via dashboard.
Does this pull request fix one issue?
Fixes #699
Describe how you did it
Use
appType
in heartbeat message of client to distinguish normal service or gateway service.Add
appType
inAppInfo
andMachineInfo
class, add two menus for gateway type app insidebar.html
.Add
GatewayApiController
,GatewayFlowRuleController
and corresponding entity,repository,voin gateway package(controller/gateway,datasource/entity/gateway,repository/gateway,vo/gateway).
Add fetch and modify method in
SentinelApiClient
to interactive with gateway command handler.Add
api.html
,flow.html
and js files in corresponding gateway folder.Describe how to verify it
Start dashboard with VM argument:
-Dproject.name=sentinel-dashboard -Dcsp.sentinel.dashboard.server=localhost:8080
Start sentinel-demo-spring-cloud-gateway with VM argument:
-Dproject.name=sc-gateway-demo -Dcsp.sentinel.dashboard.server=localhost:8080 -Dcsp.sentinel.app.type=1
Visit http://localhost:8090/httpbin/json to make sentinel init in
sc-gateway-demo.
View and operate list/add/update/delete API group and gateway flow rules via dashboard pages,
check sc-gateway-demo's api groups or flow rules by visiting http://localhost:8720/gateway/getApiDefinitions,http://localhost:8720/gateway/getRules
Check via UI and view logs to verify.
A simple test class
ApiDefinitionControllerTest
is added, I'll try to improve the unit test cases later.Special notes for reviews
To make the gateway flow rules in order, the type of rule map in
GatewayRuleManager
class is changed fromConcurrentHashMap
toCollections.synchronizedMap(new LinkedHashMap()
HashSet
toLinkedhashSet
.In this way, when user add or update one rule, then refresh the list page, the order of rules can remain
(in adding order).
To support set interval by different unit of second,minute,hour,day via dashboard, a property named
intervalUnit
is added inGatewayFlowRule
class, andintervalSec
change tointerval
, thecalIntervalSec
method is added to get the interval second byinterval
andintervalUnit
.Previously, a variable name in
login.js
isn't a good naming, though it doesn't affect functionality, I change theLoginService
toAuthService
, to make the meaning accurate.Besides, I post a blog about how to build the front-end developing environment of sentinel dashboard, which may help users who are in first participation, and add it in awesome-sentinel.md.
PTAL.