add LLMBackendTrafficPolicy #35
Conversation
wengyao04
commented
Dec 6, 2024
wengyao04
commented
- add LLMBackendTrafficPolicy, which controls the flow of traffic to the backend.
- add ratelimit in LLMBackendTrafficPolicy
wengyao04
force-pushed
the
trafficpolicy-ratelimit
branch
2 times, most recently
from
6b4acf3 to
27094a5
Compare
Signed-off-by: yweng14 <[email protected]>
wengyao04
force-pushed
the
trafficpolicy-ratelimit
branch
from
27094a5 to
309eaab
Compare
Signed-off-by: yweng14 <[email protected]>
wengyao04
requested review from
aabchoo,
mathetake and
missBerg
as code owners
There was a problem hiding this comment.
- do you want to add tests in https://github.com/envoyproxy/ai-gateway/tree/main/tests/cel-validation
- could you edit the PR description so that we can have more context and summary. e.g. how they are used and how do you envision them to be used to add other features. That ways reviewers can get to know the big picture before deciphering the code
| // LLMBackendTrafficPolicy controls the flow of traffic to the backend. | ||
| type LLMBackendTrafficPolicy struct { |
There was a problem hiding this comment.
could you add a bit more documentation here like for example this is used to setup rate limit etc.
Signed-off-by: yweng14 <[email protected]>
wengyao04
force-pushed
the
trafficpolicy-ratelimit
branch
from
46d1d38 to
3deaf80
Compare
| } | ||
|
|
||
| // LLMPolicyRateLimitHeaderMatch defines the match attributes within the HTTP Headers of the request. | ||
| type LLMPolicyRateLimitHeaderMatch struct { |
There was a problem hiding this comment.
Can we reuse the generic envoy gateway headerMatch type?
There was a problem hiding this comment.
yeah, I would like to reuse EG native type as much as possible
|
sorry the CI has a bug #36 this will fix it 🙏 |
| type LLMTrafficPolicyRateLimitRule struct { | ||
| // Headers is a list of request headers to match. Multiple header values are ANDed together, | ||
| // meaning, a request MUST match all the specified headers. | ||
| // At least one of headers or sourceCIDR condition must be specified. |
There was a problem hiding this comment.
it is not matching by sourceCIDR here, we can also document the canonical header such as x-ai-gateway-llm-model-name used to apply the rate limiting.
| // BackendRefs lists the LLMBackends that this traffic policy will apply | ||
| // The namespace is "local", i.e. the same namespace as the LLMRoute. | ||
| // | ||
| BackendRef LLMBackendLocalRef `json:"backendRef,omitempty"` |
There was a problem hiding this comment.
The description states "backendrefs lists the llmbackends" which implies that this variable should be updated to:
BackendRefs []LLMBackendLocalRef
Do we want a one (traffic policy) to many (backends) relationship? I think it makes sense to have that in the case where we have very similar models that we want to have the same rules for
