From b05f4fced305800b32641ae84e3bed5f1794fa7d Mon Sep 17 00:00:00 2001 From: Josef Johansson Date: Tue, 29 Nov 2022 21:49:10 +0100 Subject: [PATCH 1/4] Remove hardwired '127.0.0.1' values in default addresses (#7784) While running all-in-one Loki instance, it's not possible to listen on anything other than IPv4 since compactor_address is then set to http://127.0.0.1. This enables listening on only IPv6. Also if frontend_address, scheduler_ring and scheduler_address is not set, the default value is to connect to '127.0.0.1:'. Which means that grpc_listen_address is ignored. **What this PR does / why we need it**: It's not possible to use compactor while using an all-in-one instance and listening to IPv6. Let's not ignore grpc_listen_address to not confuse users. **Special notes for your reviewer**: As noted in the first commit, the default behavior is changed from defaulting to `127.0.0.1` to `localhost`. Not sure how major that change is though. My /etc/hosts says ``` $ grep localhost /etc/hosts 127.0.0.1 localhost ``` Signed-off-by: Josef Johansson --- CHANGELOG.md | 2 ++ pkg/loki/modules.go | 7 ++++++- pkg/querier/worker_service.go | 7 ++++++- 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92bbd7ea02d7..8b9f6a13f81b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ * [7708](https://github.com/grafana/loki/pull/7708) **DylanGuedes**: Fix multitenant querying. +* [7784](https://github.com/grafana/loki/pull/7784) **isodude**: Fix default values of connect addresses for compactor and querier workers to work with IPv6. + ##### Changes #### Promtail diff --git a/pkg/loki/modules.go b/pkg/loki/modules.go index 1ae8acf9e30a..5a9eb2bec442 100644 --- a/pkg/loki/modules.go +++ b/pkg/loki/modules.go @@ -339,6 +339,7 @@ func (t *Loki) initQuerier() (services.Service, error) { querierWorkerServiceConfig := querier.WorkerServiceConfig{ AllEnabled: t.Cfg.isModuleEnabled(All), ReadEnabled: t.Cfg.isModuleEnabled(Read), + GrpcListenAddress: t.Cfg.Server.GRPCListenAddress, GrpcListenPort: t.Cfg.Server.GRPCListenPort, QuerierMaxConcurrent: t.Cfg.Querier.MaxConcurrent, QuerierWorkerConfig: &t.Cfg.Worker, @@ -703,7 +704,11 @@ func (t *Loki) supportIndexDeleteRequest() bool { func (t *Loki) compactorAddress() (string, error) { if t.Cfg.isModuleEnabled(All) || t.Cfg.isModuleEnabled(Read) { // In single binary or read modes, this module depends on Server - return fmt.Sprintf("http://127.0.0.1:%d", t.Cfg.Server.HTTPListenPort), nil + proto := "http" + if len(t.Cfg.Server.HTTPTLSConfig.TLSCertPath) > 0 && len(t.Cfg.Server.HTTPTLSConfig.TLSKeyPath) > 0 { + proto = "https" + } + return fmt.Sprintf("%s://%s:%d", proto, t.Cfg.Server.HTTPListenAddress, t.Cfg.Server.HTTPListenPort), nil } if t.Cfg.Common.CompactorAddress == "" { diff --git a/pkg/querier/worker_service.go b/pkg/querier/worker_service.go index 406502b13dfe..b9d06bf83532 100644 --- a/pkg/querier/worker_service.go +++ b/pkg/querier/worker_service.go @@ -23,6 +23,7 @@ import ( type WorkerServiceConfig struct { AllEnabled bool ReadEnabled bool + GrpcListenAddress string GrpcListenPort int QuerierMaxConcurrent int QuerierWorkerConfig *querier_worker.Config @@ -112,7 +113,11 @@ func InitWorkerService( // Since we must be running a querier with either a frontend and/or scheduler at this point, if no scheduler ring, frontend, or scheduler address // is configured, Loki will default to using the frontend on localhost on it's own GRPC listening port. if cfg.SchedulerRing == nil && (*cfg.QuerierWorkerConfig).FrontendAddress == "" && (*cfg.QuerierWorkerConfig).SchedulerAddress == "" { - address := fmt.Sprintf("127.0.0.1:%d", cfg.GrpcListenPort) + listenAddress := "127.0.0.1" + if cfg.GrpcListenAddress != "" { + listenAddress = cfg.GrpcListenAddress + } + address := fmt.Sprintf("%s:%d", listenAddress, cfg.GrpcListenPort) level.Warn(util_log.Logger).Log( "msg", "Worker address is empty, attempting automatic worker configuration. If queries are unresponsive consider configuring the worker explicitly.", "address", address) From 6e217c6aa18e5cbdf4cb6d71098d0f2d91afe29b Mon Sep 17 00:00:00 2001 From: Robert Jacob Date: Wed, 30 Nov 2022 00:27:01 +0100 Subject: [PATCH 2/4] operator: Fix alert unit tests (#7812) **What this PR does / why we need it**: Fixes the Prometheus alert tests broken by #7809 **Checklist** - [x] Reviewed the `CONTRIBUTING.md` guide - [x] Tests updated --- .../internal/manifests/internal/alerts/testdata/test.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/operator/internal/manifests/internal/alerts/testdata/test.yaml b/operator/internal/manifests/internal/alerts/testdata/test.yaml index d5f800bac247..a4d8bec8a6a4 100644 --- a/operator/internal/manifests/internal/alerts/testdata/test.yaml +++ b/operator/internal/manifests/internal/alerts/testdata/test.yaml @@ -55,10 +55,10 @@ tests: values: '0+100x20' - series: 'loki_logql_querystats_latency_seconds_bucket{namespace="my-ns", job="querier", route="my-route", le="1"}' + values: '0x20' + - series: 'loki_logql_querystats_latency_seconds_bucket{namespace="my-ns", job="querier", route="my-route", le="30"}' values: '0+10x20' - - series: 'loki_logql_querystats_latency_seconds_bucket{namespace="my-ns", job="querier", route="my-route", le="5"}' - values: '0+50x20' - - series: 'loki_logql_querystats_latency_seconds_bucket{namespace="my-ns", job="querier", route="my-route", le="10"}' + - series: 'loki_logql_querystats_latency_seconds_bucket{namespace="my-ns", job="querier", route="my-route", le="60"}' values: '0+100x20' - series: 'loki_logql_querystats_latency_seconds_bucket{namespace="my-ns", job="querier", route="my-route", le="+Inf"}' values: '0+100x20' @@ -119,7 +119,7 @@ tests: severity: critical exp_annotations: summary: "The 99th percentile is experiencing high latency (higher than 1 second)." - message: "ingester my-route is experiencing 990.00s 99th percentile latency." + message: "ingester my-route is experiencing 9.90s 99th percentile latency." runbook_url: "[[ .RunbookURL ]]#Loki-Request-Latency" - eval_time: 16m alertname: LokiTenantRateLimit From 30ddd771501fa0f2661ae7b32bbc752ea71a5cd2 Mon Sep 17 00:00:00 2001 From: Kaviraj Kanagaraj Date: Wed, 30 Nov 2022 09:50:51 +0100 Subject: [PATCH 3/4] loki-canary: respect `useTLS` flag when `push` mode is enabled. (#7701) **What this PR does / why we need it**: Currently `useTLS` flag is ignored on the push writer. This PR consider that flag and adjust the schema (http -> https) when that flag is enabled. **Which issue(s) this PR fixes**: Fixes #7692 **Special notes for your reviewer**: **Checklist** --- cmd/loki-canary/main.go | 1 + pkg/canary/writer/push.go | 5 +++++ pkg/canary/writer/push_test.go | 7 +++---- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/cmd/loki-canary/main.go b/cmd/loki-canary/main.go index 67903e89516b..a54a335be091 100644 --- a/cmd/loki-canary/main.go +++ b/cmd/loki-canary/main.go @@ -157,6 +157,7 @@ func main() { config.DefaultHTTPClientConfig, *lName, *lVal, *sName, *sValue, + *useTLS, tlsConfig, *caFile, *user, *pass, diff --git a/pkg/canary/writer/push.go b/pkg/canary/writer/push.go index 73c40674436e..6c3ebbc6ac9c 100644 --- a/pkg/canary/writer/push.go +++ b/pkg/canary/writer/push.go @@ -63,6 +63,7 @@ func NewPush( cfg config.HTTPClientConfig, labelName, labelValue string, streamName, streamValue string, + useTLS bool, tlsCfg *tls.Config, caFile string, username, password string, @@ -90,6 +91,10 @@ func NewPush( scheme = "https" } + if useTLS { + scheme = "https" + } + u := url.URL{ Scheme: scheme, Host: lokiAddr, diff --git a/pkg/canary/writer/push_test.go b/pkg/canary/writer/push_test.go index ad98484982ea..0351a2072379 100644 --- a/pkg/canary/writer/push_test.go +++ b/pkg/canary/writer/push_test.go @@ -42,7 +42,7 @@ func Test_Push(t *testing.T) { defer mock.Close() // without TLS - push, err := NewPush(mock.Listener.Addr().String(), "test1", 2*time.Second, config.DefaultHTTPClientConfig, "name", "loki-canary", "stream", "stdout", nil, "", "", "", &backoff, log.NewNopLogger()) + push, err := NewPush(mock.Listener.Addr().String(), "test1", 2*time.Second, config.DefaultHTTPClientConfig, "name", "loki-canary", "stream", "stdout", false, nil, "", "", "", &backoff, log.NewNopLogger()) require.NoError(t, err) ts, payload := testPayload() n, err := push.Write([]byte(payload)) @@ -52,7 +52,7 @@ func Test_Push(t *testing.T) { assertResponse(t, resp, false, labelSet("name", "loki-canary", "stream", "stdout"), ts, payload) // with basic Auth - push, err = NewPush(mock.Listener.Addr().String(), "test1", 2*time.Second, config.DefaultHTTPClientConfig, "name", "loki-canary", "stream", "stdout", nil, "", testUsername, testPassword, &backoff, log.NewNopLogger()) + push, err = NewPush(mock.Listener.Addr().String(), "test1", 2*time.Second, config.DefaultHTTPClientConfig, "name", "loki-canary", "stream", "stdout", false, nil, "", testUsername, testPassword, &backoff, log.NewNopLogger()) require.NoError(t, err) ts, payload = testPayload() n, err = push.Write([]byte(payload)) @@ -62,7 +62,7 @@ func Test_Push(t *testing.T) { assertResponse(t, resp, true, labelSet("name", "loki-canary", "stream", "stdout"), ts, payload) // with custom labels - push, err = NewPush(mock.Listener.Addr().String(), "test1", 2*time.Second, config.DefaultHTTPClientConfig, "name", "loki-canary", "pod", "abc", nil, "", testUsername, testPassword, &backoff, log.NewNopLogger()) + push, err = NewPush(mock.Listener.Addr().String(), "test1", 2*time.Second, config.DefaultHTTPClientConfig, "name", "loki-canary", "pod", "abc", false, nil, "", testUsername, testPassword, &backoff, log.NewNopLogger()) require.NoError(t, err) ts, payload = testPayload() n, err = push.Write([]byte(payload)) @@ -133,7 +133,6 @@ func createServerHandler(responses chan response) http.HandlerFunc { rw.WriteHeader(500) return } - fmt.Println("decoded", decoded) toks := strings.FieldsFunc(string(decoded), func(r rune) bool { return r == ':' }) From d3615f7e9e7b349c504b0b90416303c9b8c4e60a Mon Sep 17 00:00:00 2001 From: George Tsilias Date: Wed, 30 Nov 2022 11:14:21 +0200 Subject: [PATCH 4/4] promtail: Handle nil error on target Details() call (#7771) **What this PR does / why we need it**: This PR addresses issue [#6930](https://github.com/grafana/loki/issues/6930). Promtail should not panic when the Details() method is called for a target that has no errors. **Which issue(s) this PR fixes**: Fixes #6930 --- CHANGELOG.md | 1 + clients/pkg/promtail/targets/cloudflare/target.go | 6 +++++- clients/pkg/promtail/targets/docker/target.go | 6 +++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b9f6a13f81b..45b2e7b73a5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ * [7602](https://github.com/grafana/loki/pull/7602) **vmax**: Add decolorize stage to Promtail to easily parse colored logs. ##### Fixes +* [7771](https://github.com/grafana/loki/pull/7771) **GeorgeTsilias**: Handle nil error on target Details() call. * [7461](https://github.com/grafana/loki/pull/7461) **MarNicGit**: Promtail: Fix collecting userdata field from Windows Event Log diff --git a/clients/pkg/promtail/targets/cloudflare/target.go b/clients/pkg/promtail/targets/cloudflare/target.go index 2cb4cea97493..d1bc2cd9af4f 100644 --- a/clients/pkg/promtail/targets/cloudflare/target.go +++ b/clients/pkg/promtail/targets/cloudflare/target.go @@ -224,9 +224,13 @@ func (t *Target) Ready() bool { func (t *Target) Details() interface{} { fields, _ := Fields(FieldsType(t.config.FieldsType)) + var errMsg string + if t.err != nil { + errMsg = t.err.Error() + } return map[string]string{ "zone_id": t.config.ZoneID, - "error": t.err.Error(), + "error": errMsg, "position": t.positions.GetString(positions.CursorKey(t.config.ZoneID)), "last_timestamp": t.to.String(), "fields": strings.Join(fields, ","), diff --git a/clients/pkg/promtail/targets/docker/target.go b/clients/pkg/promtail/targets/docker/target.go index 71837e8b7dd2..329827e5b61c 100644 --- a/clients/pkg/promtail/targets/docker/target.go +++ b/clients/pkg/promtail/targets/docker/target.go @@ -251,9 +251,13 @@ func (t *Target) Labels() model.LabelSet { // Details returns target-specific details. func (t *Target) Details() interface{} { + var errMsg string + if t.err != nil { + errMsg = t.err.Error() + } return map[string]string{ "id": t.containerName, - "error": t.err.Error(), + "error": errMsg, "position": t.positions.GetString(positions.CursorKey(t.containerName)), "running": strconv.FormatBool(t.running.Load()), }