[WIP] Add configurable metric attributes deny list to config-observability

[creydr]

Proposed Changes

• 🎁 Add a metrics.attributes.deny key to the config-observability ConfigMap that accepts a comma-separated list of metric attribute keys to filter out
  • When configured, an OTel View strips the specified attributes from all kn.eventing.* metrics at the SDK level
  • Default is empty (existing behavior preserved, no filtering)

Example usage to strip high-cardinality attributes:

metrics.attributes.deny: "cloudevents.type,messaging.destination.name"

Add `metrics.attributes.deny` option to config-observability ConfigMap to allow filtering out configurable metric attributes from kn.eventing.* metrics, helping prevent potential OOM issues caused by unbounded metric cardinality in production.

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: creydr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [creydr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[codecov]

Codecov Report

❌ Patch coverage is 82.60870% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.03%. Comparing base (504f7e0) to head (99cabfb).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/observability/otel/otel.go	71.42%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9090      +/-   ##
==========================================
+ Coverage   50.99%   51.03%   +0.04%     
==========================================
  Files         411      411              
  Lines       22112    22133      +21     
==========================================
+ Hits        11276    11296      +20     
- Misses       9967     9969       +2     
+ Partials      869      868       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[creydr]

Waiting until all pods in namespace cert-manager are up......................................................................................................................................................
ERROR: timeout waiting for pods to come up

/test reconciler-tests

[maschmid]

Does this mean the filter would only filter the kn_eventing metrics, and not touch e.g. the http metrics, like

http_client_request_body_size_bytes_count ? (because , those also have stuff like

(from prometheus:)

http_client_request_body_size_bytes_count{cloudevents_datacontenttype="application/json",cloudevents_source="ocf-qe/test/imc-broker-k8s-counter-0/counter-0",cloudevents_specversion="1.0",cloudevents_type="com.redhat.cee.gitlab.ocf-qe.ocf-qe-testsuite.structured",http_request_method="POST",http_response_status_code="202",kn_broker_name="broker-0",kn_broker_namespace="imc-broker-k8s-counter-0",messaging_destination_name="trigger:receiver-0.imc-broker-k8s-counter-0",messaging_operation_name="send",messaging_system="knative",network_protocol_name="http",network_protocol_version="1.1",otel_scope_name="go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",otel_scope_schema_url="",otel_scope_version="0.64.0",server_address="receiver-0.imc-broker-k8s-counter-0.svc.cluster.local",url_scheme="http"} 5

which might be problematic.

[creydr]

yes this filter applies only to kn_eventing metrics (I thought, that the "problematic" ones are from there only).
But we could also add another config field, which allows the user to set this (defaulting to "kn.eventing.*" for example) 🤷

[maschmid]

well, looking at mt-broker-filter and mt-broker-ingress, the most problematic ones are in the http.*

[creydr]

ah ok. then lets use * as filter (instead of adding another config option)

[maschmid]

comment should be updated (not specific to kn.eventing.* anymore)

[maschmid]

I have tried building and running the patch on top of serverless release-v1.21 , it seems to work for the mt-broker filter and ingress, but not for the imc-dispatcher.

Claude says:

Check if the in_memory channel_dispatcher initializes the otel in such way that this new setting also applies there.                                                                                                                                                                    

● Explore(Trace channel dispatcher OTel init)
  ⎿  Done (29 tool uses · 53.0k tokens · 1m 20s)
  (ctrl+o to expand)

● No, the in-memory channel dispatcher will NOT pick up the deny filter. It uses a completely different observability init path.

  The channel dispatcher calls sharedmain.MainWithContext() from knative.dev/pkg, which has its own SetupObservabilityOrDie — separate from eventing's version in pkg/observability/otel/otel.go. The eventing-specific metricAttributesDenyFilter view is never injected into that path.

  Here's how the two paths compare:

  ┌────────────────────────────────────────┬──────────────────────────────────────────────────────┬───────────────────┐
  │               Component                │                      Init path                       │ Gets deny filter? │
  ├────────────────────────────────────────┼──────────────────────────────────────────────────────┼───────────────────┤
  │ broker/filter, broker/ingress, jobsink │ eventing's otel.SetupObservabilityOrDie              │ Yes               │
  ├────────────────────────────────────────┼──────────────────────────────────────────────────────┼───────────────────┤
  │ in-memory channel dispatcher           │ knative.dev/pkg/injection/sharedmain.MainWithContext │ No                │
  └────────────────────────────────────────┴──────────────────────────────────────────────────────┴───────────────────┘

  The dispatcher's controller (pkg/reconciler/inmemorychannel/dispatcher/controller.go:86) does call otel.GetMeterProvider() to get the global, but that global was set by sharedmain — not by eventing's setup function — so it has no views attached.

  To fix this, knative.dev/pkg's sharedmain already supports WithOTelViews(ctx, ...metric.View) via context. In cmd/in_memory/channel_dispatcher/main.go, you could read the observability configmap, parse the deny list, and call sharedmain.WithOTelViews(ctx, 
  metricAttributesDenyFilter(denyList)) before sharedmain.MainWithContext(). Alternatively, you could push the deny-list view support into knative.dev/pkg's sharedmain itself so all components get it automatically.

[creydr]

@maschmid I've started moving this functionality into knative.dev/pkg directly: knative/pkg#3356

The main reason is that components like the IMC channel dispatcher use sharedmain.SetupObservabilityOrDie from knative.dev/pkg instead of eventing's own observability setup as you pointed out. To make the deny filter work there from eventing, we'd need to do a one-shot kubeclient call in the dispatcher's main.go to read the config-observability ConfigMap, parse the deny list, construct the View, and inject it via sharedmain.WithOTelViews(ctx, ...) all before calling sharedmain.MainWithContext, having the following issues:

• It duplicates config reading that sharedmain already does internally (it reads config-observability as part of SetupObservabilityOrDie)
• It requires building a kubeclient earlier than normal just to read one config key
• Every component using sharedmain would need the same boilerplate

By adding metrics-attributes-deny in knative.dev/pkg (alongside metrics-protocol, metrics-endpoint, etc.), the deny filter is automatically wired into sharedmain.SetupObservabilityOrDie. This means every Knative component using sharedmain (eventing, serving, etc.) gets deny-list support without any component-specific code.

Once the knative.dev/pkg PR is merged, we don't need this PR here anymore and only need to revendor it.

Once the knative.dev/pkg PR is merged and it got vendored into eventing-core, we can rebase this PR (we still need it for components which not use sharedmain.SetupObservabilityOrDie (like mt-broker-filter and -ingress)

[creydr]

/hold

[knative-prow]

@creydr: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
build-tests_eventing_main	c288ebb	link	true	/test build-tests
unit-tests_eventing_main	c288ebb	link	true	/test unit-tests
upgrade-tests_eventing_main	c288ebb	link	true	/test upgrade-tests
reconciler-tests_eventing_main	c288ebb	link	true	/test reconciler-tests
conformance-tests_eventing_main	c288ebb	link	true	/test conformance-tests

Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.