From 05344a31ad7cfcc3167221f64425da8481215abd Mon Sep 17 00:00:00 2001 From: abhaygoudannavar Date: Fri, 1 May 2026 09:25:35 +0000 Subject: [PATCH 1/2] fix(metrics): fallback to mock writer on file open failure When timestamps are enabled but the target file cannot be opened, NewZerologMetrics returned nil. This caused a nil pointer dereference on subsequent metrics.Capture() calls. This fix gracefully degrades to a no-op mockWriter and logs a warning. Adds a regression test to verify the fallback behavior. Fixes #595 Signed-off-by: abhaygoudannavar --- internal/metrics/metrics.go | 4 +++- internal/metrics/metrics_test.go | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 69a815fc..cab21bae 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -18,6 +18,7 @@ import ( "os" "github.com/rs/zerolog" + "github.com/sirupsen/logrus" ) type Writer interface { @@ -57,7 +58,8 @@ func NewZerologMetrics(enabled bool, target string, containerID string) Writer { zerolog.TimeFieldFormat = zerolog.TimeFormatUnixNano file, err := os.OpenFile(target, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) if err != nil { - return nil + logrus.Warnf("Failed to open metrics file %s: %v. Metrics will be disabled.", target, err) + return &mockWriter{} } logger := zerolog.New(file).Level(zerolog.InfoLevel).With().Timestamp().Logger() return &zerologMetrics{ diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index cb4230d7..e5f541dc 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -52,3 +52,15 @@ func TestZerologMetricsMetadata(t *testing.T) { t.Errorf("timestampOrder = %v, want 0", got) } } + +func TestZerologMetricsInvalidFileDoesNotPanic(t *testing.T) { + // Provide a path to a directory that definitely does not exist + invalidPath := "/does/not/exist/timestamps.log" + + // Create the metrics writer with timestamps enabled but an invalid path + writer := NewZerologMetrics(true, invalidPath, "container-test") + + // This call would panic with a nil pointer dereference without the fix. + // With the fix, it will safely execute the mockWriter's no-op Capture(). + writer.Capture(TS00) +} From f5df6066633b573fd9d1f3374f22b97b56076eca Mon Sep 17 00:00:00 2001 From: abhaygoudannavar Date: Fri, 15 May 2026 14:14:10 +0000 Subject: [PATCH 2/2] test(metrics): explicitly check for non-nil writer instead of crash testing Signed-off-by: abhaygoudannavar --- internal/metrics/metrics_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index e5f541dc..31d096f4 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -56,11 +56,11 @@ func TestZerologMetricsMetadata(t *testing.T) { func TestZerologMetricsInvalidFileDoesNotPanic(t *testing.T) { // Provide a path to a directory that definitely does not exist invalidPath := "/does/not/exist/timestamps.log" - + // Create the metrics writer with timestamps enabled but an invalid path writer := NewZerologMetrics(true, invalidPath, "container-test") - - // This call would panic with a nil pointer dereference without the fix. - // With the fix, it will safely execute the mockWriter's no-op Capture(). - writer.Capture(TS00) + + if writer == nil { + t.Fatal("expected non-nil writer (fallback to mockWriter), got nil") + } }