Skip to content

Commit

Permalink
Revert "ADD "disable_specific_metrics" which can remove specific metr…
Browse files Browse the repository at this point in the history
…ic by name or regex"
  • Loading branch information
bobbypage authored Mar 6, 2021
1 parent 373436e commit f89291a
Show file tree
Hide file tree
Showing 13 changed files with 15 additions and 711 deletions.
26 changes: 2 additions & 24 deletions cmd/cadvisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ var (
container.ResctrlMetrics: struct{}{},
container.CPUSetMetrics: struct{}{},
}
// metrics to be ignored,the difference between ignoreMetrics and ignoreSpecificMetrics is the former filters a certain type of metrics and the later filters specific metrics.
ignoreSpecificMetrics DenyList = DenyList{}
)

type metricSetValue struct {
Expand Down Expand Up @@ -144,27 +142,8 @@ func (ml *metricSetValue) Set(value string) error {
return nil
}

type DenyList []string

func (ms *DenyList) String() string {
return strings.Join(*ms, ",")
}

// Set converts a comma-separated string of metrics into a slice and appends it to the DenyList.
func (ms *DenyList) Set(value string) error {
metrics := strings.Split(value, ",")
for _, metric := range metrics {
metric = strings.TrimSpace(metric)
if len(metric) != 0 {
*ms = append(*ms, metric)
}
}
return nil
}

func init() {
flag.Var(&ignoreMetrics, "disable_metrics", "comma-separated list of `metrics` to be disabled. Options are 'accelerator', 'cpu_topology','disk', 'diskIO', 'memory_numa', 'network', 'tcp', 'udp', 'percpu', 'sched', 'process', 'hugetlb', 'referenced_memory', 'resctrl', 'cpuset'.")
flag.Var(&ignoreSpecificMetrics, "disable_specific_metrics", "Comma-separated list of metrics not to be enabled. This list comprises of exact metric names and/or regex patterns.It differs from the 'disable_metrics' in that 'disable_specific_metrics' filters specific metrics, while 'disable_metrics' filters a certain type of metrics.Besides, 'disable_specific_metrics' will only disable metrics being exported by Prometheus ")

// Default logging verbosity to V(2)
flag.Set("v", "2")
Expand All @@ -181,7 +160,6 @@ func main() {
}

includedMetrics := toIncludedMetrics(ignoreMetrics.MetricSet)
denyList, err := metrics.NewDenyList(ignoreSpecificMetrics)

setMaxProcs()

Expand All @@ -194,7 +172,7 @@ func main() {

collectorHttpClient := createCollectorHttpClient(*collectorCert, *collectorKey)

resourceManager, err := manager.New(memoryStorage, sysFs, housekeepingConfig, includedMetrics, &collectorHttpClient, strings.Split(*rawCgroupPrefixWhiteList, ","), *perfEvents, denyList)
resourceManager, err := manager.New(memoryStorage, sysFs, housekeepingConfig, includedMetrics, &collectorHttpClient, strings.Split(*rawCgroupPrefixWhiteList, ","), *perfEvents)
if err != nil {
klog.Fatalf("Failed to create a manager: %s", err)
}
Expand All @@ -221,7 +199,7 @@ func main() {
}

// Register Prometheus collector to gather information about containers, Go runtime, processes, and machine
cadvisorhttp.RegisterPrometheusHandler(mux, resourceManager, *prometheusEndpoint, containerLabelFunc, includedMetrics, denyList)
cadvisorhttp.RegisterPrometheusHandler(mux, resourceManager, *prometheusEndpoint, containerLabelFunc, includedMetrics)

// Start the manager.
if err := resourceManager.Start(); err != nil {
Expand Down
6 changes: 3 additions & 3 deletions cmd/internal/http/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ func RegisterHandlers(mux httpmux.Mux, containerManager manager.Manager, httpAut
// RegisterPrometheusHandler creates a new PrometheusCollector and configures
// the provided HTTP mux to handle the given Prometheus endpoint.
func RegisterPrometheusHandler(mux httpmux.Mux, resourceManager manager.Manager, prometheusEndpoint string,
f metrics.ContainerLabelsFunc, includedMetrics container.MetricSet, denyList *metrics.DenyList) {
f metrics.ContainerLabelsFunc, includedMetrics container.MetricSet) {
goCollector := prometheus.NewGoCollector()
processCollector := prometheus.NewProcessCollector(prometheus.ProcessCollectorOpts{})
machineCollector := metrics.NewPrometheusMachineCollector(resourceManager, includedMetrics, denyList)
machineCollector := metrics.NewPrometheusMachineCollector(resourceManager, includedMetrics)

mux.Handle(prometheusEndpoint, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
opts, err := api.GetRequestOptions(req)
Expand All @@ -111,7 +111,7 @@ func RegisterPrometheusHandler(mux httpmux.Mux, resourceManager manager.Manager,

r := prometheus.NewRegistry()
r.MustRegister(
metrics.NewPrometheusCollector(resourceManager, f, includedMetrics, clock.RealClock{}, opts, denyList),
metrics.NewPrometheusCollector(resourceManager, f, includedMetrics, clock.RealClock{}, opts),
machineCollector,
goCollector,
processCollector,
Expand Down
5 changes: 1 addition & 4 deletions manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
info "github.com/google/cadvisor/info/v1"
"github.com/google/cadvisor/info/v2"
"github.com/google/cadvisor/machine"
"github.com/google/cadvisor/metrics"
"github.com/google/cadvisor/nvm"
"github.com/google/cadvisor/perf"
"github.com/google/cadvisor/resctrl"
Expand Down Expand Up @@ -147,7 +146,7 @@ type HouskeepingConfig = struct {
}

// New takes a memory storage and returns a new manager.
func New(memoryCache *memory.InMemoryCache, sysfs sysfs.SysFs, houskeepingConfig HouskeepingConfig, includedMetricsSet container.MetricSet, collectorHTTPClient *http.Client, rawContainerCgroupPathPrefixWhiteList []string, perfEventsFile string, denyList *metrics.DenyList) (Manager, error) {
func New(memoryCache *memory.InMemoryCache, sysfs sysfs.SysFs, houskeepingConfig HouskeepingConfig, includedMetricsSet container.MetricSet, collectorHTTPClient *http.Client, rawContainerCgroupPathPrefixWhiteList []string, perfEventsFile string) (Manager, error) {
if memoryCache == nil {
return nil, fmt.Errorf("manager requires memory storage")
}
Expand Down Expand Up @@ -199,7 +198,6 @@ func New(memoryCache *memory.InMemoryCache, sysfs sysfs.SysFs, houskeepingConfig
maxHousekeepingInterval: *houskeepingConfig.Interval,
allowDynamicHousekeeping: *houskeepingConfig.AllowDynamic,
includedMetrics: includedMetricsSet,
DenyList: denyList,
containerWatchers: []watcher.ContainerWatcher{},
eventsChannel: eventsChannel,
collectorHTTPClient: collectorHTTPClient,
Expand Down Expand Up @@ -259,7 +257,6 @@ type manager struct {
maxHousekeepingInterval time.Duration
allowDynamicHousekeeping bool
includedMetrics container.MetricSet
DenyList *metrics.DenyList
containerWatchers []watcher.ContainerWatcher
eventsChannel chan watcher.ContainerEvent
collectorHTTPClient *http.Client
Expand Down
53 changes: 0 additions & 53 deletions metrics/deny_list.go

This file was deleted.

75 changes: 0 additions & 75 deletions metrics/deny_list_test.go

This file was deleted.

9 changes: 1 addition & 8 deletions metrics/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ type PrometheusCollector struct {
// ContainerLabelsFunc specifies which base labels will be attached to all
// exported metrics. If left to nil, the DefaultContainerLabels function
// will be used instead.
func NewPrometheusCollector(i infoProvider, f ContainerLabelsFunc, includedMetrics container.MetricSet, now clock.Clock, opts v2.RequestOptions, denyList *DenyList) *PrometheusCollector {
func NewPrometheusCollector(i infoProvider, f ContainerLabelsFunc, includedMetrics container.MetricSet, now clock.Clock, opts v2.RequestOptions) *PrometheusCollector {
if f == nil {
f = DefaultContainerLabels
}
Expand Down Expand Up @@ -1757,13 +1757,6 @@ func NewPrometheusCollector(i infoProvider, f ContainerLabelsFunc, includedMetri
},
}...)
}
var filtered []containerMetric
for _, val := range c.containerMetrics {
if !denyList.IsDenied(val.name) {
filtered = append(filtered, val)
}
}
c.containerMetrics = filtered
return c
}

Expand Down
9 changes: 1 addition & 8 deletions metrics/prometheus_machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ type PrometheusMachineCollector struct {
}

// NewPrometheusMachineCollector returns a new PrometheusCollector.
func NewPrometheusMachineCollector(i infoProvider, includedMetrics container.MetricSet, denyList *DenyList) *PrometheusMachineCollector {
func NewPrometheusMachineCollector(i infoProvider, includedMetrics container.MetricSet) *PrometheusMachineCollector {
c := &PrometheusMachineCollector{

infoProvider: i,
Expand Down Expand Up @@ -192,13 +192,6 @@ func NewPrometheusMachineCollector(i infoProvider, includedMetrics container.Met
},
}...)
}
var filtered []machineMetric
for _, val := range c.machineMetrics {
if !denyList.IsDenied(val.name) {
filtered = append(filtered, val)
}
}
c.machineMetrics = filtered
return c
}

Expand Down
33 changes: 2 additions & 31 deletions metrics/prometheus_machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,12 @@ package metrics
import (
"bytes"
"io/ioutil"
"os"
"reflect"
"testing"
"time"

"github.com/google/cadvisor/container"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/prometheus/common/expfmt"
"github.com/stretchr/testify/assert"
)
Expand All @@ -33,9 +31,7 @@ const machineMetricsFile = "testdata/prometheus_machine_metrics"
const machineMetricsFailureFile = "testdata/prometheus_machine_metrics_failure"

func TestPrometheusMachineCollector(t *testing.T) {
denyList, err := NewDenyList([]string{})
assert.Nil(t, err)
collector := NewPrometheusMachineCollector(testSubcontainersInfoProvider{}, container.AllMetrics, denyList)
collector := NewPrometheusMachineCollector(testSubcontainersInfoProvider{}, container.AllMetrics)
registry := prometheus.NewRegistry()
registry.MustRegister(collector)

Expand All @@ -55,13 +51,11 @@ func TestPrometheusMachineCollector(t *testing.T) {
}

func TestPrometheusMachineCollectorWithFailure(t *testing.T) {
denyList, err := NewDenyList([]string{})
assert.Nil(t, err)
provider := &erroringSubcontainersInfoProvider{
successfulProvider: testSubcontainersInfoProvider{},
shouldFail: true,
}
collector := NewPrometheusMachineCollector(provider, container.AllMetrics, denyList)
collector := NewPrometheusMachineCollector(provider, container.AllMetrics)
registry := prometheus.NewRegistry()
registry.MustRegister(collector)

Expand All @@ -79,29 +73,6 @@ func TestPrometheusMachineCollectorWithFailure(t *testing.T) {
assert.Equal(t, string(expectedMetrics), collectedMetrics)
}

func TestPrometheusMachineCollectorWithDenyList(t *testing.T) {
denyList, err := NewDenyList(ignoreSpecificMetrics)
assert.Nil(t, err)
cMachine := NewPrometheusMachineCollector(testSubcontainersInfoProvider{}, container.AllMetrics, denyList)
registry := prometheus.NewRegistry()
registry.MustRegister(cMachine)

testPrometheusMachineCollectorWithIsDenied(t, registry, "testdata/prometheus_machine_metrics_denylist")

}

func testPrometheusMachineCollectorWithIsDenied(t *testing.T, gatherer prometheus.Gatherer, metricsFile string) {
wantMetrics, err := os.Open(metricsFile)
if err != nil {
t.Fatalf("unable to read input test file %s", metricsFile)
}

err = testutil.GatherAndCompare(gatherer, wantMetrics)
if err != nil {
t.Fatalf("Metric comparison failed: %s", err)
}
}

func TestGetMemoryByType(t *testing.T) {
machineInfo, err := testSubcontainersInfoProvider{}.GetMachineInfo()
assert.Nil(t, err)
Expand Down
Loading

0 comments on commit f89291a

Please sign in to comment.