From 596537b38c05047d28f8cfc3c805954486714a9f Mon Sep 17 00:00:00 2001 From: Conor Branagan Date: Wed, 16 Aug 2017 19:12:05 -0400 Subject: [PATCH] Parse docker health from ContainerList, remove option Since we already have this data there's no need to make this an option anymore as it doesn't add any additional load. --- agent/main.go | 1 - checks/container_test.go | 1 - config/config.go | 6 ----- util/docker/docker.go | 47 +++++++++++++++++++------------------- util/docker/docker_test.go | 33 ++++++++++++++++++++++---- 5 files changed, 52 insertions(+), 36 deletions(-) diff --git a/agent/main.go b/agent/main.go index f73d2909d..b52542598 100644 --- a/agent/main.go +++ b/agent/main.go @@ -144,7 +144,6 @@ func main() { func initMetadataProviders(cfg *config.AgentConfig) { if err := docker.InitDockerUtil(&docker.Config{ CacheDuration: cfg.ContainerCacheDuration, - CollectHealth: cfg.CollectDockerHealth, CollectNetwork: cfg.CollectDockerNetwork, Whitelist: cfg.ContainerWhitelist, Blacklist: cfg.ContainerBlacklist, diff --git a/checks/container_test.go b/checks/container_test.go index d0a23b0c6..01f52d475 100644 --- a/checks/container_test.go +++ b/checks/container_test.go @@ -74,7 +74,6 @@ func TestContainerChunking(t *testing.T) { func BenchmarkAllContainers(b *testing.B) { docker.InitDockerUtil(&docker.Config{ CacheDuration: 10 * time.Second, - CollectHealth: true, CollectNetwork: true, }) b.ReportAllocs() diff --git a/config/config.go b/config/config.go index cb6ad04b1..30431e7b5 100644 --- a/config/config.go +++ b/config/config.go @@ -53,7 +53,6 @@ type AgentConfig struct { // Docker ContainerBlacklist []string ContainerWhitelist []string - CollectDockerHealth bool CollectDockerNetwork bool ContainerCacheDuration time.Duration @@ -118,7 +117,6 @@ func NewDefaultAgentConfig() *AgentConfig { }, // Docker - CollectDockerHealth: true, CollectDockerNetwork: true, // Kubernetes @@ -249,7 +247,6 @@ func NewAgentConfig(agentConf, legacyConf *File) (*AgentConfig, error) { } // Docker config - cfg.CollectDockerHealth = file.GetBool(ns, "allow_real_time", cfg.CollectDockerHealth) cfg.CollectDockerNetwork = file.GetBool(ns, "collect_docker_network", cfg.CollectDockerNetwork) cfg.ContainerBlacklist = file.GetStrArrayDefault(ns, "container_blacklist", ",", cfg.ContainerBlacklist) cfg.ContainerWhitelist = file.GetStrArrayDefault(ns, "container_whitelist", ",", cfg.ContainerWhitelist) @@ -323,9 +320,6 @@ func mergeEnv(c *AgentConfig) *AgentConfig { } // Docker config - if v := os.Getenv("DD_COLLECT_DOCKER_HEALTH"); v == "false" { - c.CollectDockerHealth = false - } if v := os.Getenv("DD_COLLECT_DOCKER_NETWORK"); v == "false" { c.CollectDockerNetwork = false } diff --git a/util/docker/docker.go b/util/docker/docker.go index 23a59b598..2fc682556 100644 --- a/util/docker/docker.go +++ b/util/docker/docker.go @@ -184,9 +184,6 @@ type Config struct { // containers and cgroups. The actual raw metrics (e.g. MemRSS) will _not_ // be cached but will be re-calculated on all calls to AllContainers. CacheDuration time.Duration - // CollectHealth enables health collection. This requires calling a - // container.Inspect for each container on every called to dockerContainers(). - CollectHealth bool // CollectNetwork enables network stats collection. This requires at least // one call to container.Inspect for new containers and reads from the // procfs for stats. @@ -296,30 +293,14 @@ func (d *dockerUtil) dockerContainers() ([]*Container, error) { } ret := make([]*Container, 0, len(containers)) for _, c := range containers { - var health string - var i types.ContainerJSON - if d.cfg.CollectHealth { - // We could have lost the container between list and inspect so ignore these errors. - i, err = d.cli.ContainerInspect(context.Background(), c.ID) - if err != nil && client.IsErrContainerNotFound(err) { - return nil, err - } - // Healthcheck and status not available until >= 1.12 - if i.State.Health != nil { - health = i.State.Health.Status - } - } - if d.cfg.CollectNetwork { // FIXME: We might need to invalidate this cache if a containers networks are changed live. d.Lock() if _, ok := d.networkMappings[c.ID]; !ok { - if i.ContainerJSONBase == nil { - i, err = d.cli.ContainerInspect(context.Background(), c.ID) - if err != nil && client.IsErrContainerNotFound(err) { - d.Unlock() - return nil, err - } + i, err := d.cli.ContainerInspect(context.Background(), c.ID) + if err != nil && client.IsErrContainerNotFound(err) { + d.Unlock() + return nil, err } d.networkMappings[c.ID] = findDockerNetworks(c.ID, i.State.Pid, c.NetworkSettings) } @@ -334,7 +315,7 @@ func (d *dockerUtil) dockerContainers() ([]*Container, error) { ImageID: c.ImageID, Created: c.Created, State: c.State, - Health: health, + Health: parseContainerHealth(c.Status), } if !d.cfg.filter.IsExcluded(container) { ret = append(ret, container) @@ -638,3 +619,21 @@ func collectNetworkStats(containerID string, pid int, networks []dockerNetwork) } return stat, nil } + +var healthRe = regexp.MustCompile(`\(health: (\w+)\)`) + +// Parse the health out of a container status. The format is either: +// - 'Up 5 seconds (health: starting)' +// - 'Up about an hour' +// +func parseContainerHealth(status string) string { + // Avoid allocations in most cases by just checking for '(' + if strings.IndexByte(status, '(') == -1 { + return "" + } + all := healthRe.FindAllStringSubmatch(status, -1) + if len(all) < 1 || len(all[0]) < 2 { + return "" + } + return all[0][1] +} diff --git a/util/docker/docker_test.go b/util/docker/docker_test.go index ae384c650..d14853a66 100644 --- a/util/docker/docker_test.go +++ b/util/docker/docker_test.go @@ -157,11 +157,9 @@ func detab(str string) string { // Sanity-check that all containers works with different settings. func TestAllContainers(t *testing.T) { - InitDockerUtil(&Config{CollectHealth: true, CollectNetwork: true}) + InitDockerUtil(&Config{CollectNetwork: true}) AllContainers() - InitDockerUtil(&Config{CollectHealth: false, CollectNetwork: true}) - AllContainers() - InitDockerUtil(&Config{CollectHealth: true, CollectNetwork: false}) + InitDockerUtil(&Config{CollectNetwork: false}) AllContainers() } @@ -222,3 +220,30 @@ func TestContainerFilter(t *testing.T) { assert.Equal(tc.expectedIDs, allowed, "case %d", i) } } + +func TestParseContainerHealth(t *testing.T) { + assert := assert.New(t) + for i, tc := range []struct { + input string + expected string + }{ + { + input: "", + expected: "", + }, + { + input: "Up 2 minutes", + expected: "", + }, + { + input: "Up about 1 hour (health: starting)", + expected: "starting", + }, + { + input: "Up 1 minute (health: unhealthy)", + expected: "unhealthy", + }, + } { + assert.Equal(tc.expected, parseContainerHealth(tc.input), "test %d failed", i) + } +}