From a4a4dca2b2e64cc70a1a719c5f3dbf7a5c81adba Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Thu, 24 Oct 2024 13:41:26 -0700 Subject: [PATCH 01/13] add a span tag for content type if the header is set (#614) Signed-off-by: Callum Styan --- middleware/http_tracing.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/middleware/http_tracing.go b/middleware/http_tracing.go index d75535ebe..b7dfe2d59 100644 --- a/middleware/http_tracing.go +++ b/middleware/http_tracing.go @@ -38,6 +38,11 @@ func (t Tracer) Wrap(next http.Handler) http.Handler { sp.SetTag("http.user_agent", userAgent) } + // add the content type, useful when query requests are sent as POST + if ct := r.Header.Get("Content-Type"); ct != "" { + sp.SetTag("http.content_type", ct) + } + // add a tag with the client's sourceIPs to the span, if a // SourceIPExtractor is given. if t.SourceIPs != nil { From a6b453a88040c92083d1c733494e3b0f9bfa1746 Mon Sep 17 00:00:00 2001 From: Robert Lankford Date: Tue, 5 Nov 2024 07:46:43 -0800 Subject: [PATCH 02/13] logging middleware to honor logRequestHeaders config (#615) * logging middleware to honor logRequestHeaders config Signed-off-by: Robbie Lankford * update tests and changelog * remove extra log line --------- Signed-off-by: Robbie Lankford --- CHANGELOG.md | 1 + middleware/logging.go | 25 ++++++++++++++++++++----- middleware/logging_test.go | 25 +++++++++++++++++++++---- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9cb11448..d90ec93d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Changelog +* [CHANGE] Log middleware updated to honor `logRequestHeaders` in all logging scenarios. #615 * [CHANGE] Roll back the gRPC dependency to v1.65.0 to allow downstream projects to avoid a performance regression and maybe a bug in v1.66.0. #581 * [CHANGE] Update the gRPC dependency to v1.66.0 and deprecate the `grpc_server_recv_buffer_pools_enabled` option that is no longer supported by it. #580 * [CHANGE] `ring.DoBatchWithOptions` (and `ring.DoBatch`) reports the cancelation cause when the context is canceled instead of `context.Canceled`. diff --git a/middleware/logging.go b/middleware/logging.go index c2306292b..920976b3c 100644 --- a/middleware/logging.go +++ b/middleware/logging.go @@ -94,14 +94,25 @@ func (l Log) Wrap(next http.Handler) http.Handler { if writeErr != nil { if errors.Is(writeErr, context.Canceled) { if l.LogRequestAtInfoLevel { - level.Info(requestLog).Log("msg", dskit_log.LazySprintf("%s %s %s, request cancelled: %s ws: %v; %s", r.Method, uri, time.Since(begin), writeErr, IsWSHandshakeRequest(r), headers)) + if l.LogRequestHeaders && headers != nil { + level.Info(requestLog).Log("msg", dskit_log.LazySprintf("%s %s %s, request cancelled: %s ws: %v; %s", r.Method, uri, time.Since(begin), writeErr, IsWSHandshakeRequest(r), headers)) + } else { + level.Info(requestLog).Log("msg", dskit_log.LazySprintf("%s %s %s, request cancelled: %s ws: %v", r.Method, uri, time.Since(begin), writeErr, IsWSHandshakeRequest(r))) + } } else { - level.Debug(requestLog).Log("msg", dskit_log.LazySprintf("%s %s %s, request cancelled: %s ws: %v; %s", r.Method, uri, time.Since(begin), writeErr, IsWSHandshakeRequest(r), headers)) + if l.LogRequestHeaders && headers != nil { + level.Debug(requestLog).Log("msg", dskit_log.LazySprintf("%s %s %s, request cancelled: %s ws: %v; %s", r.Method, uri, time.Since(begin), writeErr, IsWSHandshakeRequest(r), headers)) + } else { + level.Debug(requestLog).Log("msg", dskit_log.LazySprintf("%s %s %s, request cancelled: %s ws: %v", r.Method, uri, time.Since(begin), writeErr, IsWSHandshakeRequest(r))) + } } } else { - level.Warn(requestLog).Log("msg", dskit_log.LazySprintf("%s %s %s, error: %s ws: %v; %s", r.Method, uri, time.Since(begin), writeErr, IsWSHandshakeRequest(r), headers)) + if l.LogRequestHeaders && headers != nil { + level.Warn(requestLog).Log("msg", dskit_log.LazySprintf("%s %s %s, error: %s ws: %v; %s", r.Method, uri, time.Since(begin), writeErr, IsWSHandshakeRequest(r), headers)) + } else { + level.Warn(requestLog).Log("msg", dskit_log.LazySprintf("%s %s %s, error: %s ws: %v", r.Method, uri, time.Since(begin), writeErr, IsWSHandshakeRequest(r))) + } } - return } @@ -125,7 +136,11 @@ func (l Log) Wrap(next http.Handler) http.Handler { } } default: - level.Warn(requestLog).Log("msg", dskit_log.LazySprintf("%s %s (%d) %s Response: %q ws: %v; %s", r.Method, uri, statusCode, time.Since(begin), buf.Bytes(), IsWSHandshakeRequest(r), headers)) + if l.LogRequestHeaders && headers != nil { + level.Warn(requestLog).Log("msg", dskit_log.LazySprintf("%s %s (%d) %s Response: %q ws: %v; %s", r.Method, uri, statusCode, time.Since(begin), buf.Bytes(), IsWSHandshakeRequest(r), headers)) + } else { + level.Warn(requestLog).Log("msg", dskit_log.LazySprintf("%s %s (%d) %s", r.Method, uri, statusCode, time.Since(begin))) + } } }) } diff --git a/middleware/logging_test.go b/middleware/logging_test.go index e3449248a..574ef078c 100644 --- a/middleware/logging_test.go +++ b/middleware/logging_test.go @@ -149,32 +149,49 @@ func TestLoggingRequestWithExcludedHeaders(t *testing.T) { setHeaderList []string excludeHeaderList []string mustNotContain []string + statusCode int + logRequestHeaders bool }{ { - name: "Default excluded headers are excluded", - setHeaderList: defaultHeaders, - mustNotContain: defaultHeaders, + name: "Default excluded headers are excluded", + setHeaderList: defaultHeaders, + mustNotContain: defaultHeaders, + statusCode: http.StatusOK, + logRequestHeaders: true, }, { name: "Extra configured header is also excluded", setHeaderList: append(defaultHeaders, "X-Secret-Header"), excludeHeaderList: []string{"X-Secret-Header"}, mustNotContain: append(defaultHeaders, "X-Secret-Header"), + statusCode: http.StatusOK, + logRequestHeaders: true, }, { name: "Multiple extra configured headers are also excluded", setHeaderList: append(defaultHeaders, "X-Secret-Header", "X-Secret-Header-2"), excludeHeaderList: []string{"X-Secret-Header", "X-Secret-Header-2"}, mustNotContain: append(defaultHeaders, "X-Secret-Header", "X-Secret-Header-2"), + statusCode: http.StatusOK, + logRequestHeaders: true, + }, + { + name: "Log request headers disabled and status code 500", + setHeaderList: append(defaultHeaders, "X-Secret-Header"), + excludeHeaderList: []string{}, + mustNotContain: append(defaultHeaders, "X-Secret-Header"), + statusCode: http.StatusInternalServerError, + logRequestHeaders: false, }, } { t.Run(tc.name, func(t *testing.T) { buf := bytes.NewBuffer(nil) - loggingMiddleware := NewLogMiddleware(log.NewGoKitWithWriter(log.LogfmtFormat, buf), true, false, nil, tc.excludeHeaderList) + loggingMiddleware := NewLogMiddleware(log.NewGoKitWithWriter(log.LogfmtFormat, buf), tc.logRequestHeaders, false, nil, tc.excludeHeaderList) handler := func(w http.ResponseWriter, _ *http.Request) { _, _ = io.WriteString(w, "Hello world!") + w.WriteHeader(tc.statusCode) } loggingHandler := loggingMiddleware.Wrap(http.HandlerFunc(handler)) From f2a7eb3aa0e9d7db353bb56f8815f57231dd5394 Mon Sep 17 00:00:00 2001 From: Nikos Angelopoulos Date: Fri, 15 Nov 2024 09:27:28 +0100 Subject: [PATCH 03/13] test(kv): create mock client for counting func calls (#618) * test(kv): create mock client for counting func calls * address pr comments --- CHANGELOG.md | 1 + kv/mock.go | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d90ec93d1..74844199b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -237,6 +237,7 @@ * [ENHANCEMENT] Cache: Add `.Advance()` methods to mock cache clients for easier testing of TTLs. #601 * [ENHANCEMENT] Memberlist: Add concurrency to the transport's WriteTo method. #525 * [ENHANCEMENT] Memberlist: Notifications can now be processed once per interval specified by `-memberlist.notify-interval` to reduce notify storms in large clusters. #592 +* [ENHANCEMENT] KV: Add `MockCountingClient`, which wraps the `kv.client` and can be used in order to count calls at specific functions of the interface. #618 * [BUGFIX] spanlogger: Support multiple tenant IDs. #59 * [BUGFIX] Memberlist: fixed corrupted packets when sending compound messages with more than 255 messages or messages bigger than 64KB. #85 * [BUGFIX] Ring: `ring_member_ownership_percent` and `ring_tokens_owned` metrics are not updated on scale down. #109 diff --git a/kv/mock.go b/kv/mock.go index 59d743067..99c84e58d 100644 --- a/kv/mock.go +++ b/kv/mock.go @@ -5,6 +5,7 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" + "go.uber.org/atomic" ) // The mockClient does not anything. @@ -37,3 +38,63 @@ func (m mockClient) WatchKey(_ context.Context, _ string, _ func(interface{}) bo func (m mockClient) WatchPrefix(_ context.Context, _ string, _ func(string, interface{}) bool) { } + +// MockCountingClient is a wrapper around the Client interface that counts the number of times its functions are called. +// This is used for testing only. +type MockCountingClient struct { + client Client + + ListCalls *atomic.Uint32 + GetCalls *atomic.Uint32 + DeleteCalls *atomic.Uint32 + CASCalls *atomic.Uint32 + WatchKeyCalls *atomic.Uint32 + WatchPrefixCalls *atomic.Uint32 +} + +func NewMockCountingClient(client Client) *MockCountingClient { + return &MockCountingClient{ + client: client, + ListCalls: atomic.NewUint32(0), + GetCalls: atomic.NewUint32(0), + DeleteCalls: atomic.NewUint32(0), + CASCalls: atomic.NewUint32(0), + WatchKeyCalls: atomic.NewUint32(0), + WatchPrefixCalls: atomic.NewUint32(0), + } +} + +func (mc *MockCountingClient) List(ctx context.Context, prefix string) ([]string, error) { + mc.ListCalls.Inc() + + return mc.client.List(ctx, prefix) +} +func (mc *MockCountingClient) Get(ctx context.Context, key string) (interface{}, error) { + mc.GetCalls.Inc() + + return mc.client.Get(ctx, key) +} + +func (mc *MockCountingClient) Delete(ctx context.Context, key string) error { + mc.DeleteCalls.Inc() + + return mc.client.Delete(ctx, key) +} + +func (mc *MockCountingClient) CAS(ctx context.Context, key string, f func(in interface{}) (out interface{}, retry bool, err error)) error { + mc.CASCalls.Inc() + + return mc.client.CAS(ctx, key, f) +} + +func (mc *MockCountingClient) WatchKey(ctx context.Context, key string, f func(interface{}) bool) { + mc.WatchKeyCalls.Inc() + + mc.client.WatchKey(ctx, key, f) +} + +func (mc *MockCountingClient) WatchPrefix(ctx context.Context, key string, f func(string, interface{}) bool) { + mc.WatchPrefixCalls.Inc() + + mc.client.WatchPrefix(ctx, key, f) +} From f9da7284b699e837e2e58cab97b69684084a27a5 Mon Sep 17 00:00:00 2001 From: Nikos Angelopoulos Date: Wed, 20 Nov 2024 13:46:43 +0100 Subject: [PATCH 04/13] feat: extend kv.client storeconfig to support memberlist flags (#621) * feat: extend kv.client storeconfig to support memberlist flags * changelog: add entry --- CHANGELOG.md | 1 + kv/client.go | 8 +++++--- kv/client_test.go | 7 +++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74844199b..d677d5d1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,7 @@ * [CHANGE] memberlist: Failure to fast-join a cluster via contacting a node is now logged at `info` instead of `debug`. #585 * [CHANGE] `Service.AddListener` and `Manager.AddListener` now return function for stopping the listener. #564 * [CHANGE] ring: Add `InstanceRingReader` interface to `ring` package. #597 +* [CHANGE] KV: Add `MemberlistKVConfig` at kv `StoreConfig`. Extended `RegisterFlagsWithPrefix` to populate `MemberlistKVConfig` flags with specific prefix. #621 * [FEATURE] Cache: Add support for configuring a Redis cache backend. #268 #271 #276 * [FEATURE] Add support for waiting on the rate limiter using the new `WaitN` method. #279 * [FEATURE] Add `log.BufferedLogger` type. #338 diff --git a/kv/client.go b/kv/client.go index 0599f6702..a61f56708 100644 --- a/kv/client.go +++ b/kv/client.go @@ -42,9 +42,10 @@ var ( // Consul, Etcd, Memberlist or MultiClient. It was extracted from Config to keep // single-client config separate from final client-config (with all the wrappers) type StoreConfig struct { - Consul consul.Config `yaml:"consul"` - Etcd etcd.Config `yaml:"etcd"` - Multi MultiConfig `yaml:"multi"` + Consul consul.Config `yaml:"consul"` + Etcd etcd.Config `yaml:"etcd"` + MemberlistKVConfig memberlist.KVConfig `yaml:"memberlist"` + Multi MultiConfig `yaml:"multi"` // Function that returns memberlist.KV store to use. By using a function, we can delay // initialization of memberlist.KV until it is actually required. @@ -74,6 +75,7 @@ func (cfg *Config) RegisterFlagsWithPrefix(flagsPrefix, defaultPrefix string, f cfg.Consul.RegisterFlags(f, flagsPrefix) cfg.Etcd.RegisterFlagsWithPrefix(f, flagsPrefix) cfg.Multi.RegisterFlagsWithPrefix(f, flagsPrefix) + cfg.MemberlistKVConfig.RegisterFlagsWithPrefix(f, flagsPrefix) if flagsPrefix == "" { flagsPrefix = "ring." diff --git a/kv/client_test.go b/kv/client_test.go index a15adc34e..4280765c0 100644 --- a/kv/client_test.go +++ b/kv/client_test.go @@ -23,6 +23,10 @@ consul: host: "consul:8500" consistentreads: true prefix: "test/" +memberlist: + node_name: "testNode" + randomize_node_name: false + stream_timeout: 3s multi: primary: consul secondary: etcd @@ -35,6 +39,9 @@ multi: require.Equal(t, "consul", cfg.Store) require.Equal(t, "test/", cfg.Prefix) require.Equal(t, "consul:8500", cfg.Consul.Host) + require.Equal(t, "testNode", cfg.MemberlistKVConfig.NodeName) + require.Equal(t, false, cfg.MemberlistKVConfig.RandomizeNodeName) + require.Equal(t, 3*time.Second, cfg.MemberlistKVConfig.StreamTimeout) require.Equal(t, "consul", cfg.Multi.Primary) require.Equal(t, "etcd", cfg.Multi.Secondary) } From 22cde9208bdb335d3b9deddb38e8ee6e0d16abe4 Mon Sep 17 00:00:00 2001 From: Nikos Angelopoulos Date: Thu, 21 Nov 2024 13:38:46 +0100 Subject: [PATCH 05/13] Revert "feat: extend kv.client storeconfig to support memberlist flags (#621)" (#622) This reverts commit f9da7284b699e837e2e58cab97b69684084a27a5. --- CHANGELOG.md | 1 - kv/client.go | 8 +++----- kv/client_test.go | 7 ------- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d677d5d1c..74844199b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,7 +86,6 @@ * [CHANGE] memberlist: Failure to fast-join a cluster via contacting a node is now logged at `info` instead of `debug`. #585 * [CHANGE] `Service.AddListener` and `Manager.AddListener` now return function for stopping the listener. #564 * [CHANGE] ring: Add `InstanceRingReader` interface to `ring` package. #597 -* [CHANGE] KV: Add `MemberlistKVConfig` at kv `StoreConfig`. Extended `RegisterFlagsWithPrefix` to populate `MemberlistKVConfig` flags with specific prefix. #621 * [FEATURE] Cache: Add support for configuring a Redis cache backend. #268 #271 #276 * [FEATURE] Add support for waiting on the rate limiter using the new `WaitN` method. #279 * [FEATURE] Add `log.BufferedLogger` type. #338 diff --git a/kv/client.go b/kv/client.go index a61f56708..0599f6702 100644 --- a/kv/client.go +++ b/kv/client.go @@ -42,10 +42,9 @@ var ( // Consul, Etcd, Memberlist or MultiClient. It was extracted from Config to keep // single-client config separate from final client-config (with all the wrappers) type StoreConfig struct { - Consul consul.Config `yaml:"consul"` - Etcd etcd.Config `yaml:"etcd"` - MemberlistKVConfig memberlist.KVConfig `yaml:"memberlist"` - Multi MultiConfig `yaml:"multi"` + Consul consul.Config `yaml:"consul"` + Etcd etcd.Config `yaml:"etcd"` + Multi MultiConfig `yaml:"multi"` // Function that returns memberlist.KV store to use. By using a function, we can delay // initialization of memberlist.KV until it is actually required. @@ -75,7 +74,6 @@ func (cfg *Config) RegisterFlagsWithPrefix(flagsPrefix, defaultPrefix string, f cfg.Consul.RegisterFlags(f, flagsPrefix) cfg.Etcd.RegisterFlagsWithPrefix(f, flagsPrefix) cfg.Multi.RegisterFlagsWithPrefix(f, flagsPrefix) - cfg.MemberlistKVConfig.RegisterFlagsWithPrefix(f, flagsPrefix) if flagsPrefix == "" { flagsPrefix = "ring." diff --git a/kv/client_test.go b/kv/client_test.go index 4280765c0..a15adc34e 100644 --- a/kv/client_test.go +++ b/kv/client_test.go @@ -23,10 +23,6 @@ consul: host: "consul:8500" consistentreads: true prefix: "test/" -memberlist: - node_name: "testNode" - randomize_node_name: false - stream_timeout: 3s multi: primary: consul secondary: etcd @@ -39,9 +35,6 @@ multi: require.Equal(t, "consul", cfg.Store) require.Equal(t, "test/", cfg.Prefix) require.Equal(t, "consul:8500", cfg.Consul.Host) - require.Equal(t, "testNode", cfg.MemberlistKVConfig.NodeName) - require.Equal(t, false, cfg.MemberlistKVConfig.RandomizeNodeName) - require.Equal(t, 3*time.Second, cfg.MemberlistKVConfig.StreamTimeout) require.Equal(t, "consul", cfg.Multi.Primary) require.Equal(t, "etcd", cfg.Multi.Secondary) } From 77bb9ddddb0cd91677c4312edd6012b0a78713ac Mon Sep 17 00:00:00 2001 From: Martin Valiente Ainz <64830185+tinitiuset@users.noreply.github.com> Date: Mon, 25 Nov 2024 13:38:40 +0100 Subject: [PATCH 06/13] Feature: Add new metric `request_throughput` (#619) * Feature: Add new metric `slow_request_server_throughput` to track the throughput of slow queries. * Refactor variables and config keys, add logic to avoid unnecesary observations * Remove "Slow" from Throughput related variables. Extract header parsing into a dedicated function. * Added testing for `ExtractValueFromMultiValueHeader()` and `ThroughputMetricHistogram()` * Updated parsing to follow `Server-Timing` syntax format. * Update config key definitions Co-authored-by: Dimitar Dimitrov * Refactor code, add tests, rename RequestCutoff to LatencyCutoff * Fixed linting --------- Co-authored-by: Dimitar Dimitrov --- CHANGELOG.md | 1 + middleware/instrument.go | 36 ++++++ middleware/instrument_test.go | 213 ++++++++++++++++++++++++++++++++++ server/metrics.go | 12 ++ server/server.go | 12 ++ 5 files changed, 274 insertions(+) create mode 100644 middleware/instrument_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 74844199b..5bd0c1657 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## Changelog +* [CHANGE] Add new metric `slow_request_server_throughput` to track the throughput of slow queries. #619 * [CHANGE] Log middleware updated to honor `logRequestHeaders` in all logging scenarios. #615 * [CHANGE] Roll back the gRPC dependency to v1.65.0 to allow downstream projects to avoid a performance regression and maybe a bug in v1.66.0. #581 * [CHANGE] Update the gRPC dependency to v1.66.0 and deprecate the `grpc_server_recv_buffer_pools_enabled` option that is no longer supported by it. #580 diff --git a/middleware/instrument.go b/middleware/instrument.go index 9813077ce..89eea3e58 100644 --- a/middleware/instrument.go +++ b/middleware/instrument.go @@ -6,10 +6,12 @@ package middleware import ( "context" + "fmt" "io" "net/http" "strconv" "strings" + "time" "github.com/felixge/httpsnoop" "github.com/gorilla/mux" @@ -50,6 +52,9 @@ type Instrument struct { RequestBodySize *prometheus.HistogramVec ResponseBodySize *prometheus.HistogramVec InflightRequests *prometheus.GaugeVec + LatencyCutoff time.Duration + ThroughputUnit string + RequestThroughput *prometheus.HistogramVec } // IsWSHandshakeRequest returns true if the given request is a websocket handshake request. @@ -105,9 +110,40 @@ func (i Instrument) Wrap(next http.Handler) http.Handler { labelValues = append(labelValues, tenantID) instrument.ObserveWithExemplar(r.Context(), i.PerTenantDuration.WithLabelValues(labelValues...), respMetrics.Duration.Seconds()) } + if i.LatencyCutoff > 0 && respMetrics.Duration > i.LatencyCutoff { + volume, err := extractValueFromMultiValueHeader(w.Header().Get("Server-Timing"), i.ThroughputUnit, "val") + if err == nil { + instrument.ObserveWithExemplar(r.Context(), i.RequestThroughput.WithLabelValues(r.Method, route), volume/respMetrics.Duration.Seconds()) + } + } }) } +// Extracts a single value from a multi-value header, e.g. "name0;key0=0.0;key1=1.1, name1;key0=1.1" +func extractValueFromMultiValueHeader(h, name string, key string) (float64, error) { + parts := strings.Split(h, ", ") + if len(parts) == 0 { + return 0, fmt.Errorf("not a multi-value header") + } + for _, part := range parts { + if part, found := strings.CutPrefix(part, name); found { + for _, spart := range strings.Split(part, ";") { + if !strings.HasPrefix(spart, key) { + continue + } + var value float64 + _, err := fmt.Sscanf(spart, key+"=%f", &value) + if err != nil { + return 0, fmt.Errorf("failed to parse value from header: %w", err) + } + return value, nil + } + } + + } + return 0, fmt.Errorf("desired name not found in header") +} + // Return a name identifier for ths request. There are three options: // 1. The request matches a gorilla mux route, with a name. Use that. // 2. The request matches an unamed gorilla mux router. Munge the path diff --git a/middleware/instrument_test.go b/middleware/instrument_test.go new file mode 100644 index 000000000..80682d150 --- /dev/null +++ b/middleware/instrument_test.go @@ -0,0 +1,213 @@ +package middleware + +import ( + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" + "github.com/prometheus/client_golang/prometheus/testutil" + "github.com/stretchr/testify/require" + + "github.com/grafana/dskit/instrument" +) + +func TestThroughputMetricHistogram(t *testing.T) { + tests := []struct { + testName string + sleep bool + header string + observed bool + }{ + { + testName: "WithSleep", + sleep: true, + header: "unit;val=0, other_unit;val=2", + observed: true, + }, + { + testName: "WithoutSleep", + sleep: false, + header: "unit;val=0, other_unit;val=2", + observed: false, + }, + { + testName: "WithSleepEmptyHeader", + sleep: true, + header: "", + observed: false, + }, + { + testName: "WithoutSleepEmptyHeader", + sleep: false, + header: "", + observed: false, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + + reg := prometheus.NewPedanticRegistry() + i := newInstrument(reg) + + wrap := i.Wrap(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if tt.sleep { + time.Sleep(i.LatencyCutoff) + } + w.Header().Set("Server-Timing", tt.header) + })) + + req := httptest.NewRequest("GET", "/", nil) + res := httptest.NewRecorder() + + wrap.ServeHTTP(res, req) + + output := `` + if tt.observed { + output = ` + # HELP request_throughput_unit Server throughput running requests. + # TYPE request_throughput_unit histogram + request_throughput_unit_bucket{cutoff_ms="100",method="GET",route="other",le="1"} 1 + request_throughput_unit_bucket{cutoff_ms="100",method="GET",route="other",le="5"} 1 + request_throughput_unit_bucket{cutoff_ms="100",method="GET",route="other",le="10"} 1 + request_throughput_unit_bucket{cutoff_ms="100",method="GET",route="other",le="+Inf"} 1 + request_throughput_unit_sum{cutoff_ms="100",method="GET",route="other"} 0 + request_throughput_unit_count{cutoff_ms="100",method="GET",route="other"} 1 + ` + } + + require.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(output), "request_throughput_"+i.ThroughputUnit)) + }) + } +} + +func newInstrument(registry *prometheus.Registry) Instrument { + reg := promauto.With(registry) + + const throughputUnit = "unit" + const LatencyCutoff = 100 * time.Millisecond + + return Instrument{ + Duration: reg.NewHistogramVec(prometheus.HistogramOpts{ + Name: "request_duration_seconds", + Help: "Time (in seconds) spent serving HTTP requests.", + Buckets: instrument.DefBuckets, + }, []string{"method", "route", "status_code", "ws"}), + PerTenantDuration: reg.NewHistogramVec(prometheus.HistogramOpts{ + Name: "per_tenant_request_duration_seconds", + Help: "Time (in seconds) spent serving HTTP requests for a particular tenant.", + Buckets: instrument.DefBuckets, + }, []string{"method", "route", "status_code", "ws", "tenant"}), + RequestBodySize: reg.NewHistogramVec(prometheus.HistogramOpts{ + Name: "request_message_bytes", + Help: "Size (in bytes) of messages received in the request.", + Buckets: BodySizeBuckets, + }, []string{"method", "route"}), + ResponseBodySize: reg.NewHistogramVec(prometheus.HistogramOpts{ + Name: "response_message_bytes", + Help: "Size (in bytes) of messages sent in response.", + Buckets: BodySizeBuckets, + }, []string{"method", "route"}), + InflightRequests: reg.NewGaugeVec(prometheus.GaugeOpts{ + Name: "inflight_requests", + Help: "Current number of inflight requests.", + }, []string{"method", "route"}), + LatencyCutoff: LatencyCutoff, + ThroughputUnit: throughputUnit, + RequestThroughput: reg.NewHistogramVec(prometheus.HistogramOpts{ + Name: "request_throughput_" + throughputUnit, + Help: "Server throughput running requests.", + ConstLabels: prometheus.Labels{"cutoff_ms": strconv.FormatInt(LatencyCutoff.Milliseconds(), 10)}, + Buckets: []float64{1, 5, 10}, + }, []string{"method", "route"}), + } +} + +func TestExtractValueFromMultiValueHeader(t *testing.T) { + tests := []struct { + testName string + header string + name string + key string + expected float64 + err bool + }{ + { + testName: "ExistantKeyInName1", + header: "name0;key0=0.0;key1=1.1, name1;key0=1.1", + name: "name0", + key: "key0", + expected: 0.0, + err: false, + }, + { + testName: "ExistantKeyInName2", + header: "name0;key0=0.0;key1=1.1, name1;key1=1.1", + name: "name0", + key: "key1", + expected: 1.1, + err: false, + }, + { + testName: "NonExistantName1", + header: "name0;key0=0.0;key1=1.1, name1;key0=1.1", + name: "name2", + key: "key0", + expected: 0.0, + err: true, + }, + { + testName: "NonExistantName2", + header: "name0;key0=0.0;key1=1.1, name1;key1=1.1", + name: "name2", + key: "key1", + expected: 0.0, + err: true, + }, + { + testName: "NonExistantKeyInName", + header: "name0;key0=0.0;key1=1.1", + name: "name0", + key: "key2", + expected: 0, + err: true, + }, + { + testName: "StringInKey", + header: "name0;key0=str;key1=1.1", + name: "name0", + key: "key0", + expected: 0, + err: true, + }, + { + testName: "EmptyHeader", + header: "", + name: "name0", + key: "key0", + expected: 0, + err: true, + }, + { + testName: "IncorrectFormat", + header: "key0=0.0, key1=1.1", + name: "key0", + key: "key0", + expected: 0, + err: true, + }, + } + + for _, tt := range tests { + t.Run(tt.testName, func(t *testing.T) { + value, err := extractValueFromMultiValueHeader(tt.header, tt.name, tt.key) + require.Equal(t, tt.err, err != nil, "expected error: %v, got: %v", tt.err, err) + require.Equal(t, tt.expected, value, "expected value: %f, got: %f", tt.expected, value) + }) + } +} diff --git a/server/metrics.go b/server/metrics.go index d6011525d..bb9a643a5 100644 --- a/server/metrics.go +++ b/server/metrics.go @@ -5,6 +5,7 @@ package server import ( + "strconv" "time" "github.com/prometheus/client_golang/prometheus" @@ -22,6 +23,7 @@ type Metrics struct { ReceivedMessageSize *prometheus.HistogramVec SentMessageSize *prometheus.HistogramVec InflightRequests *prometheus.GaugeVec + RequestThroughput *prometheus.HistogramVec } func NewServerMetrics(cfg Config) *Metrics { @@ -73,5 +75,15 @@ func NewServerMetrics(cfg Config) *Metrics { Name: "inflight_requests", Help: "Current number of inflight requests.", }, []string{"method", "route"}), + RequestThroughput: reg.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: cfg.MetricsNamespace, + Name: "request_throughput_" + cfg.Throughput.Unit, + Help: "Server throughput of running requests.", + ConstLabels: prometheus.Labels{"cutoff_ms": strconv.FormatInt(cfg.Throughput.LatencyCutoff.Milliseconds(), 10)}, + Buckets: instrument.DefBuckets, + NativeHistogramBucketFactor: cfg.MetricsNativeHistogramFactor, + NativeHistogramMaxBucketNumber: 100, + NativeHistogramMinResetDuration: time.Hour, + }, []string{"method", "route"}), } } diff --git a/server/server.go b/server/server.go index 7b8e7593d..ced2700e9 100644 --- a/server/server.go +++ b/server/server.go @@ -153,6 +153,13 @@ type Config struct { // This limiter is called for every started and finished gRPC request. GrpcMethodLimiter GrpcInflightMethodLimiter `yaml:"-"` + + Throughput Throughput `yaml:"-"` +} + +type Throughput struct { + LatencyCutoff time.Duration `yaml:"throughput_latency_cutoff"` + Unit string `yaml:"throughput_unit"` } var infinty = time.Duration(math.MaxInt64) @@ -209,6 +216,8 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { f.StringVar(&cfg.LogRequestExcludeHeadersList, "server.log-request-headers-exclude-list", "", "Comma separated list of headers to exclude from loggin. Only used if server.log-request-headers is true.") f.BoolVar(&cfg.LogRequestAtInfoLevel, "server.log-request-at-info-level-enabled", false, "Optionally log requests at info level instead of debug level. Applies to request headers as well if server.log-request-headers is enabled.") f.BoolVar(&cfg.ProxyProtocolEnabled, "server.proxy-protocol-enabled", false, "Enables PROXY protocol.") + f.DurationVar(&cfg.Throughput.LatencyCutoff, "server.throughput.latency-cutoff", 0, "Requests taking over the cutoff are be observed to measure throughput. Server-Timing header is used with specified unit as the indicator, for example 'Server-Timing: unit;val=8.2'. If set to 0, the throughput is not calculated.") + f.StringVar(&cfg.Throughput.Unit, "server.throughput.unit", "total_samples", "Unit of the server throughput metric, for example 'processed_bytes' or 'total_samples'. Observed values are gathered from the 'Server-Timing' header with the 'val' key. If set, it is appended to the request_server_throughput metric name.") } func (cfg *Config) registererOrDefault() prometheus.Registerer { @@ -527,6 +536,9 @@ func BuildHTTPMiddleware(cfg Config, router *mux.Router, metrics *Metrics, logge RequestBodySize: metrics.ReceivedMessageSize, ResponseBodySize: metrics.SentMessageSize, InflightRequests: metrics.InflightRequests, + LatencyCutoff: cfg.Throughput.LatencyCutoff, + ThroughputUnit: cfg.Throughput.Unit, + RequestThroughput: metrics.RequestThroughput, }, } var httpMiddleware []middleware.Interface From 9973facd6a4a23a09e8fc357da8dcec116393290 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Wed, 4 Dec 2024 12:22:16 -0500 Subject: [PATCH 07/13] Faster tests using the mocked consul in-memory backend (#624) Signed-off-by: Marco Pracucci --- kv/consul/mock.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/kv/consul/mock.go b/kv/consul/mock.go index e2f434b61..ec848e135 100644 --- a/kv/consul/mock.go +++ b/kv/consul/mock.go @@ -15,6 +15,9 @@ import ( "github.com/grafana/dskit/kv/codec" ) +// The max wait time allowed for mockKV operations, in order to have faster tests. +const maxWaitTime = 100 * time.Millisecond + type mockKV struct { mtx sync.Mutex cond *sync.Cond @@ -87,7 +90,7 @@ func (m *mockKV) loop() { select { case <-m.close: return - case <-time.After(time.Second): + case <-time.After(maxWaitTime): m.mtx.Lock() m.cond.Broadcast() m.mtx.Unlock() @@ -258,7 +261,6 @@ func (m *mockKV) ResetIndexForKey(key string) { // mockedMaxWaitTime returns the minimum duration between the input duration // and the max wait time allowed in this mock, in order to have faster tests. func mockedMaxWaitTime(queryWaitTime time.Duration) time.Duration { - const maxWaitTime = time.Second if queryWaitTime > maxWaitTime { return maxWaitTime } From e27df29220ea69cf7335f653da18b8ea51ab27e4 Mon Sep 17 00:00:00 2001 From: Christian Simon Date: Thu, 12 Dec 2024 15:33:28 +0000 Subject: [PATCH 08/13] docs(spanprofiler): Update the reference after example moved (#625) --- spanprofiler/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spanprofiler/README.md b/spanprofiler/README.md index a415985f6..fbe2306ed 100644 --- a/spanprofiler/README.md +++ b/spanprofiler/README.md @@ -6,7 +6,7 @@ The Span Profiler for OpenTracing-Go is a package that seamlessly integrates `op profiling through the use of pprof labels. Accessing trace span profiles is made convenient through the Grafana Explore view. You can find a complete example setup -with Grafana Tempo in the [Pyroscope repository](https://github.com/grafana/pyroscope/tree/main/examples/tracing/tempo): +with Grafana Tempo in the [Pyroscope repository](https://github.com/grafana/pyroscope/tree/main/examples/tracing/golang-push): ![image](https://github.com/grafana/otel-profiling-go/assets/12090599/31e33cd1-818b-4116-b952-c9ec7b1fb593) From 0450f2ba7c3dc8ba6abc2434174c3a4242e0cc85 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Tue, 17 Dec 2024 01:40:23 +0800 Subject: [PATCH 09/13] Replace `exp/slices` with `slices` from standard library (#626) * Replace `exp/slices` with `slices` from standard library The experimental functions in `golang.org/x/exp/slices` are now available in the standard library in Go 1.21 [1]. [1]: https://go.dev/doc/go1.21#slices --------- Signed-off-by: Eng Zer Jun Co-authored-by: Arve Knudsen --- loser/loser.go | 8 ++++---- loser/loser_test.go | 6 +++--- ring/example/local/go.mod | 1 - ring/partition_instance_ring.go | 3 +-- ring/partition_instance_ring_test.go | 2 +- ring/partition_ring.go | 3 +-- ring/partition_ring_http.go | 3 +-- ring/partition_ring_model.go | 2 +- ring/partition_ring_test.go | 2 +- ring/spread_minimizing_token_generator.go | 3 +-- ring/spread_minimizing_token_generator_test.go | 2 +- ring/token_range.go | 2 +- ring/util.go | 2 +- 13 files changed, 17 insertions(+), 22 deletions(-) diff --git a/loser/loser.go b/loser/loser.go index b02e29f6c..9da903b19 100644 --- a/loser/loser.go +++ b/loser/loser.go @@ -2,9 +2,9 @@ package loser -import "golang.org/x/exp/constraints" +import "cmp" -func New[E constraints.Ordered](lists [][]E, maxVal E) *Tree[E] { +func New[E cmp.Ordered](lists [][]E, maxVal E) *Tree[E] { nLists := len(lists) t := Tree[E]{ maxVal: maxVal, @@ -23,12 +23,12 @@ func New[E constraints.Ordered](lists [][]E, maxVal E) *Tree[E] { // A loser tree is a binary tree laid out such that nodes N and N+1 have parent N/2. // We store M leaf nodes in positions M...2M-1, and M-1 internal nodes in positions 1..M-1. // Node 0 is a special node, containing the winner of the contest. -type Tree[E constraints.Ordered] struct { +type Tree[E cmp.Ordered] struct { maxVal E nodes []node[E] } -type node[E constraints.Ordered] struct { +type node[E cmp.Ordered] struct { index int // This is the loser for all nodes except the 0th, where it is the winner. value E // Value copied from the loser node, or winner for node 0. items []E // Only populated for leaf nodes. diff --git a/loser/loser_test.go b/loser/loser_test.go index 7f19eb6c5..838867507 100644 --- a/loser/loser_test.go +++ b/loser/loser_test.go @@ -1,19 +1,19 @@ package loser_test import ( + "cmp" "fmt" "math" "math/rand" + "slices" "testing" "github.com/stretchr/testify/require" - "golang.org/x/exp/constraints" - "golang.org/x/exp/slices" "github.com/grafana/dskit/loser" ) -func checkTreeEqual[E constraints.Ordered](t *testing.T, tree *loser.Tree[E], expected []E, msg ...interface{}) { +func checkTreeEqual[E cmp.Ordered](t *testing.T, tree *loser.Tree[E], expected []E, msg ...interface{}) { t.Helper() actual := []E{} diff --git a/ring/example/local/go.mod b/ring/example/local/go.mod index 1a4c72ced..258f22f74 100644 --- a/ring/example/local/go.mod +++ b/ring/example/local/go.mod @@ -57,7 +57,6 @@ require ( go.uber.org/atomic v1.10.0 // indirect go.uber.org/multierr v1.6.0 // indirect go.uber.org/zap v1.17.0 // indirect - golang.org/x/exp v0.0.0-20230321023759-10a507213a29 // indirect golang.org/x/mod v0.17.0 // indirect golang.org/x/net v0.26.0 // indirect golang.org/x/sync v0.7.0 // indirect diff --git a/ring/partition_instance_ring.go b/ring/partition_instance_ring.go index cffa4b2fc..1702d0cd8 100644 --- a/ring/partition_instance_ring.go +++ b/ring/partition_instance_ring.go @@ -2,9 +2,8 @@ package ring import ( "fmt" + "slices" "time" - - "golang.org/x/exp/slices" ) type PartitionRingReader interface { diff --git a/ring/partition_instance_ring_test.go b/ring/partition_instance_ring_test.go index 914157340..ef29bcb0c 100644 --- a/ring/partition_instance_ring_test.go +++ b/ring/partition_instance_ring_test.go @@ -2,12 +2,12 @@ package ring import ( "fmt" + "slices" "testing" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "golang.org/x/exp/slices" ) func TestPartitionInstanceRing_GetReplicationSetsForOperation(t *testing.T) { diff --git a/ring/partition_ring.go b/ring/partition_ring.go index 911de476c..21ef2d7fd 100644 --- a/ring/partition_ring.go +++ b/ring/partition_ring.go @@ -5,11 +5,10 @@ import ( "fmt" "math" "math/rand" + "slices" "strconv" "time" - "golang.org/x/exp/slices" - shardUtil "github.com/grafana/dskit/ring/shard" ) diff --git a/ring/partition_ring_http.go b/ring/partition_ring_http.go index 8e58c58c7..8fe3778eb 100644 --- a/ring/partition_ring_http.go +++ b/ring/partition_ring_http.go @@ -6,11 +6,10 @@ import ( "fmt" "html/template" "net/http" + "slices" "sort" "strconv" "time" - - "golang.org/x/exp/slices" ) //go:embed partition_ring_status.gohtml diff --git a/ring/partition_ring_model.go b/ring/partition_ring_model.go index c95380756..f957fe6b8 100644 --- a/ring/partition_ring_model.go +++ b/ring/partition_ring_model.go @@ -2,12 +2,12 @@ package ring import ( "fmt" + "slices" "strconv" "strings" "time" "github.com/gogo/protobuf/proto" - "golang.org/x/exp/slices" "github.com/grafana/dskit/kv/codec" "github.com/grafana/dskit/kv/memberlist" diff --git a/ring/partition_ring_test.go b/ring/partition_ring_test.go index 4395eed03..db75396f8 100644 --- a/ring/partition_ring_test.go +++ b/ring/partition_ring_test.go @@ -4,6 +4,7 @@ import ( "fmt" "math" "math/rand" + "slices" "strconv" "sync" "testing" @@ -12,7 +13,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/exp/maps" - "golang.org/x/exp/slices" ) func TestPartitionRing_ActivePartitionForKey(t *testing.T) { diff --git a/ring/spread_minimizing_token_generator.go b/ring/spread_minimizing_token_generator.go index bd2ed9970..12ec23b2d 100644 --- a/ring/spread_minimizing_token_generator.go +++ b/ring/spread_minimizing_token_generator.go @@ -5,10 +5,9 @@ import ( "fmt" "math" "regexp" + "slices" "sort" "strconv" - - "golang.org/x/exp/slices" ) const ( diff --git a/ring/spread_minimizing_token_generator_test.go b/ring/spread_minimizing_token_generator_test.go index ddae2f973..912df1bfd 100644 --- a/ring/spread_minimizing_token_generator_test.go +++ b/ring/spread_minimizing_token_generator_test.go @@ -4,11 +4,11 @@ import ( "fmt" "math" "math/rand" + "slices" "testing" "time" "github.com/stretchr/testify/require" - "golang.org/x/exp/slices" ) const ( diff --git a/ring/token_range.go b/ring/token_range.go index 1020ecd33..1598defee 100644 --- a/ring/token_range.go +++ b/ring/token_range.go @@ -2,9 +2,9 @@ package ring import ( "math" + "slices" "github.com/pkg/errors" - "golang.org/x/exp/slices" // using exp/slices until moving to go 1.21. ) // TokenRanges describes token ranges owned by an instance. diff --git a/ring/util.go b/ring/util.go index a21c0f2fe..910860cc8 100644 --- a/ring/util.go +++ b/ring/util.go @@ -3,11 +3,11 @@ package ring import ( "context" "math" + "slices" "sort" "time" "github.com/go-kit/log" - "golang.org/x/exp/slices" "github.com/grafana/dskit/backoff" "github.com/grafana/dskit/netutil" From 9935aca9d266be5de05450afd8278d99637a24b6 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Mon, 30 Dec 2024 03:26:52 -0500 Subject: [PATCH 10/13] Chore: log last heartbeat timestamp along with "instance found in the ring" (#630) Signed-off-by: Marco Pracucci --- ring/basic_lifecycler.go | 2 +- ring/model.go | 10 ++++++++++ ring/model_test.go | 30 ++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index 1675cafac..c0860d682 100644 --- a/ring/basic_lifecycler.go +++ b/ring/basic_lifecycler.go @@ -294,7 +294,7 @@ func (l *BasicLifecycler) registerInstance(ctx context.Context) error { var exists bool instanceDesc, exists = ringDesc.Ingesters[l.cfg.ID] if exists { - level.Info(l.logger).Log("msg", "instance found in the ring", "instance", l.cfg.ID, "ring", l.ringName, "state", instanceDesc.GetState(), "tokens", len(instanceDesc.GetTokens()), "registered_at", instanceDesc.GetRegisteredAt().String()) + level.Info(l.logger).Log("msg", "instance found in the ring", "instance", l.cfg.ID, "ring", l.ringName, "state", instanceDesc.GetState(), "tokens", len(instanceDesc.GetTokens()), "registered_at", instanceDesc.GetRegisteredAt().String(), "last_heartbeat_at", instanceDesc.GetLastHeartbeatAt().String()) } else { level.Info(l.logger).Log("msg", "instance not found in the ring", "instance", l.cfg.ID, "ring", l.ringName) } diff --git a/ring/model.go b/ring/model.go index c4ba64466..32529b6ba 100644 --- a/ring/model.go +++ b/ring/model.go @@ -146,6 +146,16 @@ func (i *InstanceDesc) GetRegisteredAt() time.Time { return time.Unix(i.RegisteredTimestamp, 0) } +// GetLastHeartbeatAt returns the timestamp of the last heartbeat sent by the instance +// or a zero value if unknown. +func (i *InstanceDesc) GetLastHeartbeatAt() time.Time { + if i == nil || i.Timestamp == 0 { + return time.Time{} + } + + return time.Unix(i.Timestamp, 0) +} + // GetReadOnlyState returns the read-only state and timestamp of last read-only state update. func (i *InstanceDesc) GetReadOnlyState() (bool, time.Time) { if i == nil { diff --git a/ring/model_test.go b/ring/model_test.go index 2f6c00043..be8ed343f 100644 --- a/ring/model_test.go +++ b/ring/model_test.go @@ -93,6 +93,36 @@ func TestInstanceDesc_GetRegisteredAt(t *testing.T) { } } +func TestInstanceDesc_GetLastHeartbeatAt(t *testing.T) { + tests := map[string]struct { + desc *InstanceDesc + expected time.Time + }{ + "should return zero value on nil desc": { + desc: nil, + expected: time.Time{}, + }, + "should return zero value if timestamp = 0": { + desc: &InstanceDesc{ + Timestamp: 0, + }, + expected: time.Time{}, + }, + "should return timestamp parsed from desc": { + desc: &InstanceDesc{ + Timestamp: time.Unix(10000000, 0).Unix(), + }, + expected: time.Unix(10000000, 0), + }, + } + + for testName, testData := range tests { + t.Run(testName, func(t *testing.T) { + assert.True(t, testData.desc.GetLastHeartbeatAt().Equal(testData.expected)) + }) + } +} + func normalizedSource() *Desc { r := NewDesc() r.Ingesters["first"] = InstanceDesc{ From b54518d1a463bc961ec223fb04c9496f74af98d5 Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Fri, 3 Jan 2025 13:24:58 -0600 Subject: [PATCH 11/13] chore: Display token information in partition ring status page (#631) * Show tokens in partition ring status page * Show ownership column * Additional countTokens tests * Add changelog * Rename header from Instance to Partition in show tokens mode --- CHANGELOG.md | 1 + ring/partition_ring_http.go | 23 ++++++++ ring/partition_ring_http_test.go | 98 +++++++++++++++++++++++-------- ring/partition_ring_model.go | 28 +++++++++ ring/partition_ring_model_test.go | 46 +++++++++++++++ ring/partition_ring_status.gohtml | 22 +++++++ 6 files changed, 193 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bd0c1657..ee1781868 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,7 @@ * [FEATURE] Add methods `Increment`, `FlushAll`, `CompareAndSwap`, `Touch` to `cache.MemcachedClient` #477 * [FEATURE] Add `concurrency.ForEachJobMergeResults()` utility function. #486 * [FEATURE] Add `ring.DoMultiUntilQuorumWithoutSuccessfulContextCancellation()`. #495 +* [ENHANCEMENT] Display token information in partition ring status page #631 * [ENHANCEMENT] Add ability to log all source hosts from http header instead of only the first one. #444 * [ENHANCEMENT] Add configuration to customize backoff for the gRPC clients. * [ENHANCEMENT] Use `SecretReader` interface to fetch secrets when configuring TLS. #274 diff --git a/ring/partition_ring_http.go b/ring/partition_ring_http.go index 8fe3778eb..698f33b0f 100644 --- a/ring/partition_ring_http.go +++ b/ring/partition_ring_http.go @@ -5,6 +5,7 @@ import ( _ "embed" "fmt" "html/template" + "math" "net/http" "slices" "sort" @@ -18,6 +19,9 @@ var partitionRingPageTemplate = template.Must(template.New("webpage").Funcs(temp "mod": func(i, j int32) bool { return i%j == 0 }, + "humanFloat": func(f float64) string { + return fmt.Sprintf("%.3g", f) + }, "formatTimestamp": func(ts time.Time) string { return ts.Format("2006-01-02 15:04:05 MST") }, @@ -55,6 +59,7 @@ func (h *PartitionRingPageHandler) handleGetRequest(w http.ResponseWriter, req * ring = h.reader.PartitionRing() ringDesc = ring.desc ) + ownedTokens := ringDesc.countTokens() // Prepare the data to render partitions in the page. partitionsByID := make(map[int32]partitionPageData, len(ringDesc.Partitions)) @@ -68,6 +73,9 @@ func (h *PartitionRingPageHandler) handleGetRequest(w http.ResponseWriter, req * State: partition.State, StateTimestamp: partition.GetStateTime(), OwnerIDs: owners, + Tokens: partition.Tokens, + NumTokens: len(partition.Tokens), + Ownership: distancePercentage(ownedTokens[id]), } } @@ -83,6 +91,9 @@ func (h *PartitionRingPageHandler) handleGetRequest(w http.ResponseWriter, req * State: PartitionUnknown, StateTimestamp: time.Time{}, OwnerIDs: []string{ownerID}, + Tokens: partition.Tokens, + NumTokens: len(partition.Tokens), + Ownership: distancePercentage(ownedTokens[owner.OwnedPartition]), } partitionsByID[owner.OwnedPartition] = partition @@ -105,6 +116,8 @@ func (h *PartitionRingPageHandler) handleGetRequest(w http.ResponseWriter, req * return partitions[i].ID < partitions[j].ID }) + tokensParam := req.URL.Query().Get("tokens") + renderHTTPResponse(w, partitionRingPageData{ Partitions: partitions, PartitionStateChanges: map[PartitionState]PartitionState{ @@ -112,6 +125,7 @@ func (h *PartitionRingPageHandler) handleGetRequest(w http.ResponseWriter, req * PartitionActive: PartitionInactive, PartitionInactive: PartitionActive, }, + ShowTokens: tokensParam == "true", }, partitionRingPageTemplate, req) } @@ -146,6 +160,7 @@ type partitionRingPageData struct { // PartitionStateChanges maps the allowed state changes through the UI. PartitionStateChanges map[PartitionState]PartitionState `json:"-"` + ShowTokens bool `json:"-"` } type partitionPageData struct { @@ -154,4 +169,12 @@ type partitionPageData struct { State PartitionState `json:"state"` StateTimestamp time.Time `json:"state_timestamp"` OwnerIDs []string `json:"owner_ids"` + Tokens []uint32 `json:"tokens"` + NumTokens int `json:"-"` + Ownership float64 `json:"-"` +} + +// distancePercentage renders a given token distance as the percentage of all possible token values covered by that distance. +func distancePercentage(distance int64) float64 { + return (float64(distance) / float64(math.MaxUint32)) * 100 } diff --git a/ring/partition_ring_http_test.go b/ring/partition_ring_http_test.go index 2b73321ea..aef11603d 100644 --- a/ring/partition_ring_http_test.go +++ b/ring/partition_ring_http_test.go @@ -28,10 +28,12 @@ func TestPartitionRingPageHandler_ViewPage(t *testing.T) { 1: { State: PartitionActive, StateTimestamp: time.Now().Unix(), + Tokens: []uint32{1000000, 3000000, 6000000}, }, 2: { State: PartitionInactive, StateTimestamp: time.Now().Unix(), + Tokens: []uint32{2000000, 4000000, 5000000, 7000000}, }, }, Owners: map[string]OwnerDesc{ @@ -59,31 +61,77 @@ func TestPartitionRingPageHandler_ViewPage(t *testing.T) { ) recorder := httptest.NewRecorder() - handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/partition-ring", nil)) - - assert.Equal(t, http.StatusOK, recorder.Code) - assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) - - assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ - "", "1", "", - "", "Active", "", - "", "[^<]+", "", - "", "ingester-zone-a-0", "
", "ingester-zone-b-0", "
", "", - }, `\s*`))), recorder.Body.String()) - - assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ - "", "2", "", - "", "Inactive", "", - "", "[^<]+", "", - "", "ingester-zone-a-1", "
", "ingester-zone-b-1", "
", "", - }, `\s*`))), recorder.Body.String()) - - assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ - "", "3", "", - "", "Corrupt", "", - "", "N/A", "", - "", "ingester-zone-b-2", "
", "", - }, `\s*`))), recorder.Body.String()) + + t.Run("displays expected partition info", func(t *testing.T) { + handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/partition-ring", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "1", "", + "", "Active", "", + "", "[^<]+", "", + "", "ingester-zone-a-0", "
", "ingester-zone-b-0", "
", "", + "", "3", "", + "", "99.9%", "", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "2", "", + "", "Inactive", "", + "", "[^<]+", "", + "", "ingester-zone-a-1", "
", "ingester-zone-b-1", "
", "", + "", "4", "", + "", "0.0931%", "", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "3", "", + "", "Corrupt", "", + "", "N/A", "", + "", "ingester-zone-b-2", "
", "", + "", "0", "", + "", "0%", "", + }, `\s*`))), recorder.Body.String()) + }) + + t.Run("displays Show Tokens button by default", func(t *testing.T) { + handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/partition-ring", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + ``, + }, `\s*`))), recorder.Body.String()) + }) + + t.Run("displays tokens when Show Tokens is enabled", func(t *testing.T) { + handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/partition-ring?tokens=true", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + ``, + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "

", "Partition: 1", "

", + "

", "Tokens:
", "1000000", "3000000", "6000000", "

", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "

", "Partition: 2", "

", + "

", "Tokens:
", "2000000", "4000000", "5000000", "7000000", "

", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "

", "Partition: 3", "

", + "

", "Tokens:
", "

", + }, `\s*`))), recorder.Body.String()) + }) } func TestPartitionRingPageHandler_ChangePartitionState(t *testing.T) { diff --git a/ring/partition_ring_model.go b/ring/partition_ring_model.go index f957fe6b8..cecda6b89 100644 --- a/ring/partition_ring_model.go +++ b/ring/partition_ring_model.go @@ -94,6 +94,34 @@ func (m *PartitionRingDesc) partitionByToken() map[Token]int32 { return out } +// CountTokens returns the summed token distance of all tokens in each partition. +func (m *PartitionRingDesc) countTokens() map[int32]int64 { + owned := make(map[int32]int64, len(m.Partitions)) + sortedTokens := m.tokens() + tokensToPartitions := m.partitionByToken() + + for i, token := range sortedTokens { + partition := tokensToPartitions[Token(token)] + + var prevToken uint32 + if i == 0 { + prevToken = sortedTokens[len(sortedTokens)-1] + } else { + prevToken = sortedTokens[i-1] + } + diff := tokenDistance(prevToken, token) + owned[partition] = owned[partition] + diff + } + + // Partitions with 0 tokens should still exist in the result. + for id := range m.Partitions { + if _, ok := owned[id]; !ok { + owned[id] = 0 + } + } + return owned +} + // ownersByPartition returns a map where the key is the partition ID and the value is a list of owner IDs. func (m *PartitionRingDesc) ownersByPartition() map[int32][]string { out := make(map[int32][]string, len(m.Partitions)) diff --git a/ring/partition_ring_model_test.go b/ring/partition_ring_model_test.go index 183b9bf56..ca71429b4 100644 --- a/ring/partition_ring_model_test.go +++ b/ring/partition_ring_model_test.go @@ -2,6 +2,7 @@ package ring import ( "fmt" + "math" "testing" "time" @@ -79,6 +80,51 @@ func TestPartitionRingDesc_countPartitionsByState(t *testing.T) { }) } +func TestPartitionRingDesc_countTokens(t *testing.T) { + t.Run("empty ring should return an empty result", func(t *testing.T) { + desc := &PartitionRingDesc{} + + result := desc.countTokens() + + assert.Empty(t, result) + }) + + t.Run("ring with some partitions should return correct distances", func(t *testing.T) { + desc := &PartitionRingDesc{ + Partitions: map[int32]PartitionDesc{ + 1: {Tokens: []uint32{1000000, 3000000, 6000000}}, + 2: {Tokens: []uint32{2000000, 4000000, 8000000}}, + 3: {Tokens: []uint32{5000000, 9000000}}, + }, + } + + result := desc.countTokens() + + expected := map[int32]int64{ + 1: 3000000 + (int64(math.MaxUint32) + 1 - 9000000), + 2: 4000000, + 3: 2000000, + } + assert.Equal(t, expected, result) + }) + + t.Run("partitions with no tokens should be present in the result, with 0 distance", func(t *testing.T) { + desc := &PartitionRingDesc{ + Partitions: map[int32]PartitionDesc{ + 1: {Tokens: []uint32{1000000, 3000000, 6000000}}, + 2: {Tokens: []uint32{2000000, 4000000, 8000000}}, + 3: {Tokens: []uint32{5000000, 9000000}}, + 4: {Tokens: []uint32{}}, + }, + } + + result := desc.countTokens() + + assert.Contains(t, result, int32(4)) + assert.Equal(t, int64(0), result[4]) + }) +} + func TestPartitionRingDesc_AddOrUpdateOwner(t *testing.T) { now := time.Now() diff --git a/ring/partition_ring_status.gohtml b/ring/partition_ring_status.gohtml index f4f9afe87..1f0a2eaf0 100644 --- a/ring/partition_ring_status.gohtml +++ b/ring/partition_ring_status.gohtml @@ -15,6 +15,8 @@ State State updated at Owners + Tokens + Ownership Actions @@ -42,6 +44,8 @@ {{$ownerID}}
{{ end }} + {{ .NumTokens }} + {{ .Ownership | humanFloat }}% {{ if and (not .Corrupted) (ne (index $stateChanges .State) 0) }} @@ -59,5 +63,23 @@ {{ end }} +
+ {{ if .ShowTokens }} + + {{ else }} + + {{ end }} + + {{ if .ShowTokens }} + {{ range $i, $partition := .Partitions }} +

Partition: {{ .ID }}

+

+ Tokens:
+ {{ range $token := .Tokens }} + {{ $token }} + {{ end }} +

+ {{ end }} + {{ end }} \ No newline at end of file From 3702098cbd0c676b648945ee812313ca409a8daa Mon Sep 17 00:00:00 2001 From: Alexander Weaver Date: Mon, 6 Jan 2025 14:57:46 -0600 Subject: [PATCH 12/13] chore: Add option to hide tokens from ring statuspage (#633) * Add option to hide tokens from status page * drive-by: use a per-case recorder * changelog * linter * Rename to HideTokensInStatusPage --- CHANGELOG.md | 1 + ring/basic_lifecycler.go | 4 +- ring/lifecycler.go | 4 +- ring/partition_ring_http_test.go | 5 +- ring/ring.go | 5 +- ring/ring_http.go | 20 ++-- ring/ring_http_test.go | 154 +++++++++++++++++++++++++++++++ ring/ring_status.gohtml | 35 ++++--- 8 files changed, 202 insertions(+), 26 deletions(-) create mode 100644 ring/ring_http_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index ee1781868..cb4029f73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,7 @@ * [FEATURE] Add methods `Increment`, `FlushAll`, `CompareAndSwap`, `Touch` to `cache.MemcachedClient` #477 * [FEATURE] Add `concurrency.ForEachJobMergeResults()` utility function. #486 * [FEATURE] Add `ring.DoMultiUntilQuorumWithoutSuccessfulContextCancellation()`. #495 +* [ENHANCEMENT] Add option to hide token information in ring status page #633 * [ENHANCEMENT] Display token information in partition ring status page #631 * [ENHANCEMENT] Add ability to log all source hosts from http header instead of only the first one. #444 * [ENHANCEMENT] Add configuration to customize backoff for the gRPC clients. diff --git a/ring/basic_lifecycler.go b/ring/basic_lifecycler.go index c0860d682..1a2e10380 100644 --- a/ring/basic_lifecycler.go +++ b/ring/basic_lifecycler.go @@ -53,6 +53,8 @@ type BasicLifecyclerConfig struct { HeartbeatTimeout time.Duration TokensObservePeriod time.Duration NumTokens int + // HideTokensInStatusPage allows tokens to be hidden from management tools e.g. the status page, for use in contexts which do not utilize tokens. + HideTokensInStatusPage bool // If true lifecycler doesn't unregister instance from the ring when it's stopping. Default value is false, // which means unregistering. @@ -546,5 +548,5 @@ func (l *BasicLifecycler) getRing(ctx context.Context) (*Desc, error) { } func (l *BasicLifecycler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - newRingPageHandler(l, l.cfg.HeartbeatTimeout).handle(w, req) + newRingPageHandler(l, l.cfg.HeartbeatTimeout, l.cfg.HideTokensInStatusPage).handle(w, req) } diff --git a/ring/lifecycler.go b/ring/lifecycler.go index 083f112bd..bb9f1e8f3 100644 --- a/ring/lifecycler.go +++ b/ring/lifecycler.go @@ -55,6 +55,8 @@ type LifecyclerConfig struct { // Injected internally ListenPort int `yaml:"-"` + // HideTokensInStatusPage allows tokens to be hidden from management tools e.g. the status page, for use in contexts which do not utilize tokens. + HideTokensInStatusPage bool `yaml:"-"` // If set, specifies the TokenGenerator implementation that will be used for generating tokens. // Default value is nil, which means that RandomTokenGenerator is used. @@ -1088,7 +1090,7 @@ func (i *Lifecycler) getRing(ctx context.Context) (*Desc, error) { } func (i *Lifecycler) ServeHTTP(w http.ResponseWriter, req *http.Request) { - newRingPageHandler(i, i.cfg.HeartbeatTimeout).handle(w, req) + newRingPageHandler(i, i.cfg.HeartbeatTimeout, i.cfg.HideTokensInStatusPage).handle(w, req) } // unregister removes our entry from consul. diff --git a/ring/partition_ring_http_test.go b/ring/partition_ring_http_test.go index aef11603d..13293ff32 100644 --- a/ring/partition_ring_http_test.go +++ b/ring/partition_ring_http_test.go @@ -60,9 +60,8 @@ func TestPartitionRingPageHandler_ViewPage(t *testing.T) { nil, ) - recorder := httptest.NewRecorder() - t.Run("displays expected partition info", func(t *testing.T) { + recorder := httptest.NewRecorder() handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/partition-ring", nil)) assert.Equal(t, http.StatusOK, recorder.Code) @@ -97,6 +96,7 @@ func TestPartitionRingPageHandler_ViewPage(t *testing.T) { }) t.Run("displays Show Tokens button by default", func(t *testing.T) { + recorder := httptest.NewRecorder() handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/partition-ring", nil)) assert.Equal(t, http.StatusOK, recorder.Code) @@ -108,6 +108,7 @@ func TestPartitionRingPageHandler_ViewPage(t *testing.T) { }) t.Run("displays tokens when Show Tokens is enabled", func(t *testing.T) { + recorder := httptest.NewRecorder() handler.ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/partition-ring?tokens=true", nil)) assert.Equal(t, http.StatusOK, recorder.Code) diff --git a/ring/ring.go b/ring/ring.go index d47eb8fe2..62a49a6d8 100644 --- a/ring/ring.go +++ b/ring/ring.go @@ -150,6 +150,9 @@ type Config struct { // Whether the shuffle-sharding subring cache is disabled. This option is set // internally and never exposed to the user. SubringCacheDisabled bool `yaml:"-"` + // HideTokensInStatusPage allows tokens to be hidden from management tools e.g. the status page, for use in contexts which do not utilize tokens. + // This option is set internally and never exposed to the user. + HideTokensInStatusPage bool `yaml:"-"` } // RegisterFlags adds the flags required to config this to the given FlagSet with a specified prefix @@ -1223,7 +1226,7 @@ func (r *Ring) getRing(_ context.Context) (*Desc, error) { } func (r *Ring) ServeHTTP(w http.ResponseWriter, req *http.Request) { - newRingPageHandler(r, r.cfg.HeartbeatTimeout).handle(w, req) + newRingPageHandler(r, r.cfg.HeartbeatTimeout, r.cfg.HideTokensInStatusPage).handle(w, req) } // InstancesCount returns the number of instances in the ring. diff --git a/ring/ring_http.go b/ring/ring_http.go index 67249e2b4..d961d8b15 100644 --- a/ring/ring_http.go +++ b/ring/ring_http.go @@ -30,9 +30,12 @@ var defaultPageTemplate = template.Must(template.New("webpage").Funcs(template.F }).Parse(defaultPageContent)) type httpResponse struct { - Ingesters []ingesterDesc `json:"shards"` - Now time.Time `json:"now"` - ShowTokens bool `json:"-"` + Ingesters []ingesterDesc `json:"shards"` + Now time.Time `json:"now"` + // ShowTokens indicates whether the Show Tokens button is clicked. + ShowTokens bool `json:"-"` + // DisableTokens hides the concept of tokens entirely in the page, across all elements. + DisableTokens bool `json:"-"` } type ingesterDesc struct { @@ -57,12 +60,14 @@ type ringAccess interface { type ringPageHandler struct { r ringAccess heartbeatTimeout time.Duration + disableTokens bool } -func newRingPageHandler(r ringAccess, heartbeatTimeout time.Duration) *ringPageHandler { +func newRingPageHandler(r ringAccess, heartbeatTimeout time.Duration, disableTokens bool) *ringPageHandler { return &ringPageHandler{ r: r, heartbeatTimeout: heartbeatTimeout, + disableTokens: disableTokens, } } @@ -132,9 +137,10 @@ func (h *ringPageHandler) handle(w http.ResponseWriter, req *http.Request) { tokensParam := req.URL.Query().Get("tokens") renderHTTPResponse(w, httpResponse{ - Ingesters: ingesters, - Now: now, - ShowTokens: tokensParam == "true", + Ingesters: ingesters, + Now: now, + ShowTokens: tokensParam == "true", + DisableTokens: h.disableTokens, }, defaultPageTemplate, req) } diff --git a/ring/ring_http_test.go b/ring/ring_http_test.go new file mode 100644 index 000000000..8c27016f7 --- /dev/null +++ b/ring/ring_http_test.go @@ -0,0 +1,154 @@ +package ring + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "regexp" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestRingPageHandler_handle(t *testing.T) { + now := time.Now() + ring := fakeRingAccess{ + desc: &Desc{ + Ingesters: map[string]InstanceDesc{ + "1": { + Zone: "zone-a", + State: ACTIVE, + Addr: "addr-a", + Timestamp: now.Unix(), + Tokens: []uint32{1000000, 3000000, 6000000}, + }, + "2": { + Zone: "zone-b", + State: ACTIVE, + Addr: "addr-b", + Timestamp: now.Unix(), + Tokens: []uint32{2000000, 4000000, 5000000, 7000000}, + }, + }, + }, + } + handler := newRingPageHandler(&ring, 10*time.Second, false) + + t.Run("displays instance info", func(t *testing.T) { + recorder := httptest.NewRecorder() + handler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "1", "", + "", "zone-a", "", + "", "ACTIVE", "", + "", "addr-a", "", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "3", "", + "", "100%", "", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "2", "", + "", "zone-b", "", + "", "ACTIVE", "", + "", "addr-b", "", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "4", "", + "", "100%", "", + }, `\s*`))), recorder.Body.String()) + }) + + t.Run("displays Show Tokens button by default", func(t *testing.T) { + recorder := httptest.NewRecorder() + handler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + ``, + }, `\s*`))), recorder.Body.String()) + }) + + t.Run("displays tokens when Show Tokens is enabled", func(t *testing.T) { + recorder := httptest.NewRecorder() + handler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring?tokens=true", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + ``, + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "

", "Instance: 1", "

", + "

", "Tokens:
", "1000000", "3000000", "6000000", "

", + }, `\s*`))), recorder.Body.String()) + + assert.Regexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "

", "Instance: 2", "

", + "

", "Tokens:
", "2000000", "4000000", "5000000", "7000000", "

", + }, `\s*`))), recorder.Body.String()) + }) + + tokenDisabledHandler := newRingPageHandler(&ring, 10*time.Second, true) + + t.Run("hides token columns when tokens are disabled", func(t *testing.T) { + recorder := httptest.NewRecorder() + tokenDisabledHandler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "Tokens", "", + "", "Ownership", "", + }, `\s*`))), recorder.Body.String()) + + assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "3", "", + "", "100%", "", + }, `\s*`))), recorder.Body.String()) + + assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + "", "4", "", + "", "100%", "", + }, `\s*`))), recorder.Body.String()) + }) + + t.Run("hides Show Tokens button when tokens are disabled", func(t *testing.T) { + recorder := httptest.NewRecorder() + tokenDisabledHandler.handle(recorder, httptest.NewRequest(http.MethodGet, "/ring", nil)) + + assert.Equal(t, http.StatusOK, recorder.Code) + assert.Equal(t, "text/html", recorder.Header().Get("Content-Type")) + + assert.NotRegexp(t, regexp.MustCompile(fmt.Sprintf("(?m)%s", strings.Join([]string{ + `input type="button" value="Show Tokens"`, + }, `\s*`))), recorder.Body.String()) + }) +} + +type fakeRingAccess struct { + desc *Desc +} + +func (f *fakeRingAccess) getRing(context.Context) (*Desc, error) { + return f.desc, nil +} + +func (f *fakeRingAccess) casRing(_ context.Context, _ func(in interface{}) (out interface{}, retry bool, err error)) error { + return nil +} diff --git a/ring/ring_status.gohtml b/ring/ring_status.gohtml index 157f8d89e..055873f3b 100644 --- a/ring/ring_status.gohtml +++ b/ring/ring_status.gohtml @@ -21,8 +21,10 @@ Read-Only Read-Only Updated Last Heartbeat + {{ if not .DisableTokens }} Tokens Ownership + {{ end }} Actions @@ -46,8 +48,10 @@ {{ .ReadOnlyUpdatedTimestamp | timeOrEmptyString }} {{ end }} {{ .HeartbeatTimestamp | durationSince }} ago ({{ .HeartbeatTimestamp.Format "15:04:05.999" }}) + {{ if not $.DisableTokens }} {{ .NumTokens }} {{ .Ownership | humanFloat }}% + {{ end }} @@ -56,21 +60,24 @@
- {{ if .ShowTokens }} - - {{ else }} - - {{ end }} - {{ if .ShowTokens }} - {{ range $i, $ing := .Ingesters }} -

Instance: {{ .ID }}

-

- Tokens:
- {{ range $token := .Tokens }} - {{ $token }} - {{ end }} -

+ {{ if not .DisableTokens}} + {{ if .ShowTokens }} + + {{ else }} + + {{ end }} + + {{ if .ShowTokens }} + {{ range $i, $ing := .Ingesters }} +

Instance: {{ .ID }}

+

+ Tokens:
+ {{ range $token := .Tokens }} + {{ $token }} + {{ end }} +

+ {{ end }} {{ end }} {{ end }} From 441a90acd4e5761bb88ebb7629f27c2e69652e02 Mon Sep 17 00:00:00 2001 From: Martin Valiente Ainz <64830185+tinitiuset@users.noreply.github.com> Date: Tue, 7 Jan 2025 15:25:22 +0100 Subject: [PATCH 13/13] Update default values for server.throughput.unit (#635) --- server/server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/server.go b/server/server.go index ced2700e9..f6c7f997f 100644 --- a/server/server.go +++ b/server/server.go @@ -217,7 +217,7 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) { f.BoolVar(&cfg.LogRequestAtInfoLevel, "server.log-request-at-info-level-enabled", false, "Optionally log requests at info level instead of debug level. Applies to request headers as well if server.log-request-headers is enabled.") f.BoolVar(&cfg.ProxyProtocolEnabled, "server.proxy-protocol-enabled", false, "Enables PROXY protocol.") f.DurationVar(&cfg.Throughput.LatencyCutoff, "server.throughput.latency-cutoff", 0, "Requests taking over the cutoff are be observed to measure throughput. Server-Timing header is used with specified unit as the indicator, for example 'Server-Timing: unit;val=8.2'. If set to 0, the throughput is not calculated.") - f.StringVar(&cfg.Throughput.Unit, "server.throughput.unit", "total_samples", "Unit of the server throughput metric, for example 'processed_bytes' or 'total_samples'. Observed values are gathered from the 'Server-Timing' header with the 'val' key. If set, it is appended to the request_server_throughput metric name.") + f.StringVar(&cfg.Throughput.Unit, "server.throughput.unit", "samples_processed", "Unit of the server throughput metric, for example 'processed_bytes' or 'samples_processed'. Observed values are gathered from the 'Server-Timing' header with the 'val' key. If set, it is appended to the request_server_throughput metric name.") } func (cfg *Config) registererOrDefault() prometheus.Registerer {