Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mdatagen] allow filtering out metrics based on resource attributes #9660

Merged
merged 3 commits into from
Apr 12, 2024

Conversation

povilasv
Copy link
Contributor

Description:

This PR allows filtering out metrics based on resource attributes. For example:

        resource_attributes:                                                                      
          k8s.pod.name:
            enabled: true 
            exclude:
              #- strict: "kube-apiserver-kind-control-plane"
              - regexp: "kube-.*"
              - regexp: "coredns-.*"
              - strict: "coredns"
              - strict: "kindnet-mpb2p"

Would remove metrics that match regex or strict rules on resource attributes.

Link to tracking Issue:

open-telemetry/opentelemetry-collector-contrib#25134

Testing:

  • Tested with k8scluster receiver in kind cluster.
  • unit tests added
    Documentation:

@povilasv povilasv force-pushed the mdatagen-filter branch 5 times, most recently from d38c6b3 to 7dceff3 Compare February 28, 2024 11:45
@povilasv povilasv force-pushed the mdatagen-filter branch 4 times, most recently from 7c14f45 to 269193f Compare February 28, 2024 11:59
@povilasv povilasv marked this pull request as ready for review February 28, 2024 12:03
@povilasv povilasv requested review from a team and dmitryax February 28, 2024 12:03
Copy link

codecov bot commented Feb 28, 2024

Codecov Report

Attention: Patch coverage is 84.28571% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 91.91%. Comparing base (0d62e4f) to head (d054b0e).

Files Patch % Lines
...plereceiver/internal/metadata/generated_metrics.go 75.60% 5 Missing and 5 partials ⚠️
filter/config.go 96.55% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9660      +/-   ##
==========================================
- Coverage   91.95%   91.91%   -0.04%     
==========================================
  Files         357      358       +1     
  Lines       16501    16571      +70     
==========================================
+ Hits        15173    15232      +59     
- Misses       1000     1006       +6     
- Partials      328      333       +5     

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

@dmitryax
Copy link
Member

dmitryax commented Mar 6, 2024

@povilasv can you please rebase? I finalized the mdatagen migration from the contrib repo

@povilasv povilasv force-pushed the mdatagen-filter branch 2 times, most recently from 073cd48 to 5893642 Compare March 6, 2024 08:09
@povilasv
Copy link
Contributor Author

povilasv commented Mar 6, 2024

@dmitryax rebased

cmd/mdatagen/templates/config.go.tmpl Show resolved Hide resolved
@@ -16,6 +16,9 @@ import (
{{- if .SemConvVersion }}
conventions "go.opentelemetry.io/collector/semconv/v{{ .SemConvVersion }}"
{{- end }}
{{ if .ResourceAttributes -}}
"go.opentelemetry.io/collector/filter"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@open-telemetry/collector-contrib-approvers, FIY: we are going to introduce another core module for this filtering functionality as discussed in open-telemetry/opentelemetry-collector-contrib#25161.

It has to be introduced as a public API in the core. I'm not sure what can be a better place other than filter. Any suggestions are welcome.

Copy link
Member

@andrzej-stencel andrzej-stencel Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like it's for defining the configuration, shouldn't it be something like config/configfilter?

If it's also for matching, then perhaps more like config/configmatch.

Copy link
Contributor Author

@povilasv povilasv Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It defines both configuration and the matching logic. We can split into two packages if needed, but currently it's really small package so I wouldn't 'bother 🤔

What about filter/filtermatcher?

In contrib we called following naming conventions:

https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/internal/filter

cmd/mdatagen/templates/metrics.go.tmpl Show resolved Hide resolved
@povilasv povilasv force-pushed the mdatagen-filter branch 5 times, most recently from fe5d38f to 59a4e27 Compare March 27, 2024 10:05
@dmitryax dmitryax merged commit 8cc8d40 into open-telemetry:main Apr 12, 2024
48 of 49 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 12, 2024
dmitryax added a commit that referenced this pull request Apr 12, 2024
…#9950)

- Make sure we always pass a string to the filter.Match even if the
attribute value has a different type. Otherwise, it panics.
- Make sure we show the if_configured warning if the user sets
include/exclude without enabled.
- Simplify generated tests

Follow up to
#9660
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants