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

allow ignoreMetrics in new manager #1159

Merged
merged 1 commit into from
Mar 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 40 additions & 1 deletion cadvisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,11 @@ import (
"os"
"os/signal"
"runtime"
"strings"
"syscall"
"time"

"github.com/google/cadvisor/container"
cadvisorhttp "github.com/google/cadvisor/http"
"github.com/google/cadvisor/manager"
"github.com/google/cadvisor/utils/sysfs"
Expand Down Expand Up @@ -52,6 +54,43 @@ var allowDynamicHousekeeping = flag.Bool("allow_dynamic_housekeeping", true, "Wh

var enableProfiling = flag.Bool("profiling", false, "Enable profiling via web interface host:port/debug/pprof/")

var (
// Metrics to be ignored.
ignoreMetrics metricSetValue = metricSetValue{container.MetricSet{}}

// List of metrics that can be ignored.
ignoreWhitelist = container.MetricSet{
container.DiskUsageMetrics: struct{}{},
container.NetworkUsageMetrics: struct{}{},
container.NetworkTcpUsageMetrics: struct{}{},
}
)

type metricSetValue struct {
container.MetricSet
}

func (ml *metricSetValue) String() string {
return fmt.Sprint(*ml)
}

func (ml *metricSetValue) Set(value string) error {
for _, metric := range strings.Split(value, ",") {
if ignoreWhitelist.Has(container.MetricKind(metric)) {
(*ml).Add(container.MetricKind(metric))
} else {
return fmt.Errorf("unsupported metric %q specified in disable_metrics", metric)
}
}
return nil
}

func init() {
flag.Var(&ignoreMetrics, "disable_metrics", "comma-separated list of metrics to be disabled. Options are `disk`, `network`, `tcp`. Note: tcp is disabled by default due to high CPU usage.")
// Tcp metrics are ignored by default.
flag.Set("disable_metrics", "tcp")
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than calling flag.Set, just add TCP to the default metrics on line 58

}

func main() {
defer glog.Flush()
flag.Parse()
Expand All @@ -73,7 +112,7 @@ func main() {
glog.Fatalf("Failed to create a system interface: %s", err)
}

containerManager, err := manager.New(memoryStorage, sysFs, *maxHousekeepingInterval, *allowDynamicHousekeeping)
containerManager, err := manager.New(memoryStorage, sysFs, *maxHousekeepingInterval, *allowDynamicHousekeeping, ignoreMetrics.MetricSet)
if err != nil {
glog.Fatalf("Failed to create a Container Manager: %s", err)
}
Expand Down
29 changes: 29 additions & 0 deletions cadvisor_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Copyright 2014 Google Inc. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package main

import (
"flag"
"testing"

"github.com/google/cadvisor/container"
"github.com/stretchr/testify/assert"
)

func TestTcpMetricsAreDisabledByDefault(t *testing.T) {
assert.True(t, ignoreMetrics.Has(container.NetworkTcpUsageMetrics))
Copy link
Contributor

Choose a reason for hiding this comment

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

This test isn't really doing anything anymore, since the ignoreMetrics flag is never actually added. I think the best way to solve this is by moving the flag declaration to an init() function, or an addFlags() function that you can call here.

Also, consider adding a flag.Parse() and verifying that the assertion still holds.

flag.Parse()
assert.True(t, ignoreMetrics.Has(container.NetworkTcpUsageMetrics))
}
40 changes: 2 additions & 38 deletions manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,42 +50,6 @@ var eventStorageAgeLimit = flag.String("event_storage_age_limit", "default=24h",
var eventStorageEventLimit = flag.String("event_storage_event_limit", "default=100000", "Max number of events to store (per type). Value is a comma separated list of key values, where the keys are event types (e.g.: creation, oom) or \"default\" and the value is an integer. Default is applied to all non-specified event types")
var applicationMetricsCountLimit = flag.Int("application_metrics_count_limit", 100, "Max number of application metrics to store (per container)")

var (
// Metrics to be ignored.
ignoreMetrics metricSetValue = metricSetValue{container.MetricSet{}}
// List of metrics that can be ignored.
ignoreWhitelist = container.MetricSet{
container.DiskUsageMetrics: struct{}{},
container.NetworkUsageMetrics: struct{}{},
container.NetworkTcpUsageMetrics: struct{}{},
}
)

func init() {
flag.Var(&ignoreMetrics, "disable_metrics", "comma-separated list of metrics to be disabled. Options are `disk`, `network`, `tcp`. Note: tcp is disabled by default due to high CPU usage.")
// Tcp metrics are ignored by default.
flag.Set("disable_metrics", "tcp")
}

type metricSetValue struct {
container.MetricSet
}

func (ml *metricSetValue) String() string {
return fmt.Sprint(*ml)
}

func (ml *metricSetValue) Set(value string) error {
for _, metric := range strings.Split(value, ",") {
if ignoreWhitelist.Has(container.MetricKind(metric)) {
(*ml).Add(container.MetricKind(metric))
} else {
return fmt.Errorf("unsupported metric %q specified in disable_metrics", metric)
}
}
return nil
}

// The Manager interface defines operations for starting a manager and getting
// container and machine information.
type Manager interface {
Expand Down Expand Up @@ -155,7 +119,7 @@ type Manager interface {
}

// New takes a memory storage and returns a new manager.
func New(memoryCache *memory.InMemoryCache, sysfs sysfs.SysFs, maxHousekeepingInterval time.Duration, allowDynamicHousekeeping bool) (Manager, error) {
func New(memoryCache *memory.InMemoryCache, sysfs sysfs.SysFs, maxHousekeepingInterval time.Duration, allowDynamicHousekeeping bool, ignoreMetricsSet container.MetricSet) (Manager, error) {
if memoryCache == nil {
return nil, fmt.Errorf("manager requires memory storage")
}
Expand Down Expand Up @@ -194,7 +158,7 @@ func New(memoryCache *memory.InMemoryCache, sysfs sysfs.SysFs, maxHousekeepingIn
startupTime: time.Now(),
maxHousekeepingInterval: maxHousekeepingInterval,
allowDynamicHousekeeping: allowDynamicHousekeeping,
ignoreMetrics: ignoreMetrics.MetricSet,
ignoreMetrics: ignoreMetricsSet,
}

machineInfo, err := getMachineInfo(sysfs, fsInfo, inHostNamespace)
Expand Down
7 changes: 1 addition & 6 deletions manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
info "github.com/google/cadvisor/info/v1"
itest "github.com/google/cadvisor/info/v1/test"
"github.com/google/cadvisor/utils/sysfs/fakesysfs"
"github.com/stretchr/testify/assert"
)

// TODO(vmarmol): Refactor these tests.
Expand Down Expand Up @@ -206,12 +205,8 @@ func TestDockerContainersInfo(t *testing.T) {
}

func TestNewNilManager(t *testing.T) {
_, err := New(nil, nil, 60*time.Second, true)
_, err := New(nil, nil, 60*time.Second, true, container.MetricSet{})
if err == nil {
t.Fatalf("Expected nil manager to return error")
}
}

func TestTcpMetricsAreDisabledByDefault(t *testing.T) {
assert.True(t, ignoreMetrics.Has(container.NetworkTcpUsageMetrics))
}