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

Remove redundant cadvisor relabel rules in alloyModule #1248

Closed

Conversation

PaulFarver
Copy link

@PaulFarver PaulFarver commented Feb 18, 2025

Thanks for maintaining this awesome chart! It has saved us a lot of trouble ❤️

We currently have trouble with our openshift clusters, where cadvisor does not populate the image label on container_network_.* metrics for some reason. We saw the same problem with v1 of the chart, but were able to work around it with dropEmptyImageLabels: false and an extraMetricProcessingRules

The options in the k8s-monitoring chart v2:

cadvisor.metricsTuning.dropEmptyContainerLabels
cadvisor.metricsTuning.dropEmptyImageLabels
cadvisor.metricsTuning.normalizeUnnecessaryLabels
cadvisor.metricsTuning.keepPhysicalFilesystemDevices
cadvisor.metricsTuning.keepPhysicalNetworkDevices

currently do not seem to have an impact on how alloy behaves in reality, because these rules are always included through the -alloy-module-kubernetes configmap.

Looking at our output k8s-monitoring-alloy-metrics configmap, we have this:

data:
  config.alloy: |-
    ...
    // Feature: Cluster Metrics
    declare "cluster_metrics" {
      argument "metrics_destinations" {
        comment = "Must be a list of metric destinations where collected metrics should be forwarded to"
      }

      remote.kubernetes.configmap "kubernetes" {
        name = "k8s-monitoring-alloy-module-kubernetes"
        namespace = "k8s-monitoring"
      }

      import.string "kubernetes" {
        content = remote.kubernetes.configmap.kubernetes.data["core_metrics.alloy"]
      }
    ...

The k8s-monitoring-alloy-module-kubernetes includes the drop rules, but we cannot disable these

charts/k8s-monitoring/charts/feature-cluster-metrics/templates/_cadvisor.alloy.tpl and charts/k8s-monitoring/alloyModules/modules/kubernetes/core/metrics.alloy seem to have the same set of rules, but charts/k8s-monitoring/charts/feature-cluster-metrics/templates/_cadvisor.alloy.tpl is configurable

@CLAassistant
Copy link

CLAassistant commented Feb 18, 2025

CLA assistant check
All committers have signed the CLA.

@petewall
Copy link
Collaborator

Thanks, but in #1257, I'm actually breaking the use of alloyModules for cadvisor. That PR will have a more permanent and cleaner solution.

@PaulFarver
Copy link
Author

@petewall Cool. Looking forward to it 👍
I'll close this PR, then

@PaulFarver PaulFarver closed this Feb 20, 2025
@petewall
Copy link
Collaborator

This should be doable now with the 2.0.12 release!

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