From 812392ec9151fda1dcfa01efd124cdb50111aa62 Mon Sep 17 00:00:00 2001 From: Paschalis Tsilias Date: Fri, 5 Jan 2024 14:22:17 +0200 Subject: [PATCH 1/9] loki.source.docker: filter out duplicate container IDs Signed-off-by: Paschalis Tsilias --- component/loki/source/docker/docker.go | 5 ++++ component/loki/source/docker/docker_test.go | 29 +++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/component/loki/source/docker/docker.go b/component/loki/source/docker/docker.go index 400b1d30b5e6..193f27f1d7a2 100644 --- a/component/loki/source/docker/docker.go +++ b/component/loki/source/docker/docker.go @@ -215,6 +215,7 @@ func (c *Component) Update(args component.Arguments) error { // Convert input targets into targets to give to tailer. targets := make([]*dt.Target, 0, len(newArgs.Targets)) + seenTargets := make(map[string]struct{}, len(newArgs.Targets)) for _, target := range newArgs.Targets { containerID, ok := target[dockerLabelContainerID] @@ -222,6 +223,10 @@ func (c *Component) Update(args component.Arguments) error { level.Debug(c.opts.Logger).Log("msg", "docker target did not include container ID label:"+dockerLabelContainerID) continue } + if _, seen := seenTargets[containerID]; seen { + continue + } + seenTargets[containerID] = struct{}{} var labels = make(model.LabelSet) for k, v := range target { diff --git a/component/loki/source/docker/docker_test.go b/component/loki/source/docker/docker_test.go index 51c2a4568cff..0fa0a9921322 100644 --- a/component/loki/source/docker/docker_test.go +++ b/component/loki/source/docker/docker_test.go @@ -5,9 +5,11 @@ import ( "testing" "time" + "github.com/grafana/agent/component" "github.com/grafana/agent/pkg/flow/componenttest" "github.com/grafana/agent/pkg/util" "github.com/grafana/river" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" ) @@ -33,3 +35,30 @@ func Test(t *testing.T) { require.NoError(t, ctrl.WaitRunning(time.Minute)) } + +func TestDuplicateTargets(t *testing.T) { + // Use host that works on all platforms (including Windows). + var cfg = ` + host = "tcp://127.0.0.1:9375" + targets = [ + {__meta_docker_container_id = "foo", __meta_docker_port_private = "8080"}, + {__meta_docker_container_id = "foo", __meta_docker_port_private = "8080"}, + {__meta_docker_container_id = "bar", __meta_docker_port_private = "8080"}, + ] + forward_to = [] + ` + + var args Arguments + err := river.Unmarshal([]byte(cfg), &args) + require.NoError(t, err) + + cmp, err := New(component.Options{ + ID: "loki.source.docker.test", + Logger: util.TestFlowLogger(t), + Registerer: prometheus.NewRegistry(), + DataPath: t.TempDir(), + }, args) + require.NoError(t, err) + + require.Len(t, cmp.manager.tasks, 2) +} From fd8652376454146119922953933f66c02603958d Mon Sep 17 00:00:00 2001 From: Paschalis Tsilias Date: Fri, 5 Jan 2024 16:51:54 +0200 Subject: [PATCH 2/9] Add CHANGELOG entry and docs Signed-off-by: Paschalis Tsilias --- CHANGELOG.md | 4 ++++ docs/sources/flow/reference/components/loki.source.docker.md | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d008e4ab023..766f57c13549 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,8 +81,12 @@ Main (unreleased) - Add `max_cache_size` to `prometheus.relabel` to allow configurability instead of hard coded 100,000. (@mattdurham) - Add support for `http_sd_config` within a `scrape_config` for prometheus to flow config conversion. (@erikbaranowski) + - `discovery.lightsail` now supports additional parameters for configuring HTTP client settings. (@ptodev) +- `loki.source.docker` now deduplicates targets which report the same container + ID. (@tpaschalis) + ### Bugfixes - Update `pyroscope.ebpf` to fix a logical bug causing to profile to many kthreads instead of regular processes https://github.com/grafana/pyroscope/pull/2778 (@korniltsev) diff --git a/docs/sources/flow/reference/components/loki.source.docker.md b/docs/sources/flow/reference/components/loki.source.docker.md index cbf77163d646..7787294911f5 100644 --- a/docs/sources/flow/reference/components/loki.source.docker.md +++ b/docs/sources/flow/reference/components/loki.source.docker.md @@ -131,6 +131,11 @@ fully qualified name) to store its _positions file_. The positions file stores the read offsets so that if there is a component or Agent restart, `loki.source.docker` can pick up tailing from the same spot. +In case the targets Argument contains multiple targets with the same container +ID, for example as a result of `discovery.docker` picking up multiple exposed +ports or networks, `loki.source.docker` will deduplicate these, keeping only +the first of each container ID instances. + ## Example This example collects log entries from the files specified in the `targets` From 8ff2a931478345666dd2f08d4fbbf7aae0892360 Mon Sep 17 00:00:00 2001 From: Paschalis Tsilias Date: Fri, 5 Jan 2024 17:02:51 +0200 Subject: [PATCH 3/9] Amend docs Signed-off-by: Paschalis Tsilias --- .../sources/flow/reference/components/loki.source.docker.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/sources/flow/reference/components/loki.source.docker.md b/docs/sources/flow/reference/components/loki.source.docker.md index 7787294911f5..59af866b8982 100644 --- a/docs/sources/flow/reference/components/loki.source.docker.md +++ b/docs/sources/flow/reference/components/loki.source.docker.md @@ -131,9 +131,9 @@ fully qualified name) to store its _positions file_. The positions file stores the read offsets so that if there is a component or Agent restart, `loki.source.docker` can pick up tailing from the same spot. -In case the targets Argument contains multiple targets with the same container -ID, for example as a result of `discovery.docker` picking up multiple exposed -ports or networks, `loki.source.docker` will deduplicate these, keeping only +In case the targets argument contains multiple entries with the same container +ID (for example as a result of `discovery.docker` picking up multiple exposed +ports or networks) `loki.source.docker` will deduplicate them, and only keep the first of each container ID instances. ## Example From 7348a5c0177938e8fb14a09a0183c38c01ea5a9d Mon Sep 17 00:00:00 2001 From: Paschalis Tsilias Date: Fri, 5 Jan 2024 18:09:41 +0200 Subject: [PATCH 4/9] Test Signed-off-by: Paschalis Tsilias --- component/loki/source/docker/docker_test.go | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/component/loki/source/docker/docker_test.go b/component/loki/source/docker/docker_test.go index 0fa0a9921322..315f4b7cf522 100644 --- a/component/loki/source/docker/docker_test.go +++ b/component/loki/source/docker/docker_test.go @@ -1,3 +1,5 @@ +//go:build !race + package docker import ( @@ -39,11 +41,10 @@ func Test(t *testing.T) { func TestDuplicateTargets(t *testing.T) { // Use host that works on all platforms (including Windows). var cfg = ` - host = "tcp://127.0.0.1:9375" + host = "tcp://127.0.0.1:9376" targets = [ {__meta_docker_container_id = "foo", __meta_docker_port_private = "8080"}, {__meta_docker_container_id = "foo", __meta_docker_port_private = "8080"}, - {__meta_docker_container_id = "bar", __meta_docker_port_private = "8080"}, ] forward_to = [] ` @@ -52,6 +53,16 @@ func TestDuplicateTargets(t *testing.T) { err := river.Unmarshal([]byte(cfg), &args) require.NoError(t, err) + ctrl, err := componenttest.NewControllerFromID(util.TestLogger(t), "loki.source.docker") + require.NoError(t, err) + + go func() { + err := ctrl.Run(context.Background(), args) + require.NoError(t, err) + }() + + require.NoError(t, ctrl.WaitRunning(time.Minute)) + cmp, err := New(component.Options{ ID: "loki.source.docker.test", Logger: util.TestFlowLogger(t), @@ -60,5 +71,5 @@ func TestDuplicateTargets(t *testing.T) { }, args) require.NoError(t, err) - require.Len(t, cmp.manager.tasks, 2) + require.Len(t, cmp.manager.tasks, 1) } From ea96e30051119969f7e807bc1648bd2ce54dd82c Mon Sep 17 00:00:00 2001 From: Paschalis Tsilias Date: Fri, 5 Jan 2024 18:36:01 +0200 Subject: [PATCH 5/9] Add package to Makefile Signed-off-by: Paschalis Tsilias --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 024a624d2223..5b7d4f424759 100644 --- a/Makefile +++ b/Makefile @@ -174,7 +174,7 @@ lint: agentlint # more without -race for packages that have known race detection issues. test: $(GO_ENV) go test $(GO_FLAGS) -race $(shell go list ./... | grep -v /integration-tests/) - $(GO_ENV) go test $(GO_FLAGS) ./pkg/integrations/node_exporter ./pkg/logs ./pkg/operator ./pkg/util/k8s ./component/otelcol/processor/tail_sampling ./component/loki/source/file + $(GO_ENV) go test $(GO_FLAGS) ./pkg/integrations/node_exporter ./pkg/logs ./pkg/operator ./pkg/util/k8s ./component/otelcol/processor/tail_sampling ./component/loki/source/file ./component/loki/source/docker test-packages: docker pull $(BUILD_IMAGE) From a818995e0ceda7c619d5a869816099f7c313395a Mon Sep 17 00:00:00 2001 From: Paschalis Tsilias Date: Mon, 8 Jan 2024 12:12:22 +0200 Subject: [PATCH 6/9] Expand docs Signed-off-by: Paschalis Tsilias --- docs/sources/flow/reference/components/loki.source.docker.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/sources/flow/reference/components/loki.source.docker.md b/docs/sources/flow/reference/components/loki.source.docker.md index 59af866b8982..1c2175e6b555 100644 --- a/docs/sources/flow/reference/components/loki.source.docker.md +++ b/docs/sources/flow/reference/components/loki.source.docker.md @@ -134,7 +134,10 @@ stores the read offsets so that if there is a component or Agent restart, In case the targets argument contains multiple entries with the same container ID (for example as a result of `discovery.docker` picking up multiple exposed ports or networks) `loki.source.docker` will deduplicate them, and only keep -the first of each container ID instances. +the first of each container ID instances, based on the +`__meta_docker_container_id` label. As such, the Docker daemon will be queried +for each container ID only once, and only one target will be available in the +component's debug info. ## Example From 694249efa43e02ffe0c7dd5f03d7109c8e827b34 Mon Sep 17 00:00:00 2001 From: Paschalis Tsilias Date: Mon, 8 Jan 2024 12:12:59 +0200 Subject: [PATCH 7/9] Change port of second target Signed-off-by: Paschalis Tsilias --- component/loki/source/docker/docker_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/loki/source/docker/docker_test.go b/component/loki/source/docker/docker_test.go index 315f4b7cf522..c0a6eb0a5a9e 100644 --- a/component/loki/source/docker/docker_test.go +++ b/component/loki/source/docker/docker_test.go @@ -44,7 +44,7 @@ func TestDuplicateTargets(t *testing.T) { host = "tcp://127.0.0.1:9376" targets = [ {__meta_docker_container_id = "foo", __meta_docker_port_private = "8080"}, - {__meta_docker_container_id = "foo", __meta_docker_port_private = "8080"}, + {__meta_docker_container_id = "foo", __meta_docker_port_private = "8081}, ] forward_to = [] ` From dd776edb0ef4fa7379388062f488f63d4a361752 Mon Sep 17 00:00:00 2001 From: Paschalis Tsilias Date: Mon, 8 Jan 2024 14:14:19 +0200 Subject: [PATCH 8/9] Terminate string Signed-off-by: Paschalis Tsilias --- component/loki/source/docker/docker_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/loki/source/docker/docker_test.go b/component/loki/source/docker/docker_test.go index c0a6eb0a5a9e..c4b99c47388c 100644 --- a/component/loki/source/docker/docker_test.go +++ b/component/loki/source/docker/docker_test.go @@ -44,7 +44,7 @@ func TestDuplicateTargets(t *testing.T) { host = "tcp://127.0.0.1:9376" targets = [ {__meta_docker_container_id = "foo", __meta_docker_port_private = "8080"}, - {__meta_docker_container_id = "foo", __meta_docker_port_private = "8081}, + {__meta_docker_container_id = "foo", __meta_docker_port_private = "8081"}, ] forward_to = [] ` From 77df5fbe9e038dd61dd830b776ada4aed696c422 Mon Sep 17 00:00:00 2001 From: Paschalis Tsilias Date: Wed, 10 Jan 2024 02:57:33 +0200 Subject: [PATCH 9/9] Apply suggestions from code review Co-authored-by: Clayton Cornell <131809008+clayton-cornell@users.noreply.github.com> --- .../sources/flow/reference/components/loki.source.docker.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/sources/flow/reference/components/loki.source.docker.md b/docs/sources/flow/reference/components/loki.source.docker.md index 1c2175e6b555..3dd327f70a80 100644 --- a/docs/sources/flow/reference/components/loki.source.docker.md +++ b/docs/sources/flow/reference/components/loki.source.docker.md @@ -131,11 +131,11 @@ fully qualified name) to store its _positions file_. The positions file stores the read offsets so that if there is a component or Agent restart, `loki.source.docker` can pick up tailing from the same spot. -In case the targets argument contains multiple entries with the same container +If the target's argument contains multiple entries with the same container ID (for example as a result of `discovery.docker` picking up multiple exposed -ports or networks) `loki.source.docker` will deduplicate them, and only keep +ports or networks), `loki.source.docker` will deduplicate them, and only keep the first of each container ID instances, based on the -`__meta_docker_container_id` label. As such, the Docker daemon will be queried +`__meta_docker_container_id` label. As such, the Docker daemon is queried for each container ID only once, and only one target will be available in the component's debug info.