Skip to content

[WIP] Add configurable metric attributes deny list#3356

Open
creydr wants to merge 2 commits into
knative:mainfrom
creydr:metrics-attributes-deny
Open

[WIP] Add configurable metric attributes deny list#3356
creydr wants to merge 2 commits into
knative:mainfrom
creydr:metrics-attributes-deny

Conversation

@creydr
Copy link
Copy Markdown
Member

@creydr creydr commented May 21, 2026

Adds a metrics-attributes-deny config key to the observability ConfigMap. When set, the listed attribute keys (comma-separated) are stripped from all metric instruments via an OTel SDK View using attribute.NewDenyKeysFilter.

This prevents unbounded metric cardinality from attributes like cloudevents.type or messaging.destination.name that can cause OOM in production.

The deny filter is automatically applied in sharedmain.SetupObservabilityOrDie, so all components using sharedmain benefit without any component-specific code.

Example configuration:

apiVersion: v1
kind: ConfigMap
metadata:
  name: config-observability
data:
  metrics-attributes-deny: "cloudevents.type, messaging.destination.name"

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.
@knative-prow knative-prow Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 21, 2026
@knative-prow
Copy link
Copy Markdown

knative-prow Bot commented May 21, 2026

[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:

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

@knative-prow knative-prow Bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 21, 2026
@knative-prow knative-prow Bot requested review from Cali0707 and Leo6Leo May 21, 2026 08:04
@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.91%. Comparing base (91e8142) to head (7a12787).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3356      +/-   ##
==========================================
+ Coverage   74.85%   74.91%   +0.06%     
==========================================
  Files         189      190       +1     
  Lines        8347     8367      +20     
==========================================
+ Hits         6248     6268      +20     
  Misses       1854     1854              
  Partials      245      245              

☔ 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.

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.
@creydr creydr requested a review from maschmid May 21, 2026 14:14
creydr added a commit to creydr/knative-eventing that referenced this pull request May 21, 2026
Remove eventing-specific MetricAttributesDenyList field, parsing, and
filter function. Use cfg.Metrics.AttributesDenyList() and
metrics.MetricAttributesDenyFilter from knative.dev/pkg instead.

Rename ConfigMap key from metrics.attributes.deny to
metrics-attributes-deny to match knative.dev/pkg convention.

Depends on: knative/pkg#3356
@dprotaso
Copy link
Copy Markdown
Member

Do you think we should just remove metrics with high cardinality?

cc @Cali0707

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants