From 52dbf0cc23d26c984f61edebe9f770e055099ac1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Thu, 21 May 2026 10:01:46 +0200 Subject: [PATCH 1/2] Add configurable metric attributes deny list Add a metrics-attributes-deny config key (comma-separated) that strips the listed attribute keys from all instruments via an OTel View. This prevents unbounded metric cardinality from attributes like cloudevents.type or messaging.destination.name. --- injection/sharedmain/main.go | 7 ++- observability/metrics/config.go | 15 ++++++ observability/metrics/config_test.go | 34 +++++++++++-- observability/metrics/view.go | 37 +++++++++++++++ observability/metrics/view_test.go | 71 ++++++++++++++++++++++++++++ 5 files changed, 160 insertions(+), 4 deletions(-) create mode 100644 observability/metrics/view.go create mode 100644 observability/metrics/view_test.go diff --git a/injection/sharedmain/main.go b/injection/sharedmain/main.go index 50d5f4c798..55d6dce17e 100644 --- a/injection/sharedmain/main.go +++ b/injection/sharedmain/main.go @@ -399,10 +399,15 @@ func SetupObservabilityOrDie( resource := resource.Default(component) + views := OTelViews(ctx) + if len(cfg.Metrics.AttributesDenyList) > 0 { + views = append(views, metrics.MetricAttributesDenyFilter(cfg.Metrics.AttributesDenyList)) + } + meterProvider, err := metrics.NewMeterProvider( ctx, cfg.Metrics, - metric.WithView(OTelViews(ctx)...), + metric.WithView(views...), metric.WithResource(resource), ) if err != nil { diff --git a/observability/metrics/config.go b/observability/metrics/config.go index dc911c2e6d..90fa44a8c4 100644 --- a/observability/metrics/config.go +++ b/observability/metrics/config.go @@ -18,6 +18,7 @@ package metrics import ( "fmt" + "strings" "time" configmap "knative.dev/pkg/configmap/parser" @@ -38,6 +39,10 @@ type Config struct { Protocol string `json:"protocol,omitempty"` Endpoint string `json:"endpoint,omitempty"` ExportInterval time.Duration `json:"exportInterval,omitempty"` + + // AttributesDenyList is a list of metric attribute keys to filter out + // from all instruments (e.g. "cloudevents.type,messaging.destination.name"). + AttributesDenyList []string `json:"attributesDenyList,omitempty"` } func (c *Config) Validate() error { @@ -78,6 +83,16 @@ func NewFromMap(m map[string]string) (Config, error) { func NewFromMapWithPrefix(prefix string, m map[string]string) (Config, error) { c := DefaultConfig() + if v, ok := m[prefix+"metrics-attributes-deny"]; ok && v != "" { + parts := strings.Split(v, ",") + c.AttributesDenyList = make([]string, 0, len(parts)) + for _, p := range parts { + if t := strings.TrimSpace(p); t != "" { + c.AttributesDenyList = append(c.AttributesDenyList, t) + } + } + } + err := configmap.Parse(m, configmap.As(prefix+"metrics-protocol", &c.Protocol), configmap.As(prefix+"metrics-endpoint", &c.Endpoint), diff --git a/observability/metrics/config_test.go b/observability/metrics/config_test.go index 944cc4d7a8..8e55f0beda 100644 --- a/observability/metrics/config_test.go +++ b/observability/metrics/config_test.go @@ -66,20 +66,48 @@ func TestNewFromMapBadInput(t *testing.T) { } } +func TestNewFromMapAttributesDenyList(t *testing.T) { + got, err := NewFromMap(map[string]string{ + "metrics-attributes-deny": "cloudevents.type, messaging.destination.name", + }) + if err != nil { + t.Fatal("unexpected error:", err) + } + + want := []string{"cloudevents.type", "messaging.destination.name"} + if diff := cmp.Diff(want, got.AttributesDenyList); diff != "" { + t.Error("unexpected diff (-want +got): ", diff) + } +} + +func TestNewFromMapAttributesDenyListEmpty(t *testing.T) { + got, err := NewFromMap(nil) + if err != nil { + t.Fatal("unexpected error:", err) + } + + if got.AttributesDenyList != nil { + t.Errorf("expected nil deny list, got %v", got.AttributesDenyList) + } +} + func TestNewFromMapWithPrefix(t *testing.T) { got, err := NewFromMapWithPrefix("request-", map[string]string{ "request-metrics-protocol": ProtocolGRPC, "request-metrics-endpoint": "https://blah.example.com", "request-metrics-export-interval": "15s", + "request-metrics-attributes-deny": "cloudevents.type", + "metrics-attributes-deny": "should.be.ignored", }) if err != nil { t.Error("unexpected error", err) } want := Config{ - Protocol: ProtocolGRPC, - Endpoint: "https://blah.example.com", - ExportInterval: 15 * time.Second, + Protocol: ProtocolGRPC, + Endpoint: "https://blah.example.com", + ExportInterval: 15 * time.Second, + AttributesDenyList: []string{"cloudevents.type"}, } if diff := cmp.Diff(want, got); diff != "" { diff --git a/observability/metrics/view.go b/observability/metrics/view.go new file mode 100644 index 0000000000..00406e126b --- /dev/null +++ b/observability/metrics/view.go @@ -0,0 +1,37 @@ +/* +Copyright 2026 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric" +) + +// MetricAttributesDenyFilter returns a View that strips the given attribute +// keys from every instrument. +func MetricAttributesDenyFilter(denyList []string) metric.View { + keys := make([]attribute.Key, len(denyList)) + for i, k := range denyList { + keys[i] = attribute.Key(k) + } + return metric.NewView( + metric.Instrument{Name: "*"}, + metric.Stream{ + AttributeFilter: attribute.NewDenyKeysFilter(keys...), + }, + ) +} diff --git a/observability/metrics/view_test.go b/observability/metrics/view_test.go new file mode 100644 index 0000000000..6c270c9f3f --- /dev/null +++ b/observability/metrics/view_test.go @@ -0,0 +1,71 @@ +/* +Copyright 2026 The Knative Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package metrics + +import ( + "testing" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/sdk/metric" +) + +func TestMetricAttributesDenyFilter(t *testing.T) { + view := MetricAttributesDenyFilter([]string{"cloudevents.type", "messaging.destination.name"}) + + stream, ok := view(metric.Instrument{Name: "some.metric"}) + if !ok { + t.Fatal("view should match all instruments") + } + if stream.AttributeFilter == nil { + t.Fatal("expected non-nil attribute filter") + } + + denied := []attribute.KeyValue{ + attribute.String("cloudevents.type", "com.example.event"), + attribute.String("messaging.destination.name", "my-destination"), + } + for _, kv := range denied { + if stream.AttributeFilter(kv) { + t.Errorf("attribute %s should be denied", kv.Key) + } + } + + allowed := []attribute.KeyValue{ + attribute.String("messaging.system", "knative"), + attribute.Int("http.response.status_code", 200), + } + for _, kv := range allowed { + if !stream.AttributeFilter(kv) { + t.Errorf("attribute %s should be allowed", kv.Key) + } + } +} + +func TestMetricAttributesDenyFilterMatchesAllInstruments(t *testing.T) { + view := MetricAttributesDenyFilter([]string{"cloudevents.type"}) + + instruments := []string{ + "kn.eventing.dispatch.duration", + "http.server.request.duration", + "custom.metric", + } + for _, name := range instruments { + if _, ok := view(metric.Instrument{Name: name}); !ok { + t.Errorf("view should match instrument %s", name) + } + } +} From 7a12787199deba862eda7f41e7e9ddb94c6c95fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20St=C3=A4bler?= Date: Thu, 21 May 2026 10:23:59 +0200 Subject: [PATCH 2/2] Use string instead of []string for AttributesDeny config field Slices make the Config struct non-comparable, which breaks downstream code that uses == to check for empty configs. Store the raw comma-separated string and provide an AttributesDenyList() method for the parsed result. --- injection/sharedmain/main.go | 4 ++-- observability/metrics/config.go | 35 +++++++++++++++++----------- observability/metrics/config_test.go | 18 +++++++------- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/injection/sharedmain/main.go b/injection/sharedmain/main.go index 55d6dce17e..a44fde056d 100644 --- a/injection/sharedmain/main.go +++ b/injection/sharedmain/main.go @@ -400,8 +400,8 @@ func SetupObservabilityOrDie( resource := resource.Default(component) views := OTelViews(ctx) - if len(cfg.Metrics.AttributesDenyList) > 0 { - views = append(views, metrics.MetricAttributesDenyFilter(cfg.Metrics.AttributesDenyList)) + if denyList := cfg.Metrics.AttributesDenyList(); len(denyList) > 0 { + views = append(views, metrics.MetricAttributesDenyFilter(denyList)) } meterProvider, err := metrics.NewMeterProvider( diff --git a/observability/metrics/config.go b/observability/metrics/config.go index 90fa44a8c4..6cb742aa8f 100644 --- a/observability/metrics/config.go +++ b/observability/metrics/config.go @@ -40,9 +40,27 @@ type Config struct { Endpoint string `json:"endpoint,omitempty"` ExportInterval time.Duration `json:"exportInterval,omitempty"` - // AttributesDenyList is a list of metric attribute keys to filter out - // from all instruments (e.g. "cloudevents.type,messaging.destination.name"). - AttributesDenyList []string `json:"attributesDenyList,omitempty"` + // AttributesDeny is a comma-separated list of metric attribute keys to + // filter out from all instruments (e.g. "cloudevents.type,messaging.destination.name"). + // Stored as a string rather than []string to keep Config comparable, + // which is relied upon by downstream consumers. Use AttributesDenyList() + // to get the parsed list. + AttributesDeny string `json:"attributesDeny,omitempty"` +} + +// AttributesDenyList returns the deny list parsed into individual keys. +func (c *Config) AttributesDenyList() []string { + if c.AttributesDeny == "" { + return nil + } + parts := strings.Split(c.AttributesDeny, ",") + result := make([]string, 0, len(parts)) + for _, p := range parts { + if t := strings.TrimSpace(p); t != "" { + result = append(result, t) + } + } + return result } func (c *Config) Validate() error { @@ -83,17 +101,8 @@ func NewFromMap(m map[string]string) (Config, error) { func NewFromMapWithPrefix(prefix string, m map[string]string) (Config, error) { c := DefaultConfig() - if v, ok := m[prefix+"metrics-attributes-deny"]; ok && v != "" { - parts := strings.Split(v, ",") - c.AttributesDenyList = make([]string, 0, len(parts)) - for _, p := range parts { - if t := strings.TrimSpace(p); t != "" { - c.AttributesDenyList = append(c.AttributesDenyList, t) - } - } - } - err := configmap.Parse(m, + configmap.As(prefix+"metrics-attributes-deny", &c.AttributesDeny), configmap.As(prefix+"metrics-protocol", &c.Protocol), configmap.As(prefix+"metrics-endpoint", &c.Endpoint), configmap.As(prefix+"metrics-export-interval", &c.ExportInterval), diff --git a/observability/metrics/config_test.go b/observability/metrics/config_test.go index 8e55f0beda..9403b97b5a 100644 --- a/observability/metrics/config_test.go +++ b/observability/metrics/config_test.go @@ -66,7 +66,7 @@ func TestNewFromMapBadInput(t *testing.T) { } } -func TestNewFromMapAttributesDenyList(t *testing.T) { +func TestNewFromMapAttributesDeny(t *testing.T) { got, err := NewFromMap(map[string]string{ "metrics-attributes-deny": "cloudevents.type, messaging.destination.name", }) @@ -75,19 +75,19 @@ func TestNewFromMapAttributesDenyList(t *testing.T) { } want := []string{"cloudevents.type", "messaging.destination.name"} - if diff := cmp.Diff(want, got.AttributesDenyList); diff != "" { + if diff := cmp.Diff(want, got.AttributesDenyList()); diff != "" { t.Error("unexpected diff (-want +got): ", diff) } } -func TestNewFromMapAttributesDenyListEmpty(t *testing.T) { +func TestNewFromMapAttributesDenyEmpty(t *testing.T) { got, err := NewFromMap(nil) if err != nil { t.Fatal("unexpected error:", err) } - if got.AttributesDenyList != nil { - t.Errorf("expected nil deny list, got %v", got.AttributesDenyList) + if got.AttributesDenyList() != nil { + t.Errorf("expected nil deny list, got %v", got.AttributesDenyList()) } } @@ -104,10 +104,10 @@ func TestNewFromMapWithPrefix(t *testing.T) { } want := Config{ - Protocol: ProtocolGRPC, - Endpoint: "https://blah.example.com", - ExportInterval: 15 * time.Second, - AttributesDenyList: []string{"cloudevents.type"}, + Protocol: ProtocolGRPC, + Endpoint: "https://blah.example.com", + ExportInterval: 15 * time.Second, + AttributesDeny: "cloudevents.type", } if diff := cmp.Diff(want, got); diff != "" {