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

feat: hotspot rule add paramKey; #376

Merged
merged 5 commits into from
Jan 19, 2021

Conversation

cafra
Copy link
Contributor

@cafra cafra commented Jan 10, 2021

In the hotspot parameter rule, ParamKey is added to cooperate with the context attachments map to quickly specify the value that requires hotspot current limit from a large number of key-value pairs.
(#360)

Describe what this PR does / why we need it

It is convenient to specify hot spot parameters in hot spot current limiting rules. For example, there are a lot of data for system parameters, calibration parameters in header, and business parameters, and the parameters required by different businesses are different. It is very convenient to add all the data into ctx.attachments and specify the key in the rules.

Does this pull request fix one issue?

This time pr has added ParamKey, and modified the corresponding unit test and example.

Describe how you did it

ParamKey is added to the rule; trafficShaper also implements the extractArg interface correspondingly, which can be used with the existing paramIndex to easily specify hotspot parameters.

Describe how to verify it

Verified in unit test and example

Special notes for reviews

ParamKey can be considered as []string, so that users can specify hotspot rules flexibly in addition to specifying a single parameter.
For example, ParamKey=[]string{"uid","merchant_id"} This allows uid and merchant_id to be used as joint parameters as hot spots.

@louyuting louyuting added area/flow-hotspot Issues or PRs related to hotspot flow control kind/enhancement Category issues or PRs related to enhancement to-review PRs to review labels Jan 12, 2021
@louyuting louyuting added this to the 1.1.0 milestone Jan 12, 2021
@louyuting louyuting linked an issue Jan 12, 2021 that may be closed by this pull request
@cafra cafra force-pushed the hotspot_rule_add_paramKey branch from 721845c to ffa0195 Compare January 14, 2021 06:45
@codecov-io
Copy link

codecov-io commented Jan 14, 2021

Codecov Report

Merging #376 (d2411f6) into master (aafce3f) will decrease coverage by 0.08%.
The diff coverage is 40.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #376      +/-   ##
==========================================
- Coverage   51.42%   51.33%   -0.09%     
==========================================
  Files          70       70              
  Lines        4331     4356      +25     
==========================================
+ Hits         2227     2236       +9     
- Misses       1806     1818      +12     
- Partials      298      302       +4     
Impacted Files Coverage Δ
core/hotspot/concurrency_stat_slot.go 6.66% <0.00%> (+0.41%) ⬆️
core/hotspot/rule_manager.go 58.44% <0.00%> (ø)
core/hotspot/slot.go 9.52% <0.00%> (-18.26%) ⬇️
core/hotspot/traffic_shaping.go 57.71% <40.47%> (-4.56%) ⬇️
core/hotspot/rule.go 66.66% <100.00%> (ø)

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 aafce3f...d2411f6. Read the comment docs.

core/hotspot/rule.go Outdated Show resolved Hide resolved
chore:ParamKey add supplementary explanation
chore:adjust the order of extraction parameters, give priority to paramkey, and avoid the problem of paramIndex default value 0;update test unit
@cafra cafra force-pushed the hotspot_rule_add_paramKey branch from ffa0195 to f33b480 Compare January 14, 2021 07:54
core/hotspot/rule.go Outdated Show resolved Hide resolved
@cafra cafra requested a review from louyuting January 19, 2021 02:32
Copy link
Collaborator

@binbin0325 binbin0325 left a comment

Choose a reason for hiding this comment

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

LGTM

@louyuting louyuting merged commit 6b43fd2 into alibaba:master Jan 19, 2021
@sczyh30 sczyh30 removed the to-review PRs to review label Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/flow-hotspot Issues or PRs related to hotspot flow control kind/enhancement Category issues or PRs related to enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hotspot rule add ParamKey
6 participants