diff --git a/circuit_breaker_nil_monitoring_test.go b/circuit_breaker_nil_monitoring_test.go new file mode 100644 index 0000000..ad2de5d --- /dev/null +++ b/circuit_breaker_nil_monitoring_test.go @@ -0,0 +1,44 @@ +package main + +import ( + "bytes" + "testing" + + libpack_logger "github.com/lukaszraczylo/graphql-monitoring-proxy/logging" + "github.com/stretchr/testify/require" +) + +// TestInitCircuitBreaker_NilMonitoring_NoPanic is a regression test for a boot +// panic: enabling the circuit breaker (ENABLE_CIRCUIT_BREAKER=true) crashed at +// startup because parseConfig calls initCircuitBreaker before the monitoring +// server assigns cfg.Monitoring (StartMonitoringServer runs later). The metrics +// gauge registration then dereferenced a nil *MetricsSetup. +// +// The breaker (and the admin dashboard, which reads gobreaker state directly) +// must initialise without panicking even when monitoring is not yet available. +func TestInitCircuitBreaker_NilMonitoring_NoPanic(t *testing.T) { + origCfg := cfg + cbMutex.Lock() + origCb, origMetrics := cb, cbMetrics + cb, cbMetrics = nil, nil + cbMutex.Unlock() + t.Cleanup(func() { + cbMutex.Lock() + cb, cbMetrics = origCb, origMetrics + cbMutex.Unlock() + cfg = origCfg + }) + + cfg = &config{} + cfg.Logger = libpack_logger.New().SetOutput(&bytes.Buffer{}) + cfg.Monitoring = nil // the production state when parseConfig runs + cfg.CircuitBreaker.Enable = true + cfg.CircuitBreaker.MaxFailures = 3 + cfg.CircuitBreaker.Timeout = 5 + cfg.CircuitBreaker.MaxRequestsInHalfOpen = 2 + + require.NotPanics(t, func() { initCircuitBreaker(cfg) }, + "initCircuitBreaker must not panic when cfg.Monitoring is nil") + require.NotNil(t, cb, "circuit breaker must initialise even without monitoring") + require.NotNil(t, cbMetrics, "circuit breaker metrics manager must be created") +} diff --git a/monitoring/monitoring.go b/monitoring/monitoring.go index da6c147..e84901f 100644 --- a/monitoring/monitoring.go +++ b/monitoring/monitoring.go @@ -109,6 +109,11 @@ func (ms *MetricsSetup) ListActiveMetrics() []string { } func (ms *MetricsSetup) RegisterMetricsGauge(metric_name string, labels map[string]string, val float64) *metrics.Gauge { + if ms == nil { + // Monitoring may not be initialized yet (e.g. during config parsing, + // before the monitoring server starts). Return a dummy to prevent panics. + return &metrics.Gauge{} + } if err := validate_metrics_name(metric_name); err != nil { log.Error(&libpack_logger.LogMessage{ Message: "RegisterMetricsGauge() error - invalid metric name", @@ -125,6 +130,9 @@ func (ms *MetricsSetup) RegisterMetricsGauge(metric_name string, labels map[stri // RegisterMetricsGaugeFunc registers a gauge with a callback function that is called on every scrape // This is useful for metrics that need to return a dynamic value func (ms *MetricsSetup) RegisterMetricsGaugeFunc(metric_name string, labels map[string]string, fn func() float64) *metrics.Gauge { + if ms == nil { + return &metrics.Gauge{} + } if err := validate_metrics_name(metric_name); err != nil { log.Error(&libpack_logger.LogMessage{ Message: "RegisterMetricsGaugeFunc() error - invalid metric name", @@ -137,6 +145,9 @@ func (ms *MetricsSetup) RegisterMetricsGaugeFunc(metric_name string, labels map[ } func (ms *MetricsSetup) RegisterMetricsCounter(metric_name string, labels map[string]string) *metrics.Counter { + if ms == nil { + return &metrics.Counter{} + } if err := validate_metrics_name(metric_name); err != nil { log.Error(&libpack_logger.LogMessage{ Message: "RegisterMetricsCounter() error - invalid metric name", @@ -152,6 +163,9 @@ func (ms *MetricsSetup) RegisterMetricsCounter(metric_name string, labels map[st } func (ms *MetricsSetup) RegisterFloatCounter(metric_name string, labels map[string]string) *metrics.FloatCounter { + if ms == nil { + return &metrics.FloatCounter{} + } if err := validate_metrics_name(metric_name); err != nil { log.Error(&libpack_logger.LogMessage{ Message: "RegisterFloatCounter() error - invalid metric name", @@ -164,6 +178,9 @@ func (ms *MetricsSetup) RegisterFloatCounter(metric_name string, labels map[stri } func (ms *MetricsSetup) RegisterMetricsSummary(metric_name string, labels map[string]string) *metrics.Summary { + if ms == nil { + return &metrics.Summary{} + } if err := validate_metrics_name(metric_name); err != nil { log.Error(&libpack_logger.LogMessage{ Message: "RegisterMetricsSummary() error - invalid metric name", @@ -176,6 +193,9 @@ func (ms *MetricsSetup) RegisterMetricsSummary(metric_name string, labels map[st } func (ms *MetricsSetup) RegisterMetricsHistogram(metric_name string, labels map[string]string) *metrics.Histogram { + if ms == nil { + return &metrics.Histogram{} + } if err := validate_metrics_name(metric_name); err != nil { log.Error(&libpack_logger.LogMessage{ Message: "RegisterMetricsHistogram() error - invalid metric name", diff --git a/monitoring/nil_safety_test.go b/monitoring/nil_safety_test.go new file mode 100644 index 0000000..3656dd4 --- /dev/null +++ b/monitoring/nil_safety_test.go @@ -0,0 +1,25 @@ +package libpack_monitoring + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestRegistrationMethods_NilReceiver_NoPanic verifies that the metric +// registration methods tolerate a nil *MetricsSetup receiver. Callers may +// register metrics before the monitoring server has constructed the global +// MetricsSetup (e.g. circuit breaker init during config parsing); these +// methods must return a non-nil dummy instead of panicking. +func TestRegistrationMethods_NilReceiver_NoPanic(t *testing.T) { + var ms *MetricsSetup // nil + + require.NotPanics(t, func() { + require.NotNil(t, ms.RegisterMetricsGauge("g", nil, 1)) + require.NotNil(t, ms.RegisterMetricsGaugeFunc("gf", nil, func() float64 { return 1 })) + require.NotNil(t, ms.RegisterMetricsCounter("c", nil)) + require.NotNil(t, ms.RegisterFloatCounter("fc", nil)) + require.NotNil(t, ms.RegisterMetricsSummary("s", nil)) + require.NotNil(t, ms.RegisterMetricsHistogram("h", nil)) + }) +}