Skip to content

Commit

Permalink
Fix incorrect container name gathered in docker input (#4391)
Browse files Browse the repository at this point in the history
  • Loading branch information
@glinton @danielnelson
glinton authored and danielnelson committed Jul 13, 2018
1 parent 8ff63a4 commit 0da94a1
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 127 deletions.
27 changes: 17 additions & 10 deletions plugins/inputs/docker/docker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down
173 changes: 59 additions & 114 deletions plugins/inputs/docker/docker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{}{
Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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{
Expand Down Expand Up @@ -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
Expand All @@ -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"},
Expand All @@ -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
}

Expand Down
Loading

0 comments on commit 0da94a1

Please sign in to comment.