add cluster metadata to a route#598
Conversation
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
mattklein123
left a comment
There was a problem hiding this comment.
LGTM, small comment. Thanks!
envoy/api/v2/route/route.proto
Outdated
| // For instance, if the metadata is intended for the Router filter, | ||
| // the filter name should be specified as *envoy.router*. | ||
| // | ||
| // On runtime, this object will a merged with the cluster_metadata from the enclosing |
There was a problem hiding this comment.
nit: please ref link to the enclosing "cluster_metadata" field.
Signed-off-by: Yuval Kohavi <yuval.kohavi@gmail.com>
mattklein123
left a comment
There was a problem hiding this comment.
LGTM will leave for a bit to see if anyone else has any comments.
|
Sounds good, thank you! |
|
LGTM with one question: does it make sense to treat this as cluster metadata or route metadata? |
|
its a bit of both - its the metadata of the cluster chosen for routing. specifically it's useful in the case of traffic splitting where you don't know in advance which cluster will be chosen. |
|
OK, seems legit. I had originally envisaged having all the metadata that factors into a particular route or cluster selection being merged anyway prior to hitting logging/stats. That didn't end up happening, but this would fit that model if we do go there some day. |
Per envoyproxy/envoy#2851, adding metadata object to the specific routed cluster. envoy patch is ready and a PR for envoy will be created when this PR is merged (so I can update the repository_locations.bzl)