[DNM] k8s gateway cluster names #10403
Conversation
| @@ -68,10 +66,6 @@ type translatorInstance struct { | |||
| shouldEnforceNamespaceMatch bool | |||
| } | |||
|
|
|||
| func NewDefaultTranslator(settings *v1.Settings, pluginRegistry plugins.PluginRegistry) *translatorInstance { | |||
There was a problem hiding this comment.
i guess this wasn't used anywhere
|
Issues linked to changelog: |
|
Visit the preview URL for this PR (updated for commit ba951c4): https://gloo-edge--pr10403-kubegateway-clustern-c73iax59.web.app (expires Thu, 05 Dec 2024 19:52:08 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 77c2b86e287749579b7ff9cadb81e099042ef677 |
| } | ||
|
|
||
| // Don't use dots in the name as it messes up prometheus stats | ||
| return fmt.Sprintf("%s_%s", ref.GetName(), ref.GetNamespace()) |
There was a problem hiding this comment.
Is there a limit on the max length? IIRC grafana has something around 128 chars which might be similar for promethus
There was a problem hiding this comment.
Not sure which Grafana limit you have in mind (it's just the visualization layer backed by a regular database), but in Prometheus one can specify limits to label name/values in bytes. These are all 0 by default.
While no one is a big fan of long metric/label names, Envoy is doing a pretty terrible job on this front already and we aren't really making that worse.
| @@ -30,7 +54,7 @@ var _ = Describe("Plugin", func() { | |||
| }, | |||
| } | |||
| out := &envoy_config_route_v3.Route{} | |||
| err := p.ProcessRoute(plugins.RouteParams{}, in, out) | |||
There was a problem hiding this comment.
Would we need to test an empty RouteParams ?
Description
TODO:
API changes
Code changes
CI changes
Docs changes
Context
Interesting decisions
Testing steps
Notes for reviewers
Checklist: