From 0da94a1b3c21b92e3e50f91c3de97d8175f5a77d Mon Sep 17 00:00:00 2001 From: Greg Date: Thu, 12 Jul 2018 19:41:49 -0600 Subject: [PATCH] Fix incorrect container name gathered in docker input (#4391) --- plugins/inputs/docker/docker.go | 27 ++-- plugins/inputs/docker/docker_test.go | 173 ++++++++--------------- plugins/inputs/docker/docker_testdata.go | 34 ++++- 3 files changed, 107 insertions(+), 127 deletions(-) diff --git a/plugins/inputs/docker/docker.go b/plugins/inputs/docker/docker.go index aa1de7479..5d4cb0de8 100644 --- a/plugins/inputs/docker/docker.go +++ b/plugins/inputs/docker/docker.go @@ -365,10 +365,18 @@ func (d *Docker) gatherContainer( ) error { var v *types.StatsJSON // Parse container name - cname := "unknown" - if len(container.Names) > 0 { - // Not sure what to do with other names, just take the first. - cname = strings.TrimPrefix(container.Names[0], "/") + var cname string + for _, name := range container.Names { + trimmedName := strings.TrimPrefix(name, "/") + match := d.containerFilter.Match(trimmedName) + if match { + cname = trimmedName + break + } + } + + if cname == "" { + return nil } // the image name sometimes has a version part, or a private repo @@ -391,10 +399,6 @@ func (d *Docker) gatherContainer( "container_version": imageVersion, } - if !d.containerFilter.Match(cname) { - return nil - } - ctx, cancel := context.WithTimeout(context.Background(), d.Timeout.Duration) defer cancel() r, err := d.client.ContainerStats(ctx, container.ID, false) @@ -411,6 +415,9 @@ func (d *Docker) gatherContainer( } daemonOSType := r.OSType + // use common (printed at `docker ps`) name for container + tags["container_name"] = strings.TrimPrefix(v.Name, "/") + // Add labels to tags for k, label := range container.Labels { if d.labelFilter.Match(k) { @@ -461,12 +468,12 @@ func (d *Docker) gatherContainer( acc.AddFields("docker_container_health", healthfields, tags, time.Now()) } - gatherContainerStats(v, acc, tags, container.ID, d.PerDevice, d.Total, daemonOSType) + parseContainerStats(v, acc, tags, container.ID, d.PerDevice, d.Total, daemonOSType) return nil } -func gatherContainerStats( +func parseContainerStats( stat *types.StatsJSON, acc telegraf.Accumulator, tags map[string]string, diff --git a/plugins/inputs/docker/docker_test.go b/plugins/inputs/docker/docker_test.go index d51c61c00..b97c34b6b 100644 --- a/plugins/inputs/docker/docker_test.go +++ b/plugins/inputs/docker/docker_test.go @@ -77,8 +77,8 @@ var baseClient = MockClient{ ContainerListF: func(context.Context, types.ContainerListOptions) ([]types.Container, error) { return containerList, nil }, - ContainerStatsF: func(context.Context, string, bool) (types.ContainerStats, error) { - return containerStats(), nil + ContainerStatsF: func(c context.Context, s string, b bool) (types.ContainerStats, error) { + return containerStats(s), nil }, ContainerInspectF: func(context.Context, string) (types.ContainerJSON, error) { return containerInspect, nil @@ -107,7 +107,7 @@ func TestDockerGatherContainerStats(t *testing.T) { "container_image": "redis/image", } - gatherContainerStats(stats, &acc, tags, "123456789", true, true, "linux") + parseContainerStats(stats, &acc, tags, "123456789", true, true, "linux") // test docker_container_net measurement netfields := map[string]interface{}{ @@ -290,11 +290,9 @@ func TestContainerLabels(t *testing.T) { }{ { name: "Nil filters matches all", - container: types.Container{ - Labels: map[string]string{ - "a": "x", - }, - }, + container: genContainerLabeled(map[string]string{ + "a": "x", + }), include: nil, exclude: nil, expected: map[string]string{ @@ -303,11 +301,9 @@ func TestContainerLabels(t *testing.T) { }, { name: "Empty filters matches all", - container: types.Container{ - Labels: map[string]string{ - "a": "x", - }, - }, + container: genContainerLabeled(map[string]string{ + "a": "x", + }), include: []string{}, exclude: []string{}, expected: map[string]string{ @@ -316,12 +312,10 @@ func TestContainerLabels(t *testing.T) { }, { name: "Must match include", - container: types.Container{ - Labels: map[string]string{ - "a": "x", - "b": "y", - }, - }, + container: genContainerLabeled(map[string]string{ + "a": "x", + "b": "y", + }), include: []string{"a"}, exclude: []string{}, expected: map[string]string{ @@ -330,12 +324,10 @@ func TestContainerLabels(t *testing.T) { }, { name: "Must not match exclude", - container: types.Container{ - Labels: map[string]string{ - "a": "x", - "b": "y", - }, - }, + container: genContainerLabeled(map[string]string{ + "a": "x", + "b": "y", + }), include: []string{}, exclude: []string{"b"}, expected: map[string]string{ @@ -344,13 +336,11 @@ func TestContainerLabels(t *testing.T) { }, { name: "Include Glob", - container: types.Container{ - Labels: map[string]string{ - "aa": "x", - "ab": "y", - "bb": "z", - }, - }, + container: genContainerLabeled(map[string]string{ + "aa": "x", + "ab": "y", + "bb": "z", + }), include: []string{"a*"}, exclude: []string{}, expected: map[string]string{ @@ -360,13 +350,11 @@ func TestContainerLabels(t *testing.T) { }, { name: "Exclude Glob", - container: types.Container{ - Labels: map[string]string{ - "aa": "x", - "ab": "y", - "bb": "z", - }, - }, + container: genContainerLabeled(map[string]string{ + "aa": "x", + "ab": "y", + "bb": "z", + }), include: []string{}, exclude: []string{"a*"}, expected: map[string]string{ @@ -375,13 +363,11 @@ func TestContainerLabels(t *testing.T) { }, { name: "Excluded Includes", - container: types.Container{ - Labels: map[string]string{ - "aa": "x", - "ab": "y", - "bb": "z", - }, - }, + container: genContainerLabeled(map[string]string{ + "aa": "x", + "ab": "y", + "bb": "z", + }), include: []string{"a*"}, exclude: []string{"*b"}, expected: map[string]string{ @@ -425,6 +411,12 @@ func TestContainerLabels(t *testing.T) { } } +func genContainerLabeled(labels map[string]string) types.Container { + c := containerList[0] + c.Labels = labels + return c +} + func TestContainerNames(t *testing.T) { var tests = []struct { name string @@ -434,112 +426,67 @@ func TestContainerNames(t *testing.T) { expected []string }{ { - name: "Nil filters matches all", - containers: [][]string{ - {"/etcd"}, - {"/etcd2"}, - }, + name: "Nil filters matches all", include: nil, exclude: nil, - expected: []string{"etcd", "etcd2"}, + expected: []string{"etcd", "etcd2", "acme", "acme-test", "foo"}, }, { - name: "Empty filters matches all", - containers: [][]string{ - {"/etcd"}, - {"/etcd2"}, - }, + name: "Empty filters matches all", include: []string{}, exclude: []string{}, - expected: []string{"etcd", "etcd2"}, + expected: []string{"etcd", "etcd2", "acme", "acme-test", "foo"}, }, { - name: "Match all containers", - containers: [][]string{ - {"/etcd"}, - {"/etcd2"}, - }, + name: "Match all containers", include: []string{"*"}, exclude: []string{}, - expected: []string{"etcd", "etcd2"}, + expected: []string{"etcd", "etcd2", "acme", "acme-test", "foo"}, }, { - name: "Include prefix match", - containers: [][]string{ - {"/etcd"}, - {"/etcd2"}, - }, + name: "Include prefix match", include: []string{"etc*"}, exclude: []string{}, expected: []string{"etcd", "etcd2"}, }, { - name: "Exact match", - containers: [][]string{ - {"/etcd"}, - {"/etcd2"}, - }, + name: "Exact match", include: []string{"etcd"}, exclude: []string{}, expected: []string{"etcd"}, }, { - name: "Star matches zero length", - containers: [][]string{ - {"/etcd"}, - {"/etcd2"}, - }, + name: "Star matches zero length", include: []string{"etcd2*"}, exclude: []string{}, expected: []string{"etcd2"}, }, { - name: "Exclude matches all", - containers: [][]string{ - {"/etcd"}, - {"/etcd2"}, - }, + name: "Exclude matches all", include: []string{}, exclude: []string{"etc*"}, - expected: []string{}, + expected: []string{"acme", "acme-test", "foo"}, }, { - name: "Exclude single", - containers: [][]string{ - {"/etcd"}, - {"/etcd2"}, - }, + name: "Exclude single", include: []string{}, exclude: []string{"etcd"}, - expected: []string{"etcd2"}, + expected: []string{"etcd2", "acme", "acme-test", "foo"}, }, { - name: "Exclude all", - containers: [][]string{ - {"/etcd"}, - {"/etcd2"}, - }, + name: "Exclude all", include: []string{"*"}, exclude: []string{"*"}, expected: []string{}, }, { - name: "Exclude item matching include", - containers: [][]string{ - {"acme"}, - {"foo"}, - {"acme-test"}, - }, + name: "Exclude item matching include", include: []string{"acme*"}, exclude: []string{"*test*"}, expected: []string{"acme"}, }, { - name: "Exclude item no wildcards", - containers: [][]string{ - {"acme"}, - {"acme-test"}, - }, + name: "Exclude item no wildcards", include: []string{"acme*"}, exclude: []string{"test"}, expected: []string{"acme", "acme-test"}, @@ -552,14 +499,12 @@ func TestContainerNames(t *testing.T) { newClientFunc := func(host string, tlsConfig *tls.Config) (Client, error) { client := baseClient client.ContainerListF = func(context.Context, types.ContainerListOptions) ([]types.Container, error) { - var containers []types.Container - for _, names := range tt.containers { - containers = append(containers, types.Container{ - Names: names, - }) - } - return containers, nil + return containerList, nil } + client.ContainerStatsF = func(c context.Context, s string, b bool) (types.ContainerStats, error) { + return containerStats(s), nil + } + return &client, nil } diff --git a/plugins/inputs/docker/docker_testdata.go b/plugins/inputs/docker/docker_testdata.go index 1168048a2..bb275a1cc 100644 --- a/plugins/inputs/docker/docker_testdata.go +++ b/plugins/inputs/docker/docker_testdata.go @@ -1,6 +1,7 @@ package docker import ( + "fmt" "io/ioutil" "strings" "time" @@ -133,6 +134,18 @@ var containerList = []types.Container{ SizeRw: 0, SizeRootFs: 0, }, + types.Container{ + ID: "e8a713dd90604f5a257b97c15945e047ab60ed5b2c4397c5a6b5bf40e1bd2791", + Names: []string{"/acme"}, + }, + types.Container{ + ID: "9bc6faf9ba8106fae32e8faafd38a1dd6f6d262bec172398cc10bc03c0d6841a", + Names: []string{"/acme-test"}, + }, + types.Container{ + ID: "d4ccced494a1d5fe8ebdb0a86335a0dab069319912221e5838a132ab18a8bc84", + Names: []string{"/foo"}, + }, } var two = uint64(2) @@ -208,10 +221,25 @@ var NodeList = []swarm.Node{ }, } -func containerStats() types.ContainerStats { +func containerStats(s string) types.ContainerStats { var stat types.ContainerStats - jsonStat := ` + var name string + switch s { + case "e2173b9478a6ae55e237d4d74f8bbb753f0817192b5081334dc78476296b7dfb": + name = "etcd" + case "b7dfbb9478a6ae55e237d4d74f8bbb753f0817192b5081334dc78476296e2173": + name = "etcd2" + case "e8a713dd90604f5a257b97c15945e047ab60ed5b2c4397c5a6b5bf40e1bd2791": + name = "/acme" + case "9bc6faf9ba8106fae32e8faafd38a1dd6f6d262bec172398cc10bc03c0d6841a": + name = "/acme-test" + case "d4ccced494a1d5fe8ebdb0a86335a0dab069319912221e5838a132ab18a8bc84": + name = "/foo" + } + + jsonStat := fmt.Sprintf(` { + "name": "%s", "blkio_stats": { "io_service_bytes_recursive": [ { @@ -315,7 +343,7 @@ func containerStats() types.ContainerStats { "throttling_data": {} }, "read": "2016-02-24T11:42:27.472459608-05:00" -}` +}`, name) stat.Body = ioutil.NopCloser(strings.NewReader(jsonStat)) return stat }