fix: promtail race fixes #12656

paul1r · 2024-04-17T14:25:24Z

What this PR does / why we need it:
Multiple data race fixes for promtail code

Which issue(s) this PR fixes:
Relates to #8586

Special notes for your reviewer:

Checklist

Reviewed the CONTRIBUTING.md guide (required)
Documentation added
Tests updated
Title matches the required conventional commits format, see here
Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

paul1r · 2024-04-17T14:26:39Z

Before the changes, the Go data race detector found many issues.

After these changes, test results were clean:

go test -count=1 ./...
?   	github.com/grafana/loki/v3/clients/pkg/promtail/api	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/client/fake	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/limit	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/server	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/server/ui	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/journal	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/serverutils	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/target	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/testutils	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/windows	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/windows/win_eventlog	[no test files]
ok  	github.com/grafana/loki/v3/clients/pkg/promtail	21.919s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/client	7.217s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/config	2.298s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/discovery/consulagent	3.287s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/positions	1.421s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/scrapeconfig	3.001s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/azureeventhubs	2.294s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/cloudflare	3.283s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/docker	7.410s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/file	49.711s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/gcplog	4.311s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/gelf	3.331s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/heroku	5.683s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/kafka	4.752s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/lokipush	3.658s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/stdin	1.718s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/syslog	4.350s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/syslog/syslogparser	1.322s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/utils	5.693s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/wal	22.589s
progers@Pauls-MacBook-Pro promtail % go test -count=1 -race ./...
?   	github.com/grafana/loki/v3/clients/pkg/promtail/api	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/client/fake	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/limit	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/server	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/server/ui	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/journal	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/serverutils	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/target	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/testutils	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/windows	[no test files]
?   	github.com/grafana/loki/v3/clients/pkg/promtail/targets/windows/win_eventlog	[no test files]
ok  	github.com/grafana/loki/v3/clients/pkg/promtail	23.632s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/client	8.423s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/config	3.990s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/discovery/consulagent	3.646s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/positions	2.896s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/scrapeconfig	4.948s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/azureeventhubs	6.002s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/cloudflare	3.686s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/docker	9.547s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/file	53.261s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/gcplog	5.865s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/gelf	5.049s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/heroku	7.793s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/kafka	6.362s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/lokipush	5.958s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/stdin	3.850s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/syslog	7.263s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/targets/syslog/syslogparser	3.561s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/utils	8.077s
ok  	github.com/grafana/loki/v3/clients/pkg/promtail/wal	24.555s

cstyan · 2024-04-17T21:57:09Z

clients/pkg/promtail/promtail_wal_test.go

+	// Create a channel for log messages
+	logCh := make(chan string, 100) // Buffered channel to avoid blocking


why was the logging change required for fixing a test race here?

There was a race related to the t essentially. The t.Logf("received request: %s", counts) was in a goroutine and was fighting with t.Logf("started test server at URL %s", testServer.URL)

clients/pkg/promtail/targets/file/filetarget.go

… cleaner code

clients/pkg/promtail/targets/file/filetarget.go

cstyan

awesome @paul1r thanks!

paul1r added 2 commits April 17, 2024 09:07

Data race cleanup for promtail

2743e47

A few more

611ceee

paul1r requested a review from a team as a code owner April 17, 2024 14:25

pull-request-size bot added the size/L label Apr 17, 2024

paul1r changed the title ~~bug: promtail race fixes~~ Apr 17, 2024

Lint

90121c9

cstyan reviewed Apr 17, 2024

View reviewed changes

paul1r and others added 3 commits April 18, 2024 09:55

Rework mutex usage to be contained within filetarget.go, resulting in…

39a6236

… cleaner code

Merge branch 'main' into paul1r/promtail_race_fixes

bf30688

Lint

9d13e3b

cstyan reviewed Apr 18, 2024

View reviewed changes

clients/pkg/promtail/targets/file/filetarget.go Outdated Show resolved Hide resolved

clients/pkg/promtail/targets/file/filetarget.go Outdated Show resolved Hide resolved

paul1r added 2 commits April 18, 2024 15:45

Code review comment

792004a

Add setReader() function

f932a31

cstyan approved these changes Apr 18, 2024

View reviewed changes

cyriltovena merged commit 4e04d07 into main Apr 19, 2024
14 checks passed

cyriltovena deleted the paul1r/promtail_race_fixes branch April 19, 2024 07:38

loki-gh-app bot mentioned this pull request Apr 22, 2024

chore(k199): release 3.1.0 #12717

Open

ptodev mentioned this pull request Apr 25, 2024

fix(promtail): Fix bug with Promtail config reloading getting stuck indefinitely #12795

Merged

7 tasks

loki-gh-app bot mentioned this pull request Apr 29, 2024

chore(k200): release 3.1.0 #12812

Open

This was referenced May 6, 2024

chore(k201): release 3.1.0 #12894

Open

chore(k202): release 3.1.0 #12945

Open

loki-gh-app bot mentioned this pull request May 20, 2024

chore(k203): release 3.1.0 #12988

Open

This was referenced May 27, 2024

chore(k204): release 3.1.0 #13037

Open

chore(k205): release 3.1.0 #13102

Open

This was referenced Jun 10, 2024

chore(k206): release 3.1.0 #13184

Open

chore(k207): release 3.1.0 #13225

Merged

loki-gh-app bot mentioned this pull request Jun 24, 2024

chore(k208): release 3.1.0 #13291

Open

loki-gh-app bot mentioned this pull request Jul 1, 2024

chore(k209): release 3.1.0 #13356

Open

grafanabot mentioned this pull request Jul 2, 2024

chore: [main] chore(k207): release 3.1.0 #13391

Open

This was referenced Jul 3, 2024

chore(release-3.1.x): release 3.0.1 #13402

Open

chore(k210): release 3.1.0 #13435

Closed

chore(k210): release 3.1.0 #13462

Open

RodrigoCMoraes mentioned this pull request Jul 12, 2024

incognia inloco/loki#22

Closed

loki-gh-app bot mentioned this pull request Jul 15, 2024

chore(k211): release 3.1.0 #13521

Open

loki-gh-app bot mentioned this pull request Jul 22, 2024

chore(k212): release 3.1.0 #13595

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: promtail race fixes #12656

fix: promtail race fixes #12656

paul1r commented Apr 17, 2024

paul1r commented Apr 17, 2024

cstyan Apr 17, 2024

paul1r Apr 17, 2024

cstyan left a comment

		// Create a channel for log messages
		logCh := make(chan string, 100) // Buffered channel to avoid blocking

fix: promtail race fixes #12656

fix: promtail race fixes #12656

Conversation

paul1r commented Apr 17, 2024

paul1r commented Apr 17, 2024

cstyan Apr 17, 2024

Choose a reason for hiding this comment

paul1r Apr 17, 2024

Choose a reason for hiding this comment

cstyan left a comment

Choose a reason for hiding this comment