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

@@ -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,

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()

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
 	}

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]
+}

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)
+	}
+}