mirror of
https://github.com/lukaszraczylo/graphql-monitoring-proxy.git
synced 2026-10-12 05:32:36 +00:00
fix(circuitbreaker): guard nil monitoring to prevent boot panic
Starting with ENABLE_CIRCUIT_BREAKER=true panicked at boot: parseConfig -> initCircuitBreaker registers a metrics gauge, but cfg.Monitoring is assigned later by StartMonitoringServer, so the registration dereferenced a nil *MetricsSetup. Add nil-receiver guards to the MetricsSetup registration methods (RegisterMetricsGauge/GaugeFunc/Counter, RegisterFloatCounter, RegisterMetricsSummary/Histogram), matching their existing 'return a dummy to prevent panics' contract. The guard is a no-op for non-nil receivers, so existing callers are unaffected. The breaker and the admin dashboard (which reads gobreaker state directly) keep working. Add regression tests for the nil-receiver methods and for initCircuitBreaker with nil monitoring.
This commit is contained in:
@@ -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")
|
||||||
|
}
|
||||||
@@ -109,6 +109,11 @@ func (ms *MetricsSetup) ListActiveMetrics() []string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (ms *MetricsSetup) RegisterMetricsGauge(metric_name string, labels map[string]string, val float64) *metrics.Gauge {
|
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 {
|
if err := validate_metrics_name(metric_name); err != nil {
|
||||||
log.Error(&libpack_logger.LogMessage{
|
log.Error(&libpack_logger.LogMessage{
|
||||||
Message: "RegisterMetricsGauge() error - invalid metric name",
|
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
|
// 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
|
// 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 {
|
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 {
|
if err := validate_metrics_name(metric_name); err != nil {
|
||||||
log.Error(&libpack_logger.LogMessage{
|
log.Error(&libpack_logger.LogMessage{
|
||||||
Message: "RegisterMetricsGaugeFunc() error - invalid metric name",
|
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 {
|
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 {
|
if err := validate_metrics_name(metric_name); err != nil {
|
||||||
log.Error(&libpack_logger.LogMessage{
|
log.Error(&libpack_logger.LogMessage{
|
||||||
Message: "RegisterMetricsCounter() error - invalid metric name",
|
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 {
|
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 {
|
if err := validate_metrics_name(metric_name); err != nil {
|
||||||
log.Error(&libpack_logger.LogMessage{
|
log.Error(&libpack_logger.LogMessage{
|
||||||
Message: "RegisterFloatCounter() error - invalid metric name",
|
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 {
|
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 {
|
if err := validate_metrics_name(metric_name); err != nil {
|
||||||
log.Error(&libpack_logger.LogMessage{
|
log.Error(&libpack_logger.LogMessage{
|
||||||
Message: "RegisterMetricsSummary() error - invalid metric name",
|
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 {
|
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 {
|
if err := validate_metrics_name(metric_name); err != nil {
|
||||||
log.Error(&libpack_logger.LogMessage{
|
log.Error(&libpack_logger.LogMessage{
|
||||||
Message: "RegisterMetricsHistogram() error - invalid metric name",
|
Message: "RegisterMetricsHistogram() error - invalid metric name",
|
||||||
|
|||||||
@@ -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))
|
||||||
|
})
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user