[Rate-limit provider]: Add token bucket implementation of rate-limiter #386

MalloZup · 2021-01-23T09:25:39Z

Use tockenbucket provider

codecov-io · 2021-01-23T09:27:36Z

Codecov Report

Merging #386 (0bb8795) into v2 (a79558a) will decrease coverage by 27.83%.
The diff coverage is 37.48%.

@@             Coverage Diff             @@
##               v2     #386       +/-   ##
===========================================
- Coverage   83.58%   55.74%   -27.84%     
===========================================
  Files          30       37        +7     
  Lines         932     1322      +390     
===========================================
- Hits          779      737       -42     
- Misses        114      514      +400     
- Partials       39       71       +32

Impacted Files	Coverage Δ
chain.go	`0.00% <ø> (-90.91%)`	⬇️
interceptors/auth/auth.go	`100.00% <ø> (ø)`
interceptors/auth/metadata.go	`100.00% <ø> (ø)`
interceptors/ratelimit/ratelimit.go	`50.00% <0.00%> (-50.00%)`	⬇️
interceptors/tags/fieldextractor.go	`13.79% <0.00%> (-71.51%)`	⬇️
testing/testpb/interceptor_suite.go	`0.00% <0.00%> (ø)`
testing/testpb/test.manual_extractfields.pb.go	`0.00% <0.00%> (ø)`
testing/testpb/test.manual_validator.pb.go	`0.00% <0.00%> (ø)`
testing/testpb/test.pb.go	`36.92% <36.92%> (ø)`
interceptors/logging/payload.go	`67.18% <42.30%> (-15.43%)`	⬇️
... and 45 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 3ff779b...0bb8795. Read the comment docs.

MalloZup · 2021-01-23T09:34:49Z

@bwplotka this is the follow-up for the previous PR have look when you have time.

Thx for suggestion and review, no hurry I can wait the weekend I just had some free minutes so I commit-ed it 😁

bwplotka

LGTM, just few suggestions

providers/tokenbucket/doc.go

providers/tokenbucket/examples_test.go

providers/tokenbucket/go.mod

providers/tokenbucket/limiter.go

MalloZup · 2021-01-23T20:59:28Z

thx @bwplotka for review 🌞 . I will check out more in detail tomorrow. enjoy 🍋

google-cla · 2021-01-25T11:31:30Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

providers/tokenbucket/examples_test.go

google-cla · 2021-01-27T17:05:27Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

MalloZup · 2021-01-27T17:06:29Z

@googlebot I consent

google-cla · 2021-01-27T17:06:33Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

yashrsharma44

Sounds great, some nitpickings!

providers/tokenbucket/limiter.go

providers/tokenbucket/examples_test.go

providers/tokenbucket/doc.go

providers/tokenbucket/examples_test.go

providers/tokenbucket/limiter.go

bwplotka

LGTM after @yashrsharma44 valid comments are addressed.

Thanks @yashrsharma44 for solid review 👍🏽

yashrsharma44 · 2021-01-28T08:40:27Z

@googlebot I consent

MalloZup · 2021-01-28T08:41:13Z

thx @yashrsharma44 and @bwplotka 💟

providers/tokenbucket/examples_test.go

Use tockenbucket provider Signed-off-by: dmaiocchi <[email protected]>

Co-authored-by: Yash Sharma <[email protected]>

MalloZup · 2021-01-28T18:42:42Z

@bwplotka @yashrsharma44 should be ok now just check it once you have time 🍡

bwplotka

Last nit hopefully! (:

providers/tokenbucket/limiter.go

providers/tokenbucket/examples_test.go

Signed-off-by: dmaiocchi <[email protected]>

MalloZup · 2021-01-29T11:18:11Z

should be ok. 🤖

MalloZup · 2021-01-29T11:18:40Z

@bwplotka let me know if you wants that I squash/rebase commits.

bwplotka

PERFECTION ❤️

LGTM!

Thanks a lot (:

MalloZup · 2021-01-29T13:45:52Z

thx @bwplotka and @yashrsharma44 for review. YES WE DID IT ! 😁 thx and see you on the next PR :)

yashrsharma44 · 2021-01-29T14:10:18Z

MISSION ACCOMPLISHED!!! 🎉

MalloZup changed the title ~~WIP: [Rate-limit provider]: Add tocken bucket implementation of rate-limiter~~ [Rate-limit provider]: Add tocken bucket implementation of rate-limiter Jan 23, 2021

bwplotka reviewed Jan 23, 2021

View reviewed changes

providers/tokenbucket/doc.go Outdated Show resolved Hide resolved

providers/tokenbucket/examples_test.go Outdated Show resolved Hide resolved

providers/tokenbucket/go.mod Outdated Show resolved Hide resolved

providers/tokenbucket/limiter.go Outdated Show resolved Hide resolved

MalloZup force-pushed the rate-limit-provider branch from 7abb989 to bb1cfa9 Compare January 25, 2021 11:31

bwplotka reviewed Jan 25, 2021

View reviewed changes

providers/tokenbucket/examples_test.go Outdated Show resolved Hide resolved

MalloZup force-pushed the rate-limit-provider branch from bb1cfa9 to df0eb12 Compare January 27, 2021 17:05

MalloZup force-pushed the rate-limit-provider branch from df0eb12 to 36d7d4f Compare January 27, 2021 17:09

MalloZup requested review from bwplotka and yashrsharma44 January 27, 2021 17:11

yashrsharma44 reviewed Jan 27, 2021

View reviewed changes

bwplotka approved these changes Jan 27, 2021

View reviewed changes

bwplotka reviewed Jan 28, 2021

View reviewed changes

providers/tokenbucket/examples_test.go Outdated Show resolved Hide resolved

MalloZup and others added 8 commits January 28, 2021 12:53

Add tocken bucket implementation of rate-limiter

224625a

Use tockenbucket provider Signed-off-by: dmaiocchi <[email protected]>

Update providers/tokenbucket/limiter.go

96640c0

Co-authored-by: Yash Sharma <[email protected]>

Update providers/tokenbucket/examples_test.go

f8fc1cb

Co-authored-by: Yash Sharma <[email protected]>

Update providers/tokenbucket/limiter.go

f34fce1

Co-authored-by: Yash Sharma <[email protected]>

Update providers/tokenbucket/limiter.go

e58896c

Co-authored-by: Yash Sharma <[email protected]>

Update providers/tokenbucket/doc.go

31d82c9

Co-authored-by: Yash Sharma <[email protected]>

Update providers/tokenbucket/limiter.go

5f5036d

Co-authored-by: Yash Sharma <[email protected]>

Update providers/tokenbucket/examples_test.go

2e2db6f

Co-authored-by: Yash Sharma <[email protected]>

MalloZup force-pushed the rate-limit-provider branch from 29a94a5 to 6046c8b Compare January 28, 2021 11:55

MalloZup requested a review from bwplotka January 28, 2021 12:03

MalloZup requested a review from yashrsharma44 January 28, 2021 12:03

bwplotka changed the title ~~[Rate-limit provider]: Add tocken bucket implementation of rate-limiter~~ [Rate-limit provider]: Add token bucket implementation of rate-limiter Jan 28, 2021

bwplotka reviewed Jan 28, 2021

View reviewed changes

providers/tokenbucket/limiter.go Outdated Show resolved Hide resolved

providers/tokenbucket/examples_test.go Outdated Show resolved Hide resolved

Fix word and minor changes

733f36f

Signed-off-by: dmaiocchi <[email protected]>

MalloZup force-pushed the rate-limit-provider branch from 0bb8795 to 733f36f Compare January 29, 2021 10:15

bwplotka approved these changes Jan 29, 2021

View reviewed changes

bwplotka merged commit 20b2825 into grpc-ecosystem:v2 Jan 29, 2021

yashrsharma44 mentioned this pull request Jan 30, 2021

Enhance dummy rate limit interceptor we have. #351

Closed

yashrsharma44 mentioned this pull request Jul 14, 2021

Refactor tracing implements #312

Closed

aimuz mentioned this pull request Mar 8, 2023

docs: add logging.InjectFields usage description #540

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Rate-limit provider]: Add token bucket implementation of rate-limiter #386

[Rate-limit provider]: Add token bucket implementation of rate-limiter #386

MalloZup commented Jan 23, 2021 •

edited

Loading

codecov-io commented Jan 23, 2021 •

edited

Loading

MalloZup commented Jan 23, 2021

bwplotka left a comment

MalloZup commented Jan 23, 2021

google-cla bot commented Jan 25, 2021

google-cla bot commented Jan 27, 2021

MalloZup commented Jan 27, 2021

google-cla bot commented Jan 27, 2021

yashrsharma44 left a comment

bwplotka left a comment

yashrsharma44 commented Jan 28, 2021

MalloZup commented Jan 28, 2021

MalloZup commented Jan 28, 2021

bwplotka left a comment

MalloZup commented Jan 29, 2021

MalloZup commented Jan 29, 2021

bwplotka left a comment

MalloZup commented Jan 29, 2021

yashrsharma44 commented Jan 29, 2021

[Rate-limit provider]: Add token bucket implementation of rate-limiter #386

[Rate-limit provider]: Add token bucket implementation of rate-limiter #386

Conversation

MalloZup commented Jan 23, 2021 • edited Loading

codecov-io commented Jan 23, 2021 • edited Loading

Codecov Report

MalloZup commented Jan 23, 2021

bwplotka left a comment

Choose a reason for hiding this comment

MalloZup commented Jan 23, 2021

google-cla bot commented Jan 25, 2021

google-cla bot commented Jan 27, 2021

MalloZup commented Jan 27, 2021

google-cla bot commented Jan 27, 2021

yashrsharma44 left a comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

yashrsharma44 commented Jan 28, 2021

MalloZup commented Jan 28, 2021

MalloZup commented Jan 28, 2021

bwplotka left a comment

Choose a reason for hiding this comment

MalloZup commented Jan 29, 2021

MalloZup commented Jan 29, 2021

bwplotka left a comment

Choose a reason for hiding this comment

MalloZup commented Jan 29, 2021

yashrsharma44 commented Jan 29, 2021

MalloZup commented Jan 23, 2021 •

edited

Loading

codecov-io commented Jan 23, 2021 •

edited

Loading