Skip to content

Commit 05344a3

Browse files
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 <abhaysgoudnvr@gmail.com>
1 parent 11e894a commit 05344a3

2 files changed

Lines changed: 15 additions & 1 deletion

File tree

internal/metrics/metrics.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"os"
1919

2020
"github.com/rs/zerolog"
21+
"github.com/sirupsen/logrus"
2122
)
2223

2324
type Writer interface {
@@ -57,7 +58,8 @@ func NewZerologMetrics(enabled bool, target string, containerID string) Writer {
5758
zerolog.TimeFieldFormat = zerolog.TimeFormatUnixNano
5859
file, err := os.OpenFile(target, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
5960
if err != nil {
60-
return nil
61+
logrus.Warnf("Failed to open metrics file %s: %v. Metrics will be disabled.", target, err)
62+
return &mockWriter{}
6163
}
6264
logger := zerolog.New(file).Level(zerolog.InfoLevel).With().Timestamp().Logger()
6365
return &zerologMetrics{

internal/metrics/metrics_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,15 @@ func TestZerologMetricsMetadata(t *testing.T) {
5252
t.Errorf("timestampOrder = %v, want 0", got)
5353
}
5454
}
55+
56+
func TestZerologMetricsInvalidFileDoesNotPanic(t *testing.T) {
57+
// Provide a path to a directory that definitely does not exist
58+
invalidPath := "/does/not/exist/timestamps.log"
59+
60+
// Create the metrics writer with timestamps enabled but an invalid path
61+
writer := NewZerologMetrics(true, invalidPath, "container-test")
62+
63+
// This call would panic with a nil pointer dereference without the fix.
64+
// With the fix, it will safely execute the mockWriter's no-op Capture().
65+
writer.Capture(TS00)
66+
}

0 commit comments

Comments
 (0)