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

[Feature]Add metric extension to Sentinel internal statistics #730

Merged
merged 4 commits into from
May 6, 2019

Conversation

CarpenterLee
Copy link
Contributor

@CarpenterLee CarpenterLee commented May 5, 2019

Describe what this PR does / why we need it

Sentient has various flow control strategies, all these strategies are based on Sentinel internal statistics. The real-time statistics are also useful for system monitoring. Unfortunately, Sentinel does not provide an extension to obtain these data directly.

This PR provides an extension to Sentinel internal statistics.

As discussed in #211, standardizing Sentinel metrics and monitoring is necessary, but we can't expect sentinel to do this. Sentinel should not care about the metric implementation, that is in which format the data is stored or in which way the metric is exposed. That is why MetricExtension is relevant.

In the latter version, it may be necessary to refactor MetricWriter to obey the MetricExtension pattern.

Does this pull request fix one issue?

NONE

Describe how you did it

Add MetricExtension interface and related SPI classes as an extension to Sentinel internal statistics. This extension provides callbacks when a request passes rule checking, blocked by flow control, successfully end or exception occurred. Accompanied by these events, response time and current thread number will be recorded too.

Describe how to verify it

Run test cases.

Special notes for reviews

…ternal statistics.

Signed-off-by: Carpenter Lee <hooleeucas@163.com>
Signed-off-by: Carpenter Lee <hooleeucas@163.com>
Signed-off-by: Carpenter Lee <hooleeucas@163.com>
@sczyh30 sczyh30 added kind/feature Category issues or prs related to feature request. to-review To review labels May 5, 2019
@CarpenterLee CarpenterLee removed the to-review To review label May 5, 2019
@sczyh30 sczyh30 added the to-review To review label May 5, 2019
Signed-off-by: Carpenter Lee <hooleeucas@163.com>
@codecov-io
Copy link

codecov-io commented May 6, 2019

Codecov Report

Merging #730 into master will increase coverage by 0.66%.
The diff coverage is 76.19%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #730      +/-   ##
============================================
+ Coverage      40.4%   41.06%   +0.66%     
- Complexity     1312     1338      +26     
============================================
  Files           297      301       +4     
  Lines          8591     8630      +39     
  Branches       1156     1159       +3     
============================================
+ Hits           3471     3544      +73     
+ Misses         4695     4661      -34     
  Partials        425      425
Impacted Files Coverage Δ Complexity Δ
...src/main/java/com/alibaba/csp/sentinel/Tracer.java 0% <0%> (ø) 0 <0> (ø) ⬇️
.../sentinel/metric/extension/MetricCallbackInit.java 100% <100%> (ø) 2 <2> (?)
...metric/extension/callback/MetricEntryCallback.java 100% <100%> (ø) 5 <5> (?)
...inel/metric/extension/MetricExtensionProvider.java 76.92% <76.92%> (ø) 4 <4> (?)
.../metric/extension/callback/MetricExitCallback.java 88.88% <88.88%> (ø) 3 <3> (?)
...a/csp/sentinel/slots/statistic/base/LeapArray.java 70.58% <0%> (+2.94%) 34% <0%> (+1%) ⬆️
... and 4 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 1d1878c...6f09de8. Read the comment docs.

Copy link
Member

@sczyh30 sczyh30 left a comment

Choose a reason for hiding this comment

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

LGTM

@CarpenterLee CarpenterLee merged commit a268338 into master May 6, 2019
@CarpenterLee CarpenterLee deleted the feature/metric_extension branch May 6, 2019 02:57
@CarpenterLee CarpenterLee changed the title [Feature]Add an extension to Sentinel internal statistics [Feature]Add metric extension to Sentinel internal statistics May 6, 2019
@sczyh30 sczyh30 removed the to-review To review label May 10, 2019
@sczyh30 sczyh30 added this to the 1.6.1 milestone May 23, 2019
@sczyh30 sczyh30 added the area/metrics Issues or PRs related to metrics and monitoring label Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Issues or PRs related to metrics and monitoring kind/feature Category issues or prs related to feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants