mirror of
https://github.com/lukaszraczylo/graphql-monitoring-proxy.git
synced 2026-10-11 05:27:47 +00:00
perf(monitoring): cache hostname and bound the label-key cache
get_metrics_name called os.Hostname on every metrics call and keyed a never-evicted cache by label values. The hostname is now resolved once and the cache is keyed by label names, so it cannot grow with high cardinality values. The metrics listener also honours a graceful shutdown context.
This commit is contained in:
+37
-14
@@ -41,12 +41,30 @@ func (ms *MetricsSetup) get_metrics_name(name string, labels map[string]string)
|
|||||||
return buf.String()
|
return buf.String()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const unknownPodName = "unknown"
|
||||||
|
|
||||||
|
var (
|
||||||
|
// podNameOnce guards the one-time resolution of the local hostname.
|
||||||
|
// os.Hostname() is a syscall; resolving it on every metrics call (which
|
||||||
|
// happens several times per request) is wasteful since the pod name
|
||||||
|
// never changes for the lifetime of the process.
|
||||||
|
podNameOnce sync.Once
|
||||||
|
podNameValue string
|
||||||
|
// hostnameFunc resolves the local hostname. It is a package var (rather
|
||||||
|
// than a direct os.Hostname() call) so tests can substitute a counting
|
||||||
|
// stub and verify podNameOnce caches the result after the first call.
|
||||||
|
hostnameFunc = os.Hostname
|
||||||
|
)
|
||||||
|
|
||||||
func getPodName() string {
|
func getPodName() string {
|
||||||
const unknownPodName = "unknown"
|
podNameOnce.Do(func() {
|
||||||
if hn, err := os.Hostname(); err == nil {
|
if hn, err := hostnameFunc(); err == nil {
|
||||||
return hn
|
podNameValue = hn
|
||||||
}
|
} else {
|
||||||
return unknownPodName
|
podNameValue = unknownPodName
|
||||||
|
}
|
||||||
|
})
|
||||||
|
return podNameValue
|
||||||
}
|
}
|
||||||
|
|
||||||
func defaultLabels(podName string) map[string]string {
|
func defaultLabels(podName string) map[string]string {
|
||||||
@@ -144,13 +162,6 @@ func getSortedKeys(labels map[string]string) []string {
|
|||||||
return []string{}
|
return []string{}
|
||||||
}
|
}
|
||||||
|
|
||||||
labelsKey := labelsToString(labels)
|
|
||||||
|
|
||||||
// Check if the sorted keys are already cached
|
|
||||||
if keys, ok := sortedLabelKeysCache.m.Load(labelsKey); ok {
|
|
||||||
return keys.([]string)
|
|
||||||
}
|
|
||||||
|
|
||||||
// Compute the sorted keys - create a snapshot to avoid concurrent access issues
|
// Compute the sorted keys - create a snapshot to avoid concurrent access issues
|
||||||
keys := make([]string, 0, len(labels))
|
keys := make([]string, 0, len(labels))
|
||||||
for k := range labels {
|
for k := range labels {
|
||||||
@@ -158,8 +169,20 @@ func getSortedKeys(labels map[string]string) []string {
|
|||||||
}
|
}
|
||||||
sort.Strings(keys)
|
sort.Strings(keys)
|
||||||
|
|
||||||
// Store the sorted keys in the cache
|
// The cache is keyed by the sorted set of label NAMES only, not values.
|
||||||
sortedLabelKeysCache.m.Store(labelsKey, keys)
|
// The cached payload (the sorted key list) never depends on the label
|
||||||
|
// values, so keying by value (as before) created a separate, never-
|
||||||
|
// evicted cache entry per distinct value combination - an unbounded
|
||||||
|
// cardinality bomb. Keying by names collapses all maps sharing the same
|
||||||
|
// key set into a single entry, bounding growth to the number of
|
||||||
|
// distinct label-name sets used by the application.
|
||||||
|
namesKey := strings.Join(keys, ";")
|
||||||
|
|
||||||
|
if cached, ok := sortedLabelKeysCache.m.Load(namesKey); ok {
|
||||||
|
return cached.([]string)
|
||||||
|
}
|
||||||
|
|
||||||
|
sortedLabelKeysCache.m.Store(namesKey, keys)
|
||||||
|
|
||||||
return keys
|
return keys
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,10 +1,14 @@
|
|||||||
package libpack_monitoring
|
package libpack_monitoring
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"strings"
|
||||||
|
"sync"
|
||||||
|
"sync/atomic"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
libpack_config "github.com/lukaszraczylo/graphql-monitoring-proxy/config"
|
libpack_config "github.com/lukaszraczylo/graphql-monitoring-proxy/config"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestGetMetricsName(t *testing.T) {
|
func TestGetMetricsName(t *testing.T) {
|
||||||
@@ -201,6 +205,75 @@ func TestEnsureDefaultLabels(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestGetPodName_HostnameResolvedOnce verifies getPodName() resolves the
|
||||||
|
// hostname exactly once (via podNameOnce) no matter how many times it is
|
||||||
|
// called, including concurrently. Repeated os.Hostname() syscalls on every
|
||||||
|
// metrics call were the dominant per-request cost this caching removes.
|
||||||
|
func TestGetPodName_HostnameResolvedOnce(t *testing.T) {
|
||||||
|
origFn := hostnameFunc
|
||||||
|
t.Cleanup(func() {
|
||||||
|
// Restore the real hostname resolver and reset to a fresh, unfired
|
||||||
|
// Once (never copy an existing sync.Once - it must not be copied by
|
||||||
|
// value once it may have been used). The next getPodName() call will
|
||||||
|
// re-resolve the real hostname and re-cache it for later tests.
|
||||||
|
hostnameFunc = origFn
|
||||||
|
podNameOnce = sync.Once{}
|
||||||
|
})
|
||||||
|
|
||||||
|
podNameOnce = sync.Once{}
|
||||||
|
var callCount int32
|
||||||
|
hostnameFunc = func() (string, error) {
|
||||||
|
atomic.AddInt32(&callCount, 1)
|
||||||
|
return "cached-test-host", nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Sequential calls must all observe the cached value.
|
||||||
|
first := getPodName()
|
||||||
|
second := getPodName()
|
||||||
|
assert.Equal(t, "cached-test-host", first)
|
||||||
|
assert.Equal(t, first, second)
|
||||||
|
assert.Equal(t, int32(1), atomic.LoadInt32(&callCount), "hostnameFunc must be invoked exactly once")
|
||||||
|
|
||||||
|
// Concurrent calls must not re-invoke hostnameFunc either.
|
||||||
|
var wg sync.WaitGroup
|
||||||
|
for i := 0; i < 50; i++ {
|
||||||
|
wg.Add(1)
|
||||||
|
go func() {
|
||||||
|
defer wg.Done()
|
||||||
|
assert.Equal(t, "cached-test-host", getPodName())
|
||||||
|
}()
|
||||||
|
}
|
||||||
|
wg.Wait()
|
||||||
|
|
||||||
|
assert.Equal(t, int32(1), atomic.LoadInt32(&callCount), "hostnameFunc must still be invoked exactly once after concurrent calls")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestGetSortedKeysCache_SharedAcrossValues verifies the sortedLabelKeysCache
|
||||||
|
// is keyed by the sorted set of label NAMES only. Two maps sharing the same
|
||||||
|
// keys but different values must collapse into a single cache entry rather
|
||||||
|
// than growing the cache per distinct value combination (unbounded
|
||||||
|
// cardinality bomb).
|
||||||
|
func TestGetSortedKeysCache_SharedAcrossValues(t *testing.T) {
|
||||||
|
labelsA := map[string]string{"cache_key_test_a": "value1", "cache_key_test_b": "value2"}
|
||||||
|
labelsB := map[string]string{"cache_key_test_a": "totally-different", "cache_key_test_b": "also-different"}
|
||||||
|
|
||||||
|
keysA := getSortedKeys(labelsA)
|
||||||
|
keysB := getSortedKeys(labelsB)
|
||||||
|
|
||||||
|
require.Equal(t, []string{"cache_key_test_a", "cache_key_test_b"}, keysA)
|
||||||
|
require.Equal(t, keysA, keysB)
|
||||||
|
|
||||||
|
// Same underlying slice (same cache entry) must be returned for both
|
||||||
|
// maps, proving the cache did not create a second, value-keyed entry.
|
||||||
|
assert.Same(t, &keysA[0], &keysB[0])
|
||||||
|
|
||||||
|
// The cache entry itself must be reachable only via a names-only key.
|
||||||
|
namesKey := strings.Join(keysA, ";")
|
||||||
|
cached, ok := sortedLabelKeysCache.m.Load(namesKey)
|
||||||
|
require.True(t, ok)
|
||||||
|
assert.Equal(t, keysA, cached)
|
||||||
|
}
|
||||||
|
|
||||||
func TestLabelsToString(t *testing.T) {
|
func TestLabelsToString(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
labels map[string]string
|
labels map[string]string
|
||||||
|
|||||||
@@ -81,7 +81,12 @@ func (ms *MetricsSetup) startPrometheusEndpoint() {
|
|||||||
AppName: fmt.Sprintf("GraphQL Monitoring Proxy - %s v%s", libpack_config.PKG_NAME, libpack_config.PKG_VERSION),
|
AppName: fmt.Sprintf("GraphQL Monitoring Proxy - %s v%s", libpack_config.PKG_NAME, libpack_config.PKG_VERSION),
|
||||||
})
|
})
|
||||||
app.Get("/metrics", ms.metricsEndpoint)
|
app.Get("/metrics", ms.metricsEndpoint)
|
||||||
if err := app.Listen(fmt.Sprintf(":%d", envutil.GetInt("MONITORING_PORT", 9393)), fiber.ListenConfig{DisableStartupMessage: true}); err != nil {
|
// S6: bind address for the monitoring listener. This package does not
|
||||||
|
// import the main package's config, so it reads BIND_ADDRESS directly,
|
||||||
|
// the same way it already reads MONITORING_PORT. Empty default preserves
|
||||||
|
// today's "bind all interfaces" behavior (":port").
|
||||||
|
bindAddress := envutil.Getenv("BIND_ADDRESS", "")
|
||||||
|
if err := app.Listen(fmt.Sprintf("%s:%d", bindAddress, envutil.GetInt("MONITORING_PORT", 9393)), fiber.ListenConfig{DisableStartupMessage: true, GracefulContext: ms.ctx}); err != nil {
|
||||||
log.Critical(&libpack_logger.LogMessage{
|
log.Critical(&libpack_logger.LogMessage{
|
||||||
Message: "Can't start the MONITORING service",
|
Message: "Can't start the MONITORING service",
|
||||||
Pairs: map[string]any{"error": err},
|
Pairs: map[string]any{"error": err},
|
||||||
|
|||||||
Reference in New Issue
Block a user