Skip to content
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

rule.setResource(resourceName)中的resourceName关于前后空格是否trim在系统中处理不一致 #369

Closed
techzealot opened this issue Dec 30, 2018 · 33 comments
Labels
area/dashboard Issues or PRs about Sentinel Dashboard kind/enhancement Category issues or prs related to enhancement.

Comments

@techzealot
Copy link

techzealot commented Dec 30, 2018

Issue Description

例如:假设存在资源“/hello/test”,在配置中心或者设置规则时resourceName带了空格"[n ]/hello/test[n]"(方括号标识空格,n表示若干),则在dashboard上无法明显看出,但是当修改该资源的其它属性时,由于


这行代码会trim掉空格,就会导致资源名称悄悄发生了改变,保存之后刚好可以匹配,流控生效,但是若修改配置中心中该规则的配置,这条不带空格的规则也会跟着变化,导致看起来配置中心的规则已生效,实际情况是修改并未生效,此时资源名称又变回了带空格的,因此无法匹配,而且相当难以发现,实际上整个流程中产生了两条规则,但是它们却并没有以两条的形式展示出来,而且用户点击了编辑按钮,资源名也无法编辑,也不应生成新的规则,令人相当困惑
Type: bug report

Describe what happened (or what feature you want)

dashboard修改规则时去除了资源名称前后的空格导致规则发生了变化但是却又未在界面上有所区分

Describe what you expected to happen

资源名称要么都不处理空格,要么整个系统中都要统一处理空格

How to reproduce it (as minimally and precisely as possible)

  1. 将wiki中如何使用的例子中定义资源的语句修改为带空格的即可,rule.setResource(" HelloWorld");
  2. 观察dashboard发现规则未生效
  3. 点击流控规则,编辑HelloWorld规则,不作任何修改,直接保存,发现规则已生效

Tell us your environment

win 10,idea,jdk 8,sentinel 1.4

Anything else we need to know?

no

@cdfive
Copy link
Collaborator

cdfive commented Dec 30, 2018

按重新步骤试了,点保存只有1条规则;
我是用static块加了一个规则,设置了rule.setResource(" HelloWorld");,然后启动控制台测试的。

程序里resource.trim(),估计是考虑到像应用名称、资源名称最好别加空格,程序处理时就去掉;
一般规范中都不推荐用空格吧。

确实你说的对,要么不处理要么统一处理;
可是编辑保存是按id保存的,应该不会生产新的规则啊

可否将重现步骤描述得详细些,比如配置中心用的什么,有什么代码改动没有

@techzealot
Copy link
Author

techzealot commented Dec 30, 2018

并未修改代码,我的意思是整个流程涉及到两个不同的资源名称,点击保存前和保存后按照正常逻辑资源名称不能改变,如果改变了就应该是两条才对,但是如果是两条又与编辑这个语义冲突

@cdfive
Copy link
Collaborator

cdfive commented Dec 30, 2018

这个要看有空格的资源名称是怎么加的了;
因为程序里做了trim()处理了,编辑保存,有空格的按程序执行就会去掉空格;
在控制台界面上新增规则,输入了空格的资源名,保存后是没有空格的;
这说明控制台新增、编辑的逻辑是一致的,在控制台新增不会产生带空格的资源名。

如果在其它地方把有空格的资源名添加好了,那控制台编辑后名称改变了,这属于正常的吧,你觉得呢

@techzealot
Copy link
Author

如果是按照id更新的,那应该是点保存后资源名称也被改变了,如果是按资源名称查询,查询到的就是空值,因为不存在trim之后的资源名称;关于推不推荐用空格这个肯定是不推荐的,这个问题是我不小心复制了个空格但是又没发现,最终调试了几个小时才找到原因,这种又不明显又很花时间

@techzealot
Copy link
Author

techzealot commented Dec 30, 2018

我是这样觉得的,假设定义资源时采用的外部数据源或者不是通过dashboard定义的,那么规则在修改一次之后就再也不会生效了,因为与规则关联的资源名称已经发生了变化,我这种情况是在试验Apollo配置中心时出现的

@cdfive
Copy link
Collaborator

cdfive commented Dec 30, 2018

如果是按照id更新的,那应该是点保存后资源名称也被改变了,如果是按资源名称查询,查询到的就是空值,因为不存在trim之后的资源名称;关于推不推荐用空格这个肯定是不推荐的,这个问题是我不小心复制了个空格但是又没发现,最终调试了几个小时才找到原因,这种又不明显又很花时间

恩,理解你说的,因为空格不容易发现,所以影响调试定位问题。
但即使控制台这里逻辑不trim,空格在界面上看也是不容易被发现的

@cdfive
Copy link
Collaborator

cdfive commented Dec 30, 2018

我是这样觉得的,假设定义资源时采用的外部数据源或者不是通过dashboard定义的,那么规则在修改一次之后就再也不会生效了,因为与规则关联的资源名称已经发生了变化,我这种情况是在试验Apollo配置中心时出现的

明白你意思了。那这样的话,就是说修改的时候,不要trim资源名,这样应该就符合需求对吗?
本来界面上资源名也不能修改的,那讲道理后端程序也不应该做任何改动,包括trim

@techzealot
Copy link
Author

techzealot commented Dec 30, 2018

我是这样觉得的,假设定义资源时采用的外部数据源或者不是通过dashboard定义的,那么规则在修改一次之后就再也不会生效了,因为与规则关联的资源名称已经发生了变化,我这种情况是在试验Apollo配置中心时出现的

明白你意思了。那这样的话,就是说修改的时候,不要trim资源名,这样应该就符合需求对吗?
本来界面上资源名也不能修改的,那讲道理后端程序也不应该做任何改动,包括trim

对,一旦修改了,规则就再也不能生效了,但是用户看来界面上根本没发生任何变化,只是改了流控配置而已,但是流控又不生效

@techzealot
Copy link
Author

techzealot commented Dec 30, 2018

我是这样觉得的,假设定义资源时采用的外部数据源或者不是通过dashboard定义的,那么规则在修改一次之后就再也不会生效了,因为与规则关联的资源名称已经发生了变化,我这种情况是在试验Apollo配置中心时出现的

明白你意思了。那这样的话,就是说修改的时候,不要trim资源名,这样应该就符合需求对吗?
本来界面上资源名也不能修改的,那讲道理后端程序也不应该做任何改动,包括trim

对,一旦修改了,规则就再也不能生效了,但是用户看来界面上根本没发生任何变化,只是改了流控配置而已,但是流控又不生效

还有配置中心推送规则时,资源名称是带空格的,原样传输的,但是修改却能反馈在没有空格的规则上,我猜测应该是FindRuleByResourceName时也做了trim,这样才能匹配到,这种现象看起来是推送成功了,但是其实不能生效,你可以测试下:先定义一个带空格的资源名称,把对应规则放到Apollo里去,在dashboard里修改一次,这时资源名称已经没了空格,再修改Apollo里的流控配置,本来dashboard中的值不应该变化的,因为名称不能equal,实际情况却是配置中心的更改生效了

@techzealot
Copy link
Author

其实这个问题属于一个存在多处的问题,比如




最终表现出来的现象都和resourceName是一样的,希望可以有个统一的办法解决这些问题

@cdfive
Copy link
Collaborator

cdfive commented Dec 30, 2018

再修改Apollo里的流控配置,本来dashboard中的值不应该变化的,因为名称不能equal
这里生效是对的吧。资源名字是可以相同的,一个资源可以定义多个规则。

恩,app名称也是trim处理的。这个空格修改确实太细节了,等官方同学再确认下哈。理论上确实不应该修改资源名称,保持前后端处理一致比较好。

以后如果规则不生效,建议检查资源名称埋点这两个要素,这样方便定位原因。

@techzealot
Copy link
Author

techzealot commented Dec 30, 2018

再修改Apollo里的流控配置,本来dashboard中的值不应该变化的,因为名称不能equal
这里生效是对的吧。资源名字是可以相同的,一个资源可以定义多个规则。

如果规则不生效,建议检查_资源名称_和埋点这两个要素,这样方便定位原因。

主要想表达两个意思:
1.因为在dashboard中修改过一次后,资源名称变了,在Apollo中修改流控配置,不改资源名称时,按理说两个资源没有任何关系,dashboard中不会变化,实际上规则发生了变化
2.推送完成后规则发生了变化,会让人以为更改生效了,但是这里由于资源名称是去掉空格的,所以推送的配置不生效

@cdfive
Copy link
Collaborator

cdfive commented Dec 30, 2018

两者有关系吧,资源id是相同的,指一个规则。
在Apollo中修改流控配置,修改后规则的资源名称有空格吗?

Apollo是push方式数据源,控制台修改的话,
推荐的数据流向是:配置中心控制台/Sentinel 控制台 → 配置中心 → Sentinel 数据源 → Sentinel

参考:
https://github.com/alibaba/Sentinel/wiki/在生产环境中使用-Sentinel-控制台#push-模式的数据源

@techzealot
Copy link
Author

techzealot commented Dec 30, 2018

两者有关系吧,资源id是相同的,指一个规则。
在Apollo中修改流控配置,修改后规则的资源名称有空格吗?

Apollo是push方式数据源,控制台修改的话,
推荐的数据流向是:配置中心控制台/Sentinel 控制台 → 配置中心 → Sentinel 数据源 → Sentinel

参考:
https://github.com/alibaba/Sentinel/wiki/在生产环境中使用-Sentinel-控制台#push-模式的数据源

在配置中心不修改资源名称,修改其他配置时,两个资源是没有关系的,实际效果来看,sentinel合并修改的时候也有trim操作

@cdfive
Copy link
Collaborator

cdfive commented Dec 30, 2018

有点明白为什么没关系呢? 我理解是资源是按id来确认唯一性的,而不是资源名称。
配置中心改了,推送到客户端,就应该以配置中心的为准了。

而且我发现,控制台在前端就已经把输入trim了,可以用F12打开调试看看请求参数。
image
可能是跟angular1.4.8默认就trim了的,这个我不太确定。

@techzealot
Copy link
Author

techzealot commented Dec 30, 2018

有点明白为什么没关系呢? 我理解是资源是按id来确认唯一性的,而不是资源名称。
配置中心改了,推送到客户端,就应该以配置中心的为准了。

而且我发现,控制台在前端就已经把输入trim了,可以用F12打开调试看看请求参数。
可能是跟angular1.4.8默认就trim了的,这个我不太确定。

因为Apollo推送的时候推送的是:

[
          {
            "resource": "TestResource",
            "controlBehavior": 0,
            "count": 5.0,
            "grade": 1,
            "limitApp": "default",
            "strategy": 0
          }
        ]

没有id这个概念,肯定是按resource匹配的,我看下源码吧,合并的时候应该有问题

@cdfive
Copy link
Collaborator

cdfive commented Dec 30, 2018

有点明白为什么没关系呢? 我理解是资源是按id来确认唯一性的,而不是资源名称。
配置中心改了,推送到客户端,就应该以配置中心的为准了。
而且我发现,控制台在前端就已经把输入trim了,可以用F12打开调试看看请求参数。
可能是跟angular1.4.8默认就trim了的,这个我不太确定。

因为Apollo推送的时候推送的是:

[
          {
            "resource": "TestResource",
            "controlBehavior": 0,
            "count": 5.0,
            "grade": 1,
            "limitApp": "default",
            "strategy": 0
          }
        ]

没有id这个概念,肯定是按resource匹配的,我看下源码吧,合并的时候应该有问题

看了下确实是在按resource合并的,在FlowRuleUtil#buildFlowRuleMap方法,
因为Apollo推送的时候推送的是:
是指Apollo推送到控制台吗? 看这个json没有空格啊

在控制台编辑保存,这个时候Apollo上的resource有空格吗,在Apollo修改其它属性保存之前

@techzealot
Copy link
Author

techzealot commented Dec 30, 2018

有点明白为什么没关系呢? 我理解是资源是按id来确认唯一性的,而不是资源名称。
配置中心改了,推送到客户端,就应该以配置中心的为准了。
而且我发现,控制台在前端就已经把输入trim了,可以用F12打开调试看看请求参数。
可能是跟angular1.4.8默认就trim了的,这个我不太确定。

因为Apollo推送的时候推送的是:

[
          {
            "resource": "TestResource",
            "controlBehavior": 0,
            "count": 5.0,
            "grade": 1,
            "limitApp": "default",
            "strategy": 0
          }
        ]

没有id这个概念,肯定是按resource匹配的,我看下源码吧,合并的时候应该有问题

看了下确实是在按resource合并的,在FlowRuleUtil#buildFlowRuleMap方法,
因为Apollo推送的时候推送的是:
是指Apollo推送到控制台吗? 看这个json没有空格啊

在控制台编辑保存,这个时候Apollo上的resource有空格吗,在Apollo修改其它属性保存之前

apollo修改之前假设在控制台修改过一次,之后Apollo在推送,Apollo是先推到连接Apollo的客户端,dashborad看到的已经是客户端内存中的值了,效果是会把没空格的更新了,而且流控不生效

@cdfive
Copy link
Collaborator

cdfive commented Dec 30, 2018

控制台改过,然后程序会trim,如果没有改动控制台逻辑,这时控制台推送给客户端resource空格没了,规则不生效;

之后Apollo在推送,这时Apollo推送前,Apollo配置中心上的resource有空格吗?

@techzealot
Copy link
Author

控制台改过,然后程序会trim,如果没有改动控制台逻辑,这时控制台推送给客户端resource空格没了,规则不生效;

之后Apollo在推送,这时Apollo推送前,Apollo配置中心上的resource有空格吗?

这个描述起来有点麻烦,还可能说不清楚,等我明天写个例子,一跑bug就现形了

@techzealot
Copy link
Author

techzealot commented Dec 30, 2018

谢谢回复,早点休息吧

@cdfive
Copy link
Collaborator

cdfive commented Dec 30, 2018

谢谢回复,早点休息吧

客气了,正好在看控制台保存规则。

我觉得你描述得挺清楚的,可能是我哪没理解到,Apollo还没用过。感觉就差最后点就水落石出了。。
最小化一个demo也行,方便大家参考。

你也早点休息,晚安: )

@techzealot
Copy link
Author

ok,找了一下午,还是在调试sentinel源码的时候才发现多写了个空格,但是控制台给人的感觉就像是一切正常,但是新流控规则不生效,搞得我都要怀疑Apollo没有刷新了

@techzealot
Copy link
Author

关于配置中心规则推送后,规则定义的处理是我理解错了


代码里是直接用新的数组替换旧的数组,我还以为是合并了两个资源
这样也就是说在现在的策略下,如果在dashboard中添加了一个规则,当配置中心修改推送后会被配置中心的完全覆盖,也就是说不能再两个地方同时修改,外部动态数据源会覆盖dashboard的修改

@techzealot
Copy link
Author

techzealot commented Dec 31, 2018

那我总结下,目前的问题就是规则绑定资源时资源名称的空格问题了,有两种情况:

  1. 定义资源时的名称有空格,规则绑定的资源也有空格,这种情况下,在系统初次启动后,流控规则能够匹配生效,一旦在dashboard中做了任何修改,则由于资源名称不匹配,流控失效;
  2. 定义资源没有空格,规则绑定资源时有空格,这样一来,系统启动时流控是不生效的,一旦在dashboard中修改后,由于资源名称匹配,则流控生效,如果动态数据源是配置中心的话,就会表现为推送后失效,dashboard修改后生效
    如果绑定资源时没有空格,这种trim没做改动,不会改变用户的意图

@techzealot
Copy link
Author

参考示例:https://github.com/techzealot/sentinel-demo

@cdfive
Copy link
Collaborator

cdfive commented Dec 31, 2018

总结得挺清晰的。你提到两个地方改,所以像Apollo这种push模式数据源,wiki上官方推荐的方式:
image

1.如果改造控制台,就可以参考这个数据流向,在控制台修改点保存后,不直接推送给客户端,而是保存到配置中心,配置中心再把变动推给客户端。这种方式就支持两边都同时修改。这里要注意的是,前面提到的,界面改了发http请求到后端,空格已被angular框架trim掉了;

2.如果暂时不想改造控制台,建议就只在配置中心修改,控制台仅查看或者少量临时性修改;

3.如果什么都不作改动,把资源名称的空格去掉是最简单的解决方式。

参考:

https://github.com/alibaba/Sentinel/wiki/在生产环境中使用-Sentinel-控制台#push-模式的数据源

@techzealot
Copy link
Author

techzealot commented Dec 31, 2018

对,目前在不做任何扩展的情况下,最好就是在使用资源名称时不要加空格,否则会出现莫名其妙的问题,因此建议dashboard中编辑时不要做trim(新建可以trim),用户定义的是什么就是什么,这样其实很容易就会发现多了个空格,因为对于第一种情况不管如何修改都是可以匹配的,而对于第二种情况不管在哪修改,流控始终不生效,使用者很快就会意识到是资源名称不匹配,而不是改了某个地方就生效,这种会把简单的错误隐藏的很深,不深入源码调试难以发现,我觉得不应该trim不能编辑的东西,应该把用户的错误或者意图完整保留,由用户自己去发现控制

@cdfive
Copy link
Collaborator

cdfive commented Jan 1, 2019

就是编辑保存时后端代码不trim resource,因为前端界面资源名称是不能编辑的;
当然trim对于规范的没有空格的资源名称是没问题的,同时trim也规范了名称;
而界面提交http请求时,angular框架就trim了,在进后端controller之前;
有空格的资源名称毕竟太少了,也可以从开发者角度规范。

还是请@CarpenterLee @sczyh30确认下吧

@techzealot
Copy link
Author

对,正常的名称是没问题的,就怕手抖,多打了个空格,可能会出现莫名其妙的情况

@sczyh30 sczyh30 added kind/enhancement Category issues or prs related to enhancement. area/dashboard Issues or PRs about Sentinel Dashboard labels Jan 2, 2019
@sczyh30
Copy link
Member

sczyh30 commented Jan 2, 2019

感谢两位的讨论。这个地方新建的时候应该是要 trim 的(之前是有的,1.4.0 改造时搞丢了),防止空格的问题。我们会在 1.4.1 版本中修复这个问题。

@cdfive
Copy link
Collaborator

cdfive commented Jan 2, 2019

@sczyh30 新增规则,资源名有空格,点保存后发现也没空格。查看http请求,前端已经trim了;在控制台新增是没有空格的;

@techzealot 他是在配置中心上新增的,新增时填资源名有空格,然后希望在控制台编辑规则的时候,不trim掉,保存资源名称不变。逻辑上说还是有道理的,因为前端界面资源名都不能改,那么后端按理说在update时不应当trim

@sczyh30
Copy link
Member

sczyh30 commented Jan 30, 2019

Already resolved in 1.4.1. Thanks for discussion.

@sczyh30 sczyh30 closed this as completed Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dashboard Issues or PRs about Sentinel Dashboard kind/enhancement Category issues or prs related to enhancement.
Projects
None yet
Development

No branches or pull requests

3 participants