Skip to content

293 shadowmode - #294

Merged
ysawa0 merged 10 commits into
envoyproxy:mainfrom
jespersoderlund:293_shadowmode
Oct 5, 2021
Merged

ysawa0 merged 10 commits into
envoyproxy:mainfrom
jespersoderlund:293_shadowmode

Conversation

@jespersoderlund

Copy link
Copy Markdown
Contributor

Addressing issue: #293

Adding shadow-mode:

  • Rule based shadow mode so that specific keys can be put in shadow mode
  • Global shadow-mode where all rate limit requests will be OK

Statistics on key-based and global shadow mode

Jesper Söderlund added 2 commits September 21, 2021 19:36
Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>
Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>
@ysawa0

ysawa0 commented Sep 26, 2021

Copy link
Copy Markdown
Member

Looks like CI is failing, can you take a look
/wait

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>
Comment thread README.md
Jesper Söderlund added 2 commits September 29, 2021 18:58
…e with the current rate limit, extend envoy config to display rate limit headers. Extend statsd to show shadow mode

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>
Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

@ysawa0 ysawa0 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

took a quick level look, the changes make sense to me and I think shadow mode is an option that is definitely needed. left some comments, will take a deeper look later.

Comment thread src/stats/manager_impl.go Outdated
Comment thread test/service/ratelimit_test.go Outdated
Comment thread docker-compose-example.yml Outdated
Comment thread README.md
@ysawa0

ysawa0 commented Sep 30, 2021

Copy link
Copy Markdown
Member

Took a 2nd look and everything else lgtm, so just pending some changes and merge conflicts.

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>
Jesper Söderlund added 4 commits October 4, 2021 09:14
Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>
Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>
Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>
Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>
@ysawa0

ysawa0 commented Oct 5, 2021

Copy link
Copy Markdown
Member

Looks good, thanks!

@ysawa0
ysawa0 merged commit 1c1d46e into envoyproxy:main Oct 5, 2021
debbyku pushed a commit to debbyku/ratelimit that referenced this pull request Oct 13, 2021
* Global ShadowMod

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Adding shadow-mode issue envoyproxy#293

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Fix doc format

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Extended example to showcase shadow mode as well, build a docker image with the current rate limit, extend envoy config to display rate limit headers. Extend statsd to show shadow mode

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Fine tuning of docs and example

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Added integration tests, fixed some review comments

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Doc format fix

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* gitignore for vscode

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

Co-authored-by: Jesper Söderlund <jesper.soderlund@klarna.com>
Signed-off-by: Debby Ku <debbyku.2008@gmail.com>
debbyku pushed a commit to debbyku/ratelimit that referenced this pull request Oct 13, 2021
* Global ShadowMod

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Adding shadow-mode issue envoyproxy#293

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Fix doc format

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Extended example to showcase shadow mode as well, build a docker image with the current rate limit, extend envoy config to display rate limit headers. Extend statsd to show shadow mode

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Fine tuning of docs and example

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Added integration tests, fixed some review comments

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Doc format fix

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* gitignore for vscode

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

Co-authored-by: Jesper Söderlund <jesper.soderlund@klarna.com>
Signed-off-by: Debby Ku <debbyku.2008@gmail.com>
@jespersoderlund
jespersoderlund deleted the 293_shadowmode branch October 27, 2022 07:57
timcovar pushed a commit to goatapp/ratelimit that referenced this pull request Jan 16, 2024
* Global ShadowMod

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Adding shadow-mode issue envoyproxy#293

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Fix doc format

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Extended example to showcase shadow mode as well, build a docker image with the current rate limit, extend envoy config to display rate limit headers. Extend statsd to show shadow mode

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Fine tuning of docs and example

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Added integration tests, fixed some review comments

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* Doc format fix

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

* gitignore for vscode

Signed-off-by: Jesper Söderlund <jesper.soderlund@klarna.com>

Co-authored-by: Jesper Söderlund <jesper.soderlund@klarna.com>
@PatilHrushikesh

Copy link
Copy Markdown

// When limit override is provided by envoy config, we don't want to enable shadow_mode

why was this added? can you check : #1274

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.

3 participants