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

Annotation supports method name as default resource name #187

Merged
merged 6 commits into from
Oct 24, 2018

Conversation

waveng
Copy link
Contributor

@waveng waveng commented Oct 16, 2018

Describe what this PR does / why we need it

通过注解方式时,希望可以不指定资源名,而当前时必须指定的。
指定资源名我认为是一个很麻烦的事情,必须要防止资源名相同,但是当一个项目中有多个相同名称的时,而且是不通的人开发,就有可能出现两者指定的资源名是相同的,所以建议可以不用指定资源名称,默认取当前资源完全限定名,就如 dubbo filter 中一样,这样也许更方便

Does this pull request fix one issue?

Describe how you did it

在不指定的时,默认取当前资源完全限定名

Describe how to verify it

在资源上@SentinelResource , 可在dashboard 中查看资源名称

Special notes for reviews

Add rule configuration support for parameter flow control in Sentinel…
Fix the wrong coordinate scaling of QPS chart (passed/blocked) in das…
@CLAassistant
Copy link

CLAassistant commented Oct 16, 2018

CLA assistant check
All committers have signed the CLA.

@sczyh30
Copy link
Member

sczyh30 commented Oct 16, 2018

Hi, thanks for your contribution. Please fill the PR description as the PR template described. And please open a related issue then link it in this PR. See contribution guidelines for detail :)

Also please sign the CLA.

@sczyh30 sczyh30 added the to-review To review label Oct 16, 2018
@codecov-io
Copy link

codecov-io commented Oct 16, 2018

Codecov Report

Merging #187 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #187   +/-   ##
=========================================
  Coverage     50.29%   50.29%           
  Complexity      820      820           
=========================================
  Files           140      140           
  Lines          4772     4772           
  Branches        679      679           
=========================================
  Hits           2400     2400           
  Misses         2083     2083           
  Partials        289      289

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 0855809...17d435f. Read the comment docs.

return resourceName;
}
StringBuilder buf = new StringBuilder(64);
buf.append(method.getDeclaringClass().getName())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you can just use MethodUtil.getMethodName(method) to get its method representation, which will cache the method name for better performance.

Suggested change
buf.append(method.getDeclaringClass().getName())
return MethodUtil.getMethodName(method);

@sczyh30 sczyh30 changed the title Modify the annotaion aspectj Annotation supports method name as default resource name Oct 18, 2018
Add support for authority rule configuration in Sentinel Dashboard (#…
@sczyh30 sczyh30 merged commit 078df9d into alibaba:master Oct 24, 2018
@sczyh30
Copy link
Member

sczyh30 commented Oct 24, 2018

Thanks for contributing. I'll enhance the code later.

@sczyh30 sczyh30 removed the to-review To review label Oct 24, 2018
@sczyh30 sczyh30 added this to the 1.3.0 milestone Oct 24, 2018
@waveng
Copy link
Contributor Author

waveng commented Oct 30, 2018

Thank you

Arlmls pushed a commit to Arlmls/Sentinel that referenced this pull request Jan 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants