增加对properties文件的支持: ali-sentinel.properties #72

VanXD · 2018-08-17T06:24:59Z

Describe what this PR does / why we need it

更方便的对sentinel进行配置.
Convenient to write configuration.

Does this pull request fix one issue?

Fixes #64

Describe how you did it

兼容了原来的模式, 从JVM上加载相应的参数.
如果JVM上没有配置project.name,则使用默认的文件名"ali-sentinel.properties"进行加载.

Compatible with old version.
If project.name is not configured on the JVM, it is loaded with the default filename "ali-sentinel.properties".

Describe how to verify it

在resources文件夹下创建"ali-sentinel.properties"文件.并且在其中书写配置.
create file in resources dir, and write your configuration.

Special notes for reviews

no

CLAassistant · 2018-08-17T06:25:07Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ TragedyXD
❌ VanXD

VanXD 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 · 2018-08-17T06:29:34Z

Codecov Report

Merging #72 into master will increase coverage by 11.07%.
The diff coverage is 49.05%.

@@              Coverage Diff              @@
##             master      #72       +/-   ##
=============================================
+ Coverage     41.71%   52.79%   +11.07%     
+ Complexity     1376      911      -465     
=============================================
  Files           304      150      -154     
  Lines          8764     5097     -3667     
  Branches       1182      733      -449     
=============================================
- Hits           3656     2691      -965     
+ Misses         4662     2083     -2579     
+ Partials        446      323      -123

Impacted Files	Coverage Δ	Complexity Δ
...ava/com/alibaba/csp/sentinel/util/AppNameUtil.java	`88.88% <100%> (+45.41%)`	`4 <2> (-1)`	⬇️
...ava/com/alibaba/csp/sentinel/util/ClassHelper.java	`44.44% <44.44%> (ø)`	`4 <4> (?)`
...om/alibaba/csp/sentinel/config/SentinelConfig.java	`59.13% <51.02%> (+7.52%)`	`15 <3> (+1)`	⬆️
...ain/java/com/alibaba/csp/sentinel/util/IdUtil.java	`0% <0%> (-100%)`	`0% <0%> (-6%)`
.../sentinel/adapter/dubbo/DubboAppContextFilter.java	`0% <0%> (-100%)`	`0% <0%> (-3%)`
...sp/sentinel/adapter/dubbo/AbstractDubboFilter.java	`0% <0%> (-100%)`	`0% <0%> (-4%)`
...alibaba/csp/sentinel/adapter/dubbo/DubboUtils.java	`0% <0%> (-66.67%)`	`0% <0%> (-2%)`
...nel/adapter/dubbo/SentinelDubboProviderFilter.java	`0% <0%> (-64%)`	`0% <0%> (-2%)`
...nel/adapter/dubbo/SentinelDubboConsumerFilter.java	`0% <0%> (-57.15%)`	`0% <0%> (-2%)`
...ntinel/slots/block/flow/param/ParameterMetric.java	`23.33% <0%> (-51.67%)`	`7% <0%> (-22%)`
... and 270 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 062385f...d4c9840. Read the comment docs.

VanXD · 2018-08-22T15:04:25Z

这个license/cla Pending就没法解决了么。。我已经去details里签署过了。

sczyh30 · 2018-08-22T15:28:38Z

请查看对应 GitHub 账号是否正确，包括产生的提交记录的 email 需要与当前 GitHub 账号一致。貌似你的提交记录里 email 和当前账号不对应。

===

Please make sure that the email of your commits matches the email of your GitHub account.

TragedyXD · 2018-08-23T09:15:04Z

非常感谢, 我换了帐号过来.

kimmking · 2018-08-23T17:36:53Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/config/SentinelConfig.java

@@ -34,6 +34,7 @@
 */
 public class SentinelConfig {

+    private static final String DEFAULT_PROPERTIES_NAME = "ali-sentinel";


sentinel is better

kimmking · 2018-08-23T17:37:47Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/config/SentinelConfig.java

+import java.net.URL;
+import java.util.*;
+import java.util.concurrent.ConcurrentHashMap;
+
 /**
 * The universal config of Courier. The config is retrieved from
 * {@code ${user.home}/logs/csp/${appName}.properties} by default.


classpath is better.

sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/ClassHelper.java

kimmking · 2018-08-24T10:14:06Z

add some tests

TragedyXD · 2018-09-17T06:13:08Z

add some tests

参考了dubbo那边对config进行的测试

===

reference config tests from dubbo.

sczyh30 · 2018-11-26T11:57:10Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/AppNameUtil.java

@@ -60,34 +60,15 @@ private AppNameUtil() {
        RecordLog.info("App name resolved: " + appName);
    }

+    /**
+     * resolve app name from jvm options
+     */
    public static void resolveAppName() {


We have to consider the case when project.name is absent both in system property and configuration file, so we have to provide a default parsed appName rather than empty.

sczyh30 · 2018-11-26T11:58:12Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/config/SentinelConfig.java

@@ -93,6 +98,7 @@ private static void loadProps() {
                RecordLog.info("[SentinelConfig] JVM parameter overrides {0}: {1} -> {2}", configKey, configValueOld, configValue);
            }
        }
+        AppNameUtil.setAppName(SentinelConfig.getConfig(AppNameUtil.APP_NAME));


We have to consider what if the appName from SentinelConfig is invalid (null or empty)?

您的两个问题都是关于appName不合法, 在dubbo里dubbo.application.name是必填项, 不配置启动会异常.
如果我们这里需要一个默认值的话, 那或许我可以在resolveAppName()中改为解析不到project.name就设置成"sentinel".
综合你们的考虑看需要怎么改呢, 因为这里主要目的是为了加载配置文件.

sentinel-bot · 2019-01-18T09:55:03Z

Ping @VanXD . Conflict happens after merging a previous commit. Please rebase the branch against master and push it back again. Thanks a lot.

pouchrobot · 2019-06-27T08:55:29Z

ping @VanXD
Conflict happens after merging a previous commit.
Please rebase the branch against master and push it back again. Thanks a lot.

sczyh30 · 2019-07-08T11:15:54Z

Resolved via #804. Anyway thanks for nice work!

增加对properties文件的支持: ali-sentinel.properties

926c73c

sczyh30 added the to-review To review label Aug 17, 2018

TragedyXD added 2 commits August 17, 2018 14:52

Merge remote-tracking branch 'upstream/master'

b88e851

sentinel-dashboard: properties配置

f0e12b1

sczyh30 requested a review from CarpenterLee August 22, 2018 15:25

kimmking suggested changes Aug 23, 2018

View reviewed changes

code review change

254c366

SentinelConfig: 单元测试

087d7cb

anjia0532 mentioned this pull request Nov 3, 2018

is sentinel console support spring boot application configuration? #229

Closed

Merge branch 'master' into master

d4c9840

sczyh30 requested changes Nov 26, 2018

View reviewed changes

sentinel-bot added the merge-conflict Category prs with merge conflict. label Jan 18, 2019

sczyh30 added the area/configuration Issues or PRs related to configurations of Sentinel label Apr 11, 2019

sczyh30 closed this Jul 8, 2019

sczyh30 removed the to-review To review label Jul 8, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

增加对properties文件的支持: ali-sentinel.properties #72

增加对properties文件的支持: ali-sentinel.properties #72

VanXD commented Aug 17, 2018

CLAassistant commented Aug 17, 2018 •

edited

codecov-io commented Aug 17, 2018 •

edited

VanXD commented Aug 22, 2018

sczyh30 commented Aug 22, 2018

TragedyXD commented Aug 23, 2018

kimmking Aug 23, 2018

kimmking Aug 23, 2018

kimmking commented Aug 24, 2018

TragedyXD commented Sep 17, 2018

sczyh30 Nov 26, 2018

sczyh30 Nov 26, 2018

VanXD Nov 28, 2018

sentinel-bot commented Jan 18, 2019

pouchrobot commented Jun 27, 2019

sczyh30 commented Jul 8, 2019

增加对properties文件的支持: ali-sentinel.properties #72

增加对properties文件的支持: ali-sentinel.properties #72

Conversation

VanXD commented Aug 17, 2018

Describe what this PR does / why we need it

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

CLAassistant commented Aug 17, 2018 • edited

codecov-io commented Aug 17, 2018 • edited

Codecov Report

VanXD commented Aug 22, 2018

sczyh30 commented Aug 22, 2018

TragedyXD commented Aug 23, 2018

kimmking Aug 23, 2018

Choose a reason for hiding this comment

kimmking Aug 23, 2018

Choose a reason for hiding this comment

kimmking commented Aug 24, 2018

TragedyXD commented Sep 17, 2018

sczyh30 Nov 26, 2018

Choose a reason for hiding this comment

sczyh30 Nov 26, 2018

Choose a reason for hiding this comment

VanXD Nov 28, 2018

Choose a reason for hiding this comment

sentinel-bot commented Jan 18, 2019

pouchrobot commented Jun 27, 2019

sczyh30 commented Jul 8, 2019

CLAassistant commented Aug 17, 2018 •

edited

codecov-io commented Aug 17, 2018 •

edited