Fix the problem that dashboard do not correctly show the SystemRule in CPU usage strategy #922

Crazy10552 · 2019-07-16T12:12:11Z

it fix the bug that dashboard do not correctly show the system rule(the rule about properties highestCpuUsage)

Does this pull request fix one issue?

Fixes #905

Describe how you did it

I add new properties in entity [com.alibaba.csp.sentinel.dashboard.datasource.entity.rule.SystemRuleEntity]
I update the related js and html about System rule

Describe how to verify it

Special notes for reviews

None

CLAassistant · 2019-07-16T12:12:13Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Crazy10552 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

sczyh30 · 2019-07-16T14:17:56Z

Hi, thanks for contributing. Could you please sign the CLA here? And please make sure the email of your commits match your GitHub email.

感谢贡献，请将 commit 对应的 email 调整成与 GitHub 的 email 相匹配并确认一下 CLA。

sczyh30 · 2019-07-16T14:18:46Z

...rc/main/java/com/alibaba/csp/sentinel/dashboard/datasource/entity/rule/SystemRuleEntity.java

@@ -33,6 +33,7 @@
    private Long avgRt;
    private Long maxThread;
    private Double qps;
+    private Double avgCpu;


Maybe it's better to be maxCpuUsage?

I saw properties in SystemRule named highestSystemLoad and it named avgLoad in SystemRuleEntity ,so i just do like to name highestCpuUsage in SystemRule to avgCpu in SystemRuleEntity

Other legacy fields might not be good enough... For CPU usage it's an instantaneous value rather than average value.

sczyh30 · 2019-07-16T14:19:14Z

sentinel-dashboard/src/main/webapp/resources/app/scripts/controllers/system.js

@@ -119,6 +121,9 @@ app.controller('SystemCtl', ['$scope', '$stateParams', 'SystemService', 'ngDialo
      } else if (rule.qps != -1) {
        ruleTypeDesc = 'QPS';
        ruleTypeCount = rule.qps;
+      }else if (rule.avgCpu != -1) {


Ditto (maxCpuUsage)

sczyh30 · 2019-07-16T14:19:44Z

sentinel-dashboard/src/main/webapp/resources/app/views/system.html

@@ -49,12 +49,14 @@
                  <span ng-if="rule.avgRt >= 0">RT</span>
                  <span ng-if="rule.maxThread >= 0">线程数</span>
                  <span ng-if="rule.qps >= 0">QPS</span>
+                  <span ng-if="rule.avgCpu >= 0">CPU</span>


叫 CPU 使用率 可能更合适些？

i know it's much better but i just confused by the AvgLoad ，i can fix both two of them

It's indeed confusing. You could fix that.

I left a problem that i am not good at,while i try to choose another ,it does not show like i want

…(原avgLoad) 属性名称错误的命名

sczyh30 · 2019-07-17T12:26:51Z

...-dashboard/src/main/java/com/alibaba/csp/sentinel/dashboard/controller/SystemController.java

-        if (avgLoad != null) {
-            if (avgLoad < 0) {
-                return Result.ofFail(-1, "avgLoad must >= 0");
+        if (highestSystemLoad != null) {


highestSystemLoad 的取值范围是 [0, +∞)，load 没有 < 1 的限制。这个需要保持与之前一致。

codecov-io · 2019-07-17T12:27:03Z

Codecov Report

Merging #922 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #922      +/-   ##
============================================
+ Coverage     42.54%   42.55%   +0.01%     
- Complexity     1441     1442       +1     
============================================
  Files           310      310              
  Lines          8993     8993              
  Branches       1222     1222              
============================================
+ Hits           3826     3827       +1     
  Misses         4698     4698              
+ Partials        469      468       -1

Impacted Files	Coverage Δ	Complexity Δ
...ava/com/alibaba/csp/sentinel/node/ClusterNode.java	`100% <0%> (+4.76%)`	`8% <0%> (+1%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7dd20dd...44f69d9. Read the comment docs.

sczyh30 · 2019-07-17T12:29:31Z

sentinel-dashboard/src/main/webapp/resources/app/views/dialog/system-rule-dialog.html

-                <!--avgLoad -->
-                <input type="radio" name="grade" value="0" ng-model='currentRule.grade' ng-disabled="systemRuleDialog.type == 'edit'" />&nbsp;LOAD&nbsp;&nbsp;
+                <!--highestSystemLoad -->
+                <input type="radio" name="grade" value="0" ng-model='currentRule.grade' ng-disabled="systemRuleDialog.type == 'edit'" />&nbsp;最大系统LOAD&nbsp;&nbsp;


这里是阈值，不需要强调”最大“，这里直接叫 Load 即可

sczyh30 · 2019-07-17T12:29:51Z

sentinel-dashboard/src/main/webapp/resources/app/views/dialog/system-rule-dialog.html

-              />
-              <input type='number' class="form-control highlight-border" ng-model='currentRule.qps' placeholder="[0, ~)的小数" ng-if="currentRule.grade == 3"
-              />
+              <input type='number' class="form-control highlight-border" ng-model='currentRule.highestSystemLoad' placeholder="[0, 1)的小数" ng-if="currentRule.grade == 0"/>


load 的取值保持与之前一致

sczyh30 · 2019-07-17T12:30:00Z

sentinel-dashboard/src/main/webapp/resources/app/views/system.html

@@ -45,16 +45,18 @@
              <tr dir-paginate="rule in rules | filter : searchKey | itemsPerPage: rulesPageConfig.pageSize " current-page="rulesPageConfig.currentPageIndex"
                pagination-id="entriesPagination">
                <td style="word-wrap:break-word;word-break:break-all;">
-                  <span ng-if="rule.avgLoad >= 0">LOAD</span>
+                  <span ng-if="rule.highestSystemLoad >= 0">最大系统LOAD</span>


sczyh30 · 2019-07-17T12:30:04Z

sentinel-dashboard/src/main/webapp/resources/app/views/system.html

                  <span ng-if="rule.avgRt >= 0">RT</span>
                  <span ng-if="rule.maxThread >= 0">线程数</span>
                  <span ng-if="rule.qps >= 0">QPS</span>
+                  <span ng-if="rule.highestCpuUsage >= 0">最大CPU使用率</span>


sczyh30 · 2019-07-17T12:33:23Z

sentinel-dashboard/src/main/webapp/resources/app/views/dialog/system-rule-dialog.html

-                />&nbsp;入口 QPS
+                <input type="radio" name="grade" value="3" ng-model='currentRule.grade' ng-disabled="systemRuleDialog.type == 'edit'"/>&nbsp;入口 QPS
+                <!--highestCpuUsage -->
+                <input type="radio" name="grade" value="4" checked ng-model='currentRule.grade' ng-disabled="systemRuleDialog.type == 'edit'" />&nbsp;最大CPU使用&nbsp;&nbsp;


CPU 使用率

sczyh30 · 2019-07-17T14:06:56Z

另外需要将 commit 对应的 email 调整成与 GitHub 的 email 相匹配并确认一下 CLA 。可以参考：https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user

…oard-issues-905 # Conflicts: # sentinel-dashboard/src/main/webapp/resources/app/scripts/controllers/system.js # sentinel-dashboard/src/main/webapp/resources/app/views/dialog/system-rule-dialog.html # sentinel-dashboard/src/main/webapp/resources/app/views/system.html

本次提交是为了解决alibaba#905 上面 dashboard 无法正常显示system rule 的cpu部分配置规则信息的bug

7718a5d

sczyh30 changed the title ~~it fix the bug that dashboard do not correctly show the system rule connect whith issue 905~~ Fix the problem that dashboard do not correctly show the SystemRule in CPU usage strategy Jul 16, 2019

sczyh30 reviewed Jul 16, 2019

View reviewed changes

sczyh30 added the area/dashboard Issues or PRs about Sentinel Dashboard label Jul 16, 2019

按照修改意见修复SystemRuleEntity 中 highestCpuUsage(原avgCpu) highestSystemLoad…

d00857d

…(原avgLoad) 属性名称错误的命名

sczyh30 requested changes Jul 17, 2019

View reviewed changes

修正理解错误,highestSystemLoad 的取值范围是 [0, +∞)

bcf8954

sczyh30 reviewed Jul 17, 2019

View reviewed changes

调整页面显示文字

dd1dd15

Crazy10552 added 2 commits July 18, 2019 09:10

本次提交是为了解决alibaba#905 上面 dashboard 无法正常显示system rule 的cpu部分配置规则信息的bug

0ae8ffa

Crazy10552 closed this Jul 18, 2019

Crazy10552 deleted the dashboard-issues-905 branch July 18, 2019 01:20

Crazy10552 mentioned this pull request Jul 18, 2019

Fix the problem that dashboard do not correctly show the SystemRule in CPU usage strategy #927

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix the problem that dashboard do not correctly show the SystemRule in CPU usage strategy #922

Fix the problem that dashboard do not correctly show the SystemRule in CPU usage strategy #922

Crazy10552 commented Jul 16, 2019

CLAassistant commented Jul 16, 2019

sczyh30 commented Jul 16, 2019

sczyh30 Jul 16, 2019

Crazy10552 Jul 17, 2019

sczyh30 Jul 17, 2019 •

edited

Loading

sczyh30 Jul 16, 2019

sczyh30 Jul 16, 2019

Crazy10552 Jul 17, 2019

sczyh30 Jul 17, 2019

Crazy10552 Jul 17, 2019

sczyh30 Jul 17, 2019

codecov-io commented Jul 17, 2019 •

edited

Loading

sczyh30 Jul 17, 2019

Crazy10552 Jul 17, 2019

sczyh30 Jul 17, 2019

sczyh30 Jul 17, 2019

sczyh30 Jul 17, 2019

sczyh30 Jul 17, 2019

sczyh30 commented Jul 17, 2019

Fix the problem that dashboard do not correctly show the SystemRule in CPU usage strategy #922

Fix the problem that dashboard do not correctly show the SystemRule in CPU usage strategy #922

Conversation

Crazy10552 commented Jul 16, 2019

it fix the bug that dashboard do not correctly show the system rule(the rule about properties highestCpuUsage)

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

CLAassistant commented Jul 16, 2019

sczyh30 commented Jul 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sczyh30 Jul 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jul 17, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sczyh30 commented Jul 17, 2019

sczyh30 Jul 17, 2019 •

edited

Loading

codecov-io commented Jul 17, 2019 •

edited

Loading