Rate Limit Configuration: Compose actions by ccaraman · Pull Request #458 · envoyproxy/envoy

ccaraman · 2017-02-10T00:22:18Z

-Actions now append a descriptor entry to a descriptor
-Actions are now composed in the ordered specified in the rate limit configuration.
-Route Key has been deprecated and moved to action type Rate Limit Key.
-Kill Switch Key is now Disable Key and is used in the rate limit http filter to disable a rate limit configuration.
-Service_to_service action is broken into two action types: source_cluster and destination_cluster

ccaraman · 2017-02-10T00:49:34Z

@lyft/network-team

mattklein123 · 2017-02-10T16:25:48Z

docs/configuration/http_conn_man/route_config/rate_limits.rst

-  Order matters as the actions are processed sequentially and the descriptors will be composed in
-  that sequence.
+  Order matters as the actions are processed sequentially and the descriptor will be composed by
+  appending decriptor entries in that sequence.


at this point I think it would be useful to provide a small example in the docs (kind of like we do for traffic shifting).

Good idea. I will add a section about Composing actions.

mattklein123 · 2017-02-10T16:26:52Z

docs/configuration/http_conn_man/route_config/rate_limits.rst

+
+The following descriptor entry is appended to the descriptor:
+
+  * ("to_cluster", "<:ref:`route target cluster <config_http_conn_man_route_table_route_cluster>`>")


I would be more verbose here in that it will be the routed cluster, which could be do to "cluster", "weighted_clusters", "cluster_header" etc.

mattklein123 · 2017-02-10T16:28:27Z

docs/configuration/http_conn_man/route_config/rate_limits.rst


-If *route_key* is set in the rate limit configuration, the following
-descriptor is sent as well:
+Route Key


I think we should rename this at this point. You have made this fully generic (nice), so it's possible to compose multiple generic strings now. I would call it something like "generic_string" etc. (you can think of a good name).

Changed it to Rate Limit Key that will have a descriptor key of rate_limit_key

mattklein123 · 2017-02-10T16:31:17Z

source/common/http/filter/ratelimit.cc

  for (const Router::RateLimitPolicyEntry& rate_limit :
       rate_limit_policy.getApplicableRateLimit(config_->stage())) {
-    const std::string& route_key = rate_limit.routeKey();
+    const std::string& route_key = rate_limit.killSwitchKey();


rename var.

rshriram · 2017-02-12T05:36:17Z

docs/configuration/http_conn_man/route_config/rate_limits.rst

@@ -25,14 +24,10 @@ stage
 kill_switch_key
  *(optional, string)* The key to be set in runtime to disable this rate limit configuration.


This might not pertain to this PR, by anyway.
the term "kill switch" seems to imply some mechanism to shutoff a system completely before it damages something. My first impression upon reading this is, something like fencing the service instance completely.

However, what you mean here is to kill off the system that limits the flow of requests and let requests flow unhindered. [I hope you see where I am going with this. The term and implication dont align well]. If at all possible, can this field be renamed to "disable_flag" or something?

the name change sounds reasonable to me

Updated it to disable_key

junr03 · 2017-02-13T21:25:29Z

docs/configuration/http_conn_man/route_config/rate_limits.rst

 header_name
  *(required, string)* The header name to be queried from the request headers and used to
-  populate the descriptor value for the *descriptor_key*.
+  populate the descriptor entry value for the *descriptor_key*.


The header name to be queried from the request headers. The header's value is used to populate the value of the descriptor entry for the descriptor_key.

mattklein123 · 2017-02-13T22:18:14Z

docs/configuration/http_conn_man/route_config/rate_limits.rst

  *(required, string)* The type of rate limit action to perform. The currently supported action
-  types are *service_to_service* , *request_headers* and *remote_address*.
+  types are *source_cluster*, *destination_cluster* , *request_headers*, *remote_address* and
+  *route_key*.


rate_limit_key

changed it to generic_key

mattklein123 · 2017-02-13T22:20:20Z

docs/configuration/http_conn_man/route_config/rate_limits.rst

+
+  * ("to_cluster", "<routed cluster>")
+
+The routed cluster is chosen based on the :ref:`route table


This text is unclear. It should mention matching, and depending on the matched route, the value of cluster, weighted_clusters, etc.

mattklein123 · 2017-02-13T22:21:21Z

docs/configuration/http_conn_man/route_config/rate_limits.rst

+
+  {
+    "type": "rate_limit_key",
+    "rate_limit_value" : "..."


I would rename this "value", "rate_limit_" is redundant

changed it to descriptor_value to be consistent with other actions that set a descriptor key/value.

mattklein123 · 2017-02-13T22:22:30Z

docs/configuration/http_conn_man/route_config/rate_limits.rst

+create more complex rate limit descriptors, actions can be composed in any order. The descriptor
+will be populated in the order the actions are specified in the configuration.
+
+For example, if you wanted the following descriptor:


rephrase, avoid "you"

mattklein123 · 2017-02-13T22:24:01Z

docs/configuration/http_conn_man/route_config/rate_limits.rst

+.. code-block:: json
+
+  {
+    "type": "rate_limit_key",


I think I would just rename this "key" or "generic_key"

To me "rate_limit_ is also redundant here and somewhat verbose.

Renamed to generic_key

mattklein123 · 2017-02-14T01:08:17Z

docs/configuration/http_conn_man/route_config/rate_limits.rst

  }

-The following descriptor is sent using the trusted address from :ref:`x-forwarded-for <config_http_conn_man_headers_x-forwarded-for>`:
+The following descriptor entry is zappended to the descriptor and is populated using the trusted


to save the day! (Fixed)

mattklein123

looks good after typo fix

* changed additional header name * use lowercase for header name

envoyproxy#458)

**Commit Message** The CLI's standalone mode needs to use Envoy Gateway's latest patch [1] that allows us to use it inside our CLI. This upgrades the EG dependency version to the latest. It is not compatible with the latest cel-go, so this also pins it to 0.22 until the upstream k8s.io/apiserver dependency [2] issue is resolved. This commit is separated from the standalone implementation commit as the upgrade results in a CRD generation which is not relevant to the change. 1: envoyproxy/gateway#5407 2: https://github.com/kubernetes/apiserver/issues/116 **Related Issues/PRs (if applicable)** This is a preparation for #458 Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

**Commit Message** This makes changes to Main function of the extproc so that it will accept context as well as args, etc to allow the callsite to fine tune the behavior of extproc. **Related Issues/PRs (if applicable)** Extracted from #458 Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

**Commit Message** This changes the translator so that it will retain the non-AI Gateay original resources and write them back into the output. This also fixes the AI Gateway Route controller so that it can support the anonymous resources that will be necessary in the translator as the EG standalone mode requires the anonymous resources. **Related Issues/PRs (if applicable)** Extracted from #458 --------- Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

**Commit Message** This scaffolds `aigw run [path]` command that allows users to run AI Gateway natively without k8s/docker. **Related Issues/PRs (if applicable)** Extracted from #458 Contributes to #412 --------- Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

**Commit Message** This implements the `aigw run` command that runs the AI Gateway locally. This utilizes the Envoy Gateway's standalone mode [1]. With this command, users are now able to use the AI Gateway, without Docker or Kubernetes. The follow up PRs to further modify the default configuration to add more de-facto services such as ollama, and will add the command to show the default configuration. 1: https://gateway.envoyproxy.io/docs/tasks/operations/standalone-deployment-mode/ **Related Issues/PRs (if applicable)** Contributes to #412 Follow up on #498 --------- Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

ccaraman added 2 commits February 9, 2017 16:10

compose actions

946bd6e

fixes

c19e1c5

mattklein123 reviewed Feb 10, 2017

View reviewed changes

rshriram reviewed Feb 12, 2017

View reviewed changes

comments

6f43c0d

junr03 reviewed Feb 13, 2017

View reviewed changes

comments

a738d36

mattklein123 reviewed Feb 13, 2017

View reviewed changes

ccaraman added 3 commits February 13, 2017 16:25

inception words in words

1794cb6

remove space

230200c

clarity

d6b0d21

mattklein123 reviewed Feb 14, 2017

View reviewed changes

mattklein123 approved these changes Feb 14, 2017

View reviewed changes

zappended

08c48d1

ccaraman merged commit e9ab59d into master Feb 16, 2017

ccaraman deleted the compose-actions branch February 16, 2017 18:07

rshriram pushed a commit to rshriram/envoy that referenced this pull request Oct 30, 2018

jwt-auth: changed additional header name (envoyproxy#458)

5f48a26

* changed additional header name * use lowercase for header name

wolfguoliang pushed a commit to wolfguoliang/envoy that referenced this pull request Jan 23, 2021

zh-translation:docs/root/configuration/http/http_conn_man/overview.rst (

e75be2f

envoyproxy#458)


		The following descriptor entry is appended to the descriptor:

		* ("to_cluster", "<:ref:`route target cluster <config_http_conn_man_route_table_route_cluster>`>")

		@@ -25,14 +24,10 @@ stage
		kill_switch_key
		(optional, string) The key to be set in runtime to disable this rate limit configuration.


		* ("to_cluster", "<routed cluster>")

		The routed cluster is chosen based on the :ref:`route table

Conversation

ccaraman commented Feb 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccaraman commented Feb 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ccaraman commented Feb 10, 2017 •

edited

Loading