fix the method of dashboard invoking cluster api #829

Yisaki · 2019-06-13T03:25:55Z

Describe what this PR does / why we need it

被限流服务使用集群限流嵌入模式（Embedded）时，在dashborad的集群流控->新增Token Server页面，点击保存按钮后调用被限流节点以下两个api时，应该用get请求，否则会报empty data错误
http://ip:port/cluster/client/modifyConfig
http://ip:port/cluster/server/modifyFlowConfig

被限流服务为springboot应用,使用如下依赖：
org.springframework.cloud
spring-cloud-starter-alibaba-sentinel
0.9.0.RELEASE

com.alibaba.csp
sentinel-cluster-client-default
1.6.1

com.alibaba.csp
sentinel-cluster-server-default
1.6.1

…i时，应该用get请求 http://<ip>:<port>/cluster/client/modifyConfig http://<ip>:<port>/cluster/server/modifyFlowConfig

CLAassistant · 2019-06-13T03:26:03Z

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.

黄星淇 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.}

codecov-io · 2019-06-13T03:31:11Z

Codecov Report

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

@@             Coverage Diff              @@
##             master     #829      +/-   ##
============================================
+ Coverage     41.73%   42.82%   +1.08%     
- Complexity     1377     1481     +104     
============================================
  Files           304      304              
  Lines          8764     9195     +431     
  Branches       1182     1296     +114     
============================================
+ Hits           3658     3938     +280     
- Misses         4660     4782     +122     
- Partials        446      475      +29

Impacted Files	Coverage Δ	Complexity Δ
...apter/gateway/common/param/GatewayParamParser.java	`49.42% <0%> (-18.83%)`	`17% <0%> (ø)`
...src/main/java/com/alibaba/csp/sentinel/Tracer.java	`22.35% <0%> (-8.8%)`	`12% <0%> (ø)`
...er/gateway/zuul/filters/SentinelZuulPreFilter.java	`12.72% <0%> (-1.86%)`	`3% <0%> (ø)`
...a/com/alibaba/csp/sentinel/node/StatisticNode.java	`54.03% <0%> (-1.65%)`	`36% <0%> (+9%)`
...pter/gateway/common/rule/GatewayRuleConverter.java	`100% <0%> (ø)`	`4% <0%> (+1%)`	⬆️
.../main/java/com/alibaba/csp/sentinel/Constants.java	`100% <0%> (ø)`	`1% <0%> (ø)`	⬇️
...libaba/csp/sentinel/slots/block/flow/FlowSlot.java	`100% <0%> (ø)`	`12% <0%> (+7%)`	⬆️
...inel/slots/block/flow/param/ParamFlowRuleUtil.java	`61.94% <0%> (+0.4%)`	`49% <0%> (+19%)`	⬆️
.../sentinel/slots/nodeselector/NodeSelectorSlot.java	`95.65% <0%> (+0.91%)`	`5% <0%> (+1%)`	⬆️
...a/csp/sentinel/slots/system/SystemRuleManager.java	`44.09% <0%> (+0.93%)`	`13% <0%> (+6%)`	⬆️
... and 7 more

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 f762ba2...eae41b6. Read the comment docs.

sczyh30 · 2019-06-13T03:31:20Z

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-06-13T03:32:29Z

Could you please illustrate why we need this PR?

Yisaki · 2019-06-13T04:01:17Z

提交分支选错了。。

sczyh30 · 2019-06-13T04:02:23Z

The target branch should be master.

Yisaki · 2019-06-13T10:32:34Z

再试试。。。

sczyh30 · 2019-06-14T01:22:21Z

Could you please illustrate why we need this PR?

jasonjoo2010 · 2019-06-14T01:27:21Z

@Yisaki

Did you find some backward compatibility problem on it? The data need be submitted in post because they will become longer and longer finally will exceed the limit of URL can afford. And the client will make the final decision whether to use POST method when executing according the target sentinel-core versions(Maybe bigger then 1.6.0).

Yisaki · 2019-06-14T10:10:45Z

@jasonjoo2010
我英文不太好，还是用中文吧
And the client will make the final decision whether to use POST method...
这段逻辑我看到了，之前没看到-_-!，URL会超出长度的我也很认同，所以我这个改的不对

但是页面会报错的问题确实存在，经测试sentinel-core 1.6.x(测了1.6.0和1.6.1) 的那两个API只接收GET请求，而dashborad 1.6.1发送的是POST请求，原因在于dashborad判断版本选择POST还是GET那块逻辑，即你说的backward compatibility problem，我再来试着改改看

第一次在github提交开源项目有的地方不是特别懂，总之感谢对我提的问题的反馈

jasonjoo2010 · 2019-06-14T10:20:48Z

@jasonjoo2010
我英文不太好，还是用中文吧
And the client will make the final decision whether to use POST method...
这段逻辑我看到了，之前没看到-_-!，URL会超出长度的我也很认同，所以我这个改的不对

但是页面会报错的问题确实存在，经测试sentinel-core 1.6.x(测了1.6.0和1.6.1) 的那两个API只接收GET请求，而dashborad 1.6.1发送的是POST请求，原因在于dashborad判断版本选择POST还是GET那块逻辑，即你说的backward compatibility problem，我再来试着改改看

第一次在github提交开源项目有的地方不是特别懂，总之感谢对我提的问题的反馈

那么如果如你描述，问题应该是出在了集群限流api这边，不支持post方式，但看了下代码，这里应该正常走transport了，你用的是哪个transport?

Yisaki · 2019-06-14T11:16:55Z

那么如果如你描述，问题应该是出在了集群限流api这边，不支持post方式，但看了下代码，这里应该正常走transport了，你用的是哪个transport?

transport应该是由这个starter引入的，看了下是sentinel-transport-simple-http-1.5.2.jar和sentinel-transport-common-1.5.2.jar

        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-starter-alibaba-sentinel</artifactId>
            <version>0.9.0.RELEASE</version>
        </dependency>

所有跟sentinel的依赖

        <!--sentinel start-->
        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-starter-alibaba-sentinel</artifactId>
            <version>0.9.0.RELEASE</version>
        </dependency>
        <!--apollo推送限流规则-->
        <dependency>
            <groupId>com.alibaba.csp</groupId>
            <artifactId>sentinel-datasource-apollo</artifactId>
            <version>1.6.1</version>
        </dependency>
        <!--集群限流依赖-->
        <dependency>
            <groupId>com.alibaba.csp</groupId>
            <artifactId>sentinel-cluster-client-default</artifactId>
            <version>1.6.1</version>
        </dependency>
        <dependency>
            <groupId>com.alibaba.csp</groupId>
            <artifactId>sentinel-cluster-server-default</artifactId>
            <version>1.6.1</version>
        </dependency>
        <!--sentinel end-->

然后我又改了下dashborad，高于1.6.1才会使用POST请求，在公司的测试环境可以调通

jasonjoo2010 · 2019-06-14T11:29:10Z

那么如果如你描述，问题应该是出在了集群限流api这边，不支持post方式，但看了下代码，这里应该正常走transport了，你用的是哪个transport?

transport应该是由这个starter引入的，看了下是sentinel-transport-simple-http-1.5.2.jar和sentinel-transport-common-1.5.2.jar

        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-starter-alibaba-sentinel</artifactId>
            <version>0.9.0.RELEASE</version>
        </dependency>

所有跟sentinel的依赖

        <!--sentinel start-->
        <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-starter-alibaba-sentinel</artifactId>
            <version>0.9.0.RELEASE</version>
        </dependency>
        <!--apollo推送限流规则-->
        <dependency>
            <groupId>com.alibaba.csp</groupId>
            <artifactId>sentinel-datasource-apollo</artifactId>
            <version>1.6.1</version>
        </dependency>
        <!--集群限流依赖-->
        <dependency>
            <groupId>com.alibaba.csp</groupId>
            <artifactId>sentinel-cluster-client-default</artifactId>
            <version>1.6.1</version>
        </dependency>
        <dependency>
            <groupId>com.alibaba.csp</groupId>
            <artifactId>sentinel-cluster-server-default</artifactId>
            <version>1.6.1</version>
        </dependency>
        <!--sentinel end-->

然后我又改了下dashborad，高于1.6.1才会使用POST请求，在公司的测试环境可以调通

所以经你的描述，是说1.6.1的dashboard配1.6.0的节点版本不兼容对吗？

因为PR合的时机实在是不可控，所以这个版本兼容一改再改，我没有去验，但是如果照这个描述，那就是1.6.0 release时没有合入这个transport支持POST的PR了，那就应该提升兼容版本至最低的那个

jasonjoo2010 · 2019-06-17T01:36:02Z

我看到1.6.0的release说明中的确已经包括了POST支持。

而在你这个场景中，1.5.2的客户端，应该过不了这个老的160的版本号判断的，所以会走get才对，你确定你改的是对的？

Yisaki · 2019-06-17T03:21:04Z

@jasonjoo2010
照你这样说的话不能这样改
但是在限流节点使用我上边贴的依赖，配1.6.1的dashborad，确实发的POST请求(dashborad中机器列表页面显示的sentinel客户端版本是1.6.0)
你可以拿我上边的依赖配1.6.1的dashborad做测试，dashborad页面会报错

这个starter

     <dependency>
            <groupId>org.springframework.cloud</groupId>
            <artifactId>spring-cloud-starter-alibaba-sentinel</artifactId>
            <version>0.9.0.RELEASE</version>
        </dependency>

拉下的jar包core是sentinel-core-1.6.0.jar，但transport是sentinel-transport-simple-http-1.5.2.jar

所以问题是不是出在这个starter，拉下的transport应该是sentinel-transport-simple-http-1.6.x.jar

jasonjoo2010 · 2019-06-17T03:47:25Z

那这里应该有个依赖冲突的问题，SCA有自己的sentinel依赖版本，而且基本引用了所有的sentinel依赖，应该不需要你手工在项目中继续引用sentinel-transport*了，你去掉再试一下应该就好了 best regards, Jason

…

On Jun 17, 2019, at 11:21, 大和田 ***@***.***> wrote: 照你这样说的话应该不能这样改但是在限流节点使用我上边贴的依赖，配1.6.1的dashborad，确实发的POST请求(dashborad中机器列表页面显示的sentinel客户端版本是1.6.0) 你可以拿我上边的依赖配1.6.1的dashborad做测试，dashborad页面会报错这个starter <dependency> <groupId>org.springframework.cloud</groupId> <artifactId>spring-cloud-starter-alibaba-sentinel</artifactId> <version>0.9.0.RELEASE</version> </dependency> 拉下的jar包core是sentinel-core-1.6.0.jar，但transport是sentinel-transport-simple-http-1.5.2.jar 所以问题是不是出在这个starter，拉下的transport应该是sentinel-transport-simple-http-1.6.x.jar — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#829?email_source=notifications&email_token=AEGCJUK7HXHMANUP3UGT6KLP237LRA5CNFSM4HXVSJW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXZ5YVI#issuecomment-502520917>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEGCJUL63X6IQAVBWK7QMEDP237LRANCNFSM4HXVSJWQ>.

sczyh30 · 2019-06-17T03:53:51Z

I recalled that there was a similar issue (#762). Both might be due to the version mismatch of sentinel-core and sentinel-transport-common, of which the version is restricted by Spring Cloud Alibaba (it uses dependencyManagement to restrict the version of Sentinel).

Yisaki · 2019-06-17T08:04:36Z

@sczyh30 是的，我碰到的问题跟他一样，我来问问他

sczyh30 · 2019-06-17T08:08:49Z

In most circumstances, the version of the following dependencies should keep consistent with each other:

sentinel-core
sentinel-transport-common
sentinel-datasource-extension
sentinel-parameter-flow-control

* Add test case for CommitLog.handleHA * Add one UUID parent path for RocketMQ's files

使用集群限流嵌入模式（Embedded）时，在dashborad的集群流控->新增Token Server页面，调用被限流节点以下两个ap…

eae41b6

…i时，应该用get请求 http://<ip>:<port>/cluster/client/modifyConfig http://<ip>:<port>/cluster/server/modifyFlowConfig

sczyh30 added area/cluster-flow Issues or PRs related to cluster flow control to-review To review labels Jun 13, 2019

Yisaki closed this Jun 13, 2019

sczyh30 mentioned this pull request Jun 13, 2019

fix the method of dashboard invoking cluster api #830

Closed

Yisaki reopened this Jun 13, 2019

compatible with older version

eb95988

Yisaki closed this Jun 17, 2019

sczyh30 added area/dashboard Issues or PRs about Sentinel Dashboard and removed area/cluster-flow Issues or PRs related to cluster flow control to-review To review labels Jun 17, 2019

CST11021 pushed a commit to CST11021/Sentinel that referenced this pull request Nov 3, 2021

[RIP-10]Add test case for CommitLog.handleHA (alibaba#829)

6bece12

* Add test case for CommitLog.handleHA * Add one UUID parent path for RocketMQ's files

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix the method of dashboard invoking cluster api #829

fix the method of dashboard invoking cluster api #829

Yisaki commented Jun 13, 2019 •

edited

CLAassistant commented Jun 13, 2019 •

edited

codecov-io commented Jun 13, 2019 •

edited

sczyh30 commented Jun 13, 2019

sczyh30 commented Jun 13, 2019

Yisaki commented Jun 13, 2019

sczyh30 commented Jun 13, 2019

Yisaki commented Jun 13, 2019

sczyh30 commented Jun 14, 2019

jasonjoo2010 commented Jun 14, 2019

Yisaki commented Jun 14, 2019

jasonjoo2010 commented Jun 14, 2019

Yisaki commented Jun 14, 2019

jasonjoo2010 commented Jun 14, 2019

jasonjoo2010 commented Jun 17, 2019

Yisaki commented Jun 17, 2019 •

edited

jasonjoo2010 commented Jun 17, 2019 via email

sczyh30 commented Jun 17, 2019

Yisaki commented Jun 17, 2019

sczyh30 commented Jun 17, 2019

fix the method of dashboard invoking cluster api #829

fix the method of dashboard invoking cluster api #829

Conversation

Yisaki commented Jun 13, 2019 • edited

Describe what this PR does / why we need it

CLAassistant commented Jun 13, 2019 • edited

codecov-io commented Jun 13, 2019 • edited

Codecov Report

sczyh30 commented Jun 13, 2019

sczyh30 commented Jun 13, 2019

Yisaki commented Jun 13, 2019

sczyh30 commented Jun 13, 2019

Yisaki commented Jun 13, 2019

sczyh30 commented Jun 14, 2019

jasonjoo2010 commented Jun 14, 2019

Yisaki commented Jun 14, 2019

jasonjoo2010 commented Jun 14, 2019

Yisaki commented Jun 14, 2019

jasonjoo2010 commented Jun 14, 2019

jasonjoo2010 commented Jun 17, 2019

Yisaki commented Jun 17, 2019 • edited

jasonjoo2010 commented Jun 17, 2019 via email

sczyh30 commented Jun 17, 2019

Yisaki commented Jun 17, 2019

sczyh30 commented Jun 17, 2019

Yisaki commented Jun 13, 2019 •

edited

CLAassistant commented Jun 13, 2019 •

edited

codecov-io commented Jun 13, 2019 •

edited

Yisaki commented Jun 17, 2019 •

edited