From 142d3564e613557ff5deaf11d48c8e7fc6db8580 Mon Sep 17 00:00:00 2001 From: zry98 Date: Mon, 8 Jan 2024 18:07:14 +0100 Subject: [PATCH] Promtail: show a clearer reason in "disable watchConfig" log message when server is disabled (#11420) **What this PR does / why we need it**: This PR improves the clarity of log message when the Promtail server is disabled (if `server.disable` is true in config file, or flag `-server.disable` is used). Previously, the reason "promtailServer cast fail" in log message (as below) was ambiguous, leading to confusion about potential error in config file. ``` level=warn ts=2023-12-09T15:40:24.064376445Z caller=promtail.go:260 msg="disable watchConfig" reason="promtailServer cast fail" ``` The updated message is more understandable, indicating directly that the Promtail server is disabled. ``` level=warn ts=2023-12-09T15:50:58.723168763Z caller=promtail.go:260 msg="disable watchConfig" reason="Promtail server is disabled" ``` **Which issue(s) this PR fixes**: N/A. **Special notes for your reviewer**: Should we turn down the log level to INFO? **Checklist** - [x] Reviewed the [`CONTRIBUTING.md`](https://github.com/grafana/loki/blob/main/CONTRIBUTING.md) guide (**required**) - [ ] Documentation added - [ ] Tests updated - [x] `CHANGELOG.md` updated - [ ] If the change is worth mentioning in the release notes, add `add-to-release-notes` label - [ ] 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](https://github.com/grafana/loki/commit/d10549e3ece02120974929894ee333d07755d213) - [ ] 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](https://github.com/grafana/loki/pull/10840/commits/0d4416a4b03739583349934b96f272fb4f685d15) --- CHANGELOG.md | 1 + clients/pkg/promtail/promtail.go | 36 +++++++++++++++------------ clients/pkg/promtail/server/server.go | 10 ++++---- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd48775e1927..2ff7b66890f4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,7 @@ * [10677](https://github.com/grafana/loki/pull/10677) **chaudum** Remove deprecated `stream_lag_labels` setting from both the `options` and `client` configuration sections. * [10689](https://github.com/grafana/loki/pull/10689) **dylanguedes**: Ingester: Make jitter to be 20% of flush check period instead of 1%. +* [11420](https://github.com/grafana/loki/pull/11420) **zry98**: Show a clearer reason in "disable watchConfig" log message when server is disabled. ##### Fixes diff --git a/clients/pkg/promtail/promtail.go b/clients/pkg/promtail/promtail.go index 1586e2d97115..1ef3368a697e 100644 --- a/clients/pkg/promtail/promtail.go +++ b/clients/pkg/promtail/promtail.go @@ -255,25 +255,29 @@ func (p *Promtail) watchConfig() { level.Warn(p.logger).Log("msg", "disable watchConfig", "reason", "Promtail newConfig func is Empty") return } - promtailServer, ok := p.server.(*server.PromtailServer) - if !ok { - level.Warn(p.logger).Log("msg", "disable watchConfig", "reason", "promtailServer cast fail") + switch srv := p.server.(type) { + case *server.NoopServer: + level.Warn(p.logger).Log("msg", "disable watchConfig", "reason", "Promtail server is disabled") return - } - level.Warn(p.logger).Log("msg", "enable watchConfig") - hup := make(chan os.Signal, 1) - signal.Notify(hup, syscall.SIGHUP) - for { - select { - case <-hup: - _ = p.reload() - case rc := <-promtailServer.Reload(): - if err := p.reload(); err != nil { - rc <- err - } else { - rc <- nil + case *server.PromtailServer: + level.Warn(p.logger).Log("msg", "enable watchConfig") + hup := make(chan os.Signal, 1) + signal.Notify(hup, syscall.SIGHUP) + for { + select { + case <-hup: + _ = p.reload() + case rc := <-srv.Reload(): + if err := p.reload(); err != nil { + rc <- err + } else { + rc <- nil + } } } + default: + level.Warn(p.logger).Log("msg", "disable watchConfig", "reason", "Unknown Promtail server type") + return } } diff --git a/clients/pkg/promtail/server/server.go b/clients/pkg/promtail/server/server.go index 4eb61361d10a..1b47247630e0 100644 --- a/clients/pkg/promtail/server/server.go +++ b/clients/pkg/promtail/server/server.go @@ -321,25 +321,25 @@ func computeExternalURL(u string, port int) (*url.URL, error) { return eu, nil } -type noopServer struct { +type NoopServer struct { log log.Logger sigs chan os.Signal } -func newNoopServer(log log.Logger) *noopServer { - return &noopServer{ +func newNoopServer(log log.Logger) *NoopServer { + return &NoopServer{ log: log, sigs: make(chan os.Signal, 1), } } -func (s *noopServer) Run() error { +func (s *NoopServer) Run() error { signal.Notify(s.sigs, syscall.SIGINT, syscall.SIGTERM) sig := <-s.sigs level.Info(s.log).Log("msg", "received shutdown signal", "sig", sig) return nil } -func (s *noopServer) Shutdown() { +func (s *NoopServer) Shutdown() { s.sigs <- syscall.SIGTERM }