diff --git a/cmd/pulse-sensor-proxy/validation.go b/cmd/pulse-sensor-proxy/validation.go index 8a100ac..80095ff 100644 --- a/cmd/pulse-sensor-proxy/validation.go +++ b/cmd/pulse-sensor-proxy/validation.go @@ -9,8 +9,6 @@ import ( "strings" "sync" "time" - "unicode" - "unicode/utf8" "github.com/google/uuid" "github.com/rs/zerolog/log" @@ -25,13 +23,6 @@ var ( ipv4Regex = regexp.MustCompile(`^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$`) ) -var ( - allowedCommands = map[string]struct{}{ - "sensors": {}, - "ipmitool": {}, - } -) - // sanitizeCorrelationID validates and sanitizes a correlation ID // Returns a valid UUID, generating a new one if input is missing or invalid func sanitizeCorrelationID(id string) string { @@ -70,142 +61,6 @@ func validateNodeName(name string) error { return fmt.Errorf("invalid node name") } -func validateCommand(name string, args []string) error { - if err := validateCommandName(name); err != nil { - return err - } - - for _, arg := range args { - if err := validateCommandArg(arg); err != nil { - return err - } - } - - if name == "ipmitool" { - if err := validateIPMIToolArgs(args); err != nil { - return err - } - } - - return nil -} - -func validateCommandName(name string) error { - if name == "" { - return errors.New("command required") - } - - if strings.Contains(name, "/") { - return errors.New("absolute command paths not allowed") - } - - if _, ok := allowedCommands[name]; !ok { - return fmt.Errorf("command %q not permitted", name) - } - - if !isASCII(name) { - return errors.New("command must be ASCII") - } - - return nil -} - -func validateCommandArg(arg string) error { - if len(arg) == 0 { - return nil - } - - if len(arg) > 1024 { - return errors.New("argument too long") - } - - if !utf8.ValidString(arg) { - return errors.New("argument contains invalid UTF-8") - } - - if hasNullByte(arg) { - return errors.New("argument contains null byte") - } - - if !isASCII(arg) { - return errors.New("argument must be ASCII") - } - - if hasShellMeta(arg) { - return errors.New("argument contains forbidden shell characters") - } - - if strings.Contains(arg, "=") && !strings.HasPrefix(arg, "-") { - return errors.New("environment-style arguments not permitted") - } - - return nil -} - -func validateIPMIToolArgs(args []string) error { - lowered := make([]string, len(args)) - for i, arg := range args { - lowered[i] = strings.ToLower(arg) - } - - for i := 0; i < len(lowered); i++ { - token := lowered[i] - switch token { - case "shell", "raw", "exec", "lanplus", "lanplusciphers": - return errors.New("dangerous ipmitool arguments not permitted") - case "chassis": - if i+1 < len(lowered) { - switch lowered[i+1] { - case "power", "bootparam", "status", "policy": - return errors.New("chassis operations not permitted") - } - } - case "power", "reset", "off", "cycle", "bmc", "mc": - return errors.New("power control commands not permitted") - } - } - - return nil -} - -func hasShellMeta(s string) bool { - forbidden := []string{";", "|", "&", "$", "`", "\\", ">", "<", "(", ")", "[", "]", "{", "}", "!", "~"} - for _, ch := range forbidden { - if strings.Contains(s, ch) { - return true - } - } - - if strings.Contains(s, "..") { - return true - } - - if strings.ContainsAny(s, "\n\r\t") { - return true - } - - if strings.HasPrefix(s, "-") && strings.Contains(s, "=") { - if strings.Contains(s, "/") { - return true - } - } - - return false -} - -func hasNullByte(s string) bool { - return strings.IndexByte(s, 0) >= 0 -} - -func isASCII(s string) bool { - for _, r := range s { - if r > unicode.MaxASCII { - return false - } - } - return true -} - const ( nodeValidatorCacheTTL = 5 * time.Minute diff --git a/cmd/pulse-sensor-proxy/validation_fuzz_test.go b/cmd/pulse-sensor-proxy/validation_fuzz_test.go deleted file mode 100644 index b4b4c96..0000000 --- a/cmd/pulse-sensor-proxy/validation_fuzz_test.go +++ /dev/null @@ -1,31 +0,0 @@ -package main - -import ( - "strings" - "testing" -) - -func FuzzValidateCommand(f *testing.F) { - seeds := []string{ - "sensors -j", - "ipmitool sdr", - "sensors", - "ipmitool lan print", - } - for _, seed := range seeds { - f.Add(seed) - } - - f.Fuzz(func(t *testing.T, input string) { - fields := strings.Fields(input) - if len(fields) == 0 { - return - } - cmd := fields[0] - args := []string{} - if len(fields) > 1 { - args = fields[1:] - } - validateCommand(cmd, args) // ensure no panics - }) -} diff --git a/cmd/pulse-sensor-proxy/validation_test.go b/cmd/pulse-sensor-proxy/validation_test.go index 3fbac30..6c501d3 100644 --- a/cmd/pulse-sensor-proxy/validation_test.go +++ b/cmd/pulse-sensor-proxy/validation_test.go @@ -83,48 +83,6 @@ func TestValidateNodeName(t *testing.T) { } } -func TestValidateCommand(t *testing.T) { - type tc struct { - name string - args []string - wantErr bool - desc string - } - - cases := []tc{ - {name: "sensors", args: nil, wantErr: false, desc: "bare sensors"}, - {name: "sensors", args: []string{"-j"}, wantErr: false, desc: "json flag"}, - {name: "ipmitool", args: []string{"sdr"}, wantErr: false, desc: "safe ipmitool"}, - {name: "sensors", args: []string{"; rm -rf /"}, wantErr: true, desc: "shell metachar"}, - {name: "sensors", args: []string{"$(id)"}, wantErr: true, desc: "subshell"}, - {name: "ipmitool", args: []string{"-H", "1.2.3.4", "&&", "shutdown"}, wantErr: true, desc: "command chaining"}, - {name: "sensors", args: []string{">/tmp/out"}, wantErr: true, desc: "redirect"}, - {name: "senso\u200Brs", wantErr: true, desc: "unicode homoglyph"}, - {name: "sensors", args: []string{"-" + strings.Repeat("v", 2000)}, wantErr: true, desc: "arg too long"}, - {name: "sensors", args: []string{"test\x00"}, wantErr: true, desc: "null byte arg"}, - {name: "ipmitool", args: []string{"chassis", "power", "off"}, wantErr: true, desc: "dangerous ipmitool"}, - {name: "sensors", args: []string{"LC_ALL=C"}, wantErr: true, desc: "env prefix"}, - {name: "/usr/bin/sensors", wantErr: true, desc: "absolute path"}, - {name: "ipmitool", args: []string{"--extraneous=../../etc/passwd"}, wantErr: true, desc: "path traversal"}, - } - - for _, tc := range cases { - tc := tc - if tc.desc == "" { - tc.desc = tc.name - } - t.Run(tc.desc, func(t *testing.T) { - err := validateCommand(tc.name, tc.args) - if tc.wantErr && err == nil { - t.Fatalf("expected error for %s %v", tc.name, tc.args) - } - if !tc.wantErr && err != nil { - t.Fatalf("unexpected error for %s %v: %v", tc.name, tc.args, err) - } - }) - } -} - type stubResolver struct { ips []net.IP err error diff --git a/frontend-modern/src/components/Alerts/ActivationBanner.tsx b/frontend-modern/src/components/Alerts/ActivationBanner.tsx deleted file mode 100644 index a112d56..0000000 --- a/frontend-modern/src/components/Alerts/ActivationBanner.tsx +++ /dev/null @@ -1,18 +0,0 @@ -import type { JSX } from 'solid-js'; -import type { Alert } from '@/types/api'; -import type { ActivationState, AlertConfig } from '@/types/alerts'; - -interface ActivationBannerProps { - activationState: () => ActivationState | null; - activeAlerts: () => Alert[] | undefined; - config: () => AlertConfig | null; - isPastObservationWindow: () => boolean; - isLoading: () => boolean; - refreshActiveAlerts: () => Promise; - activate: () => Promise; -} - -export function ActivationBanner(_props: ActivationBannerProps): JSX.Element { - // Notifications activation banner/modal intentionally disabled. - return <>; -} diff --git a/frontend-modern/src/stores/metricsSampler.ts b/frontend-modern/src/stores/metricsSampler.ts index d94933d..776cb4e 100644 --- a/frontend-modern/src/stores/metricsSampler.ts +++ b/frontend-modern/src/stores/metricsSampler.ts @@ -156,25 +156,3 @@ export function startMetricsSampler(): void { } } -/** - * Stop the metrics sampler - */ -export function stopMetricsSampler(): void { - if (samplerInterval !== null) { - window.clearInterval(samplerInterval); - samplerInterval = null; - } - - isRunning = false; - logger.info('[MetricsSampler] Stopped'); -} - -/** - * Get sampler status - */ -export function getSamplerStatus() { - return { - isRunning, - intervalMs: SAMPLE_INTERVAL_MS, - }; -} diff --git a/frontend-modern/src/utils/alerts.ts b/frontend-modern/src/utils/alerts.ts index e5fb822..75ebe43 100644 --- a/frontend-modern/src/utils/alerts.ts +++ b/frontend-modern/src/utils/alerts.ts @@ -106,12 +106,3 @@ export const getAlertStyles = ( }; }; -// Get alert messages for a specific resource -export const getResourceAlerts = ( - resourceId: string, - activeAlerts: Record, - alertsEnabled: boolean | undefined = isAlertsActivationEnabled(), -): Alert[] => { - if (!alertsEnabled) return []; - return Object.values(activeAlerts).filter((alert) => alert.resourceId === resourceId); -}; diff --git a/frontend-modern/src/utils/format.ts b/frontend-modern/src/utils/format.ts index adf0b37..d3ab5c0 100644 --- a/frontend-modern/src/utils/format.ts +++ b/frontend-modern/src/utils/format.ts @@ -15,15 +15,6 @@ export function formatSpeed(bytesPerSecond: number, decimals = 0): string { return `${formatBytes(bytesPerSecond, decimals)}/s`; } -export function formatSpeedCompact(bytesPerSecond: number): string { - if (!bytesPerSecond || bytesPerSecond < 0) return '0'; - const k = 1024; - const mbps = bytesPerSecond / (k * k); - if (mbps < 1) return '<1'; - if (mbps < 10) return mbps.toFixed(1); - return Math.round(mbps).toString(); -} - export function formatPercent(value: number): string { if (!Number.isFinite(value)) return '0%'; const abs = Math.abs(value); diff --git a/frontend-modern/src/utils/metricsKeys.ts b/frontend-modern/src/utils/metricsKeys.ts index 1f2ffec..13b1529 100644 --- a/frontend-modern/src/utils/metricsKeys.ts +++ b/frontend-modern/src/utils/metricsKeys.ts @@ -17,20 +17,6 @@ export function buildMetricKey(kind: MetricResourceKind, id: string): string { return `${kind}:${id}`; } -/** - * Parse a metric key back into its components - * Returns null if the key format is invalid - */ -export function parseMetricKey(key: string): { kind: MetricResourceKind; id: string } | null { - const colonIndex = key.indexOf(':'); - if (colonIndex === -1) return null; - - const kind = key.slice(0, colonIndex) as MetricResourceKind; - const id = key.slice(colonIndex + 1); - - return { kind, id }; -} - /** * Extract the prefix from a metric resource kind * Used for bulk operations on a specific resource type diff --git a/internal/logging/logging.go b/internal/logging/logging.go index 0fbb50c..f527156 100644 --- a/internal/logging/logging.go +++ b/internal/logging/logging.go @@ -7,7 +7,6 @@ import ( "io" "os" "path/filepath" - "strconv" "strings" "sync" "time" @@ -23,7 +22,6 @@ type ctxKey string const ( requestIDKey ctxKey = "logging_request_id" - loggerKey ctxKey = "logging_logger" ) // Config controls logger initialization. @@ -37,15 +35,6 @@ type Config struct { Compress bool // gzip rotated logs } -// Option customizes logger construction. -type Option func(*options) - -type options struct { - writer io.Writer - fields map[string]interface{} - withCaller bool -} - var ( mu sync.RWMutex baseLogger zerolog.Logger @@ -90,148 +79,11 @@ func Init(cfg Config) zerolog.Logger { return baseLogger } -// InitFromConfig initialises logging with environment overrides. -func InitFromConfig(ctx context.Context, cfg Config) (zerolog.Logger, error) { - if cfg.Level == "" { - cfg.Level = "info" - } - if cfg.Format == "" { - cfg.Format = "auto" - } - - if envLevel := os.Getenv("LOG_LEVEL"); envLevel != "" { - cfg.Level = envLevel - } - if envFormat := os.Getenv("LOG_FORMAT"); envFormat != "" { - cfg.Format = envFormat - } - if envFile := os.Getenv("LOG_FILE"); envFile != "" { - cfg.FilePath = envFile - } - if envSize := os.Getenv("LOG_MAX_SIZE"); envSize != "" { - if size, err := strconv.Atoi(envSize); err == nil { - cfg.MaxSizeMB = size - } - } - if envAge := os.Getenv("LOG_MAX_AGE"); envAge != "" { - if age, err := strconv.Atoi(envAge); err == nil { - cfg.MaxAgeDays = age - } - } - if envCompress := os.Getenv("LOG_COMPRESS"); envCompress != "" { - switch strings.ToLower(strings.TrimSpace(envCompress)) { - case "0", "false", "no": - cfg.Compress = false - default: - cfg.Compress = true - } - } - - if !isValidLevel(cfg.Level) { - return zerolog.Logger{}, fmt.Errorf("invalid log level %q: must be debug, info, warn, or error", cfg.Level) - } - - format := strings.ToLower(strings.TrimSpace(cfg.Format)) - if format != "" && format != "json" && format != "console" && format != "auto" { - return zerolog.Logger{}, fmt.Errorf("invalid log format %q: must be json, console, or auto", cfg.Format) - } - - logger := Init(cfg) - return logger, nil -} - // IsLevelEnabled reports whether the provided level is enabled for logging. func IsLevelEnabled(level zerolog.Level) bool { return level >= zerolog.GlobalLevel() } -// New creates a logger tailored to a specific component. -func New(component string, opts ...Option) zerolog.Logger { - cfg := collectOptions(opts...) - - mu.RLock() - globalWriter := baseWriter - globalComponent := baseComponent - mu.RUnlock() - - writer := cfg.writer - if writer == nil { - writer = globalWriter - } - - component = strings.TrimSpace(component) - if component == "" { - component = globalComponent - } - - logger := zerolog.New(writer) - contextBuilder := logger.With().Timestamp() - - if component != "" { - contextBuilder = contextBuilder.Str("component", component) - } - if len(cfg.fields) > 0 { - contextBuilder = contextBuilder.Fields(cfg.fields) - } - if cfg.withCaller { - contextBuilder = contextBuilder.Caller() - } - - return contextBuilder.Logger() -} - -// WithCaller enables caller logging. -func WithCaller() Option { - return func(o *options) { - o.withCaller = true - } -} - -// WithWriter overrides the logger writer. -func WithWriter(w io.Writer) Option { - return func(o *options) { - if w != nil { - o.writer = w - } - } -} - -// WithFields adds static fields to the logger. -func WithFields(fields map[string]interface{}) Option { - return func(o *options) { - if len(fields) == 0 { - return - } - if o.fields == nil { - o.fields = make(map[string]interface{}, len(fields)) - } - for k, v := range fields { - o.fields[k] = v - } - } -} - -// FromContext returns a logger enriched with context metadata. -func FromContext(ctx context.Context) zerolog.Logger { - if ctx == nil { - return getBaseLogger() - } - - if existing, ok := ctx.Value(loggerKey).(zerolog.Logger); ok { - return enrichWithRequestID(existing, ctx) - } - - return enrichWithRequestID(getBaseLogger(), ctx) -} - -// WithLogger stores a logger on the context. -func WithLogger(ctx context.Context, logger zerolog.Logger) context.Context { - if ctx == nil { - ctx = context.Background() - } - return context.WithValue(ctx, loggerKey, logger) -} - // WithRequestID stores (or generates) a request ID on the context. func WithRequestID(ctx context.Context, id string) (context.Context, string) { if ctx == nil { @@ -244,27 +96,6 @@ func WithRequestID(ctx context.Context, id string) (context.Context, string) { return context.WithValue(ctx, requestIDKey, id), id } -// GetRequestID retrieves the request ID from context. -func GetRequestID(ctx context.Context) string { - if ctx == nil { - return "" - } - if id, ok := ctx.Value(requestIDKey).(string); ok { - return id - } - return "" -} - -func collectOptions(opts ...Option) options { - cfg := options{} - for _, opt := range opts { - if opt != nil { - opt(&cfg) - } - } - return cfg -} - func parseLevel(level string) zerolog.Level { switch strings.ToLower(strings.TrimSpace(level)) { case "debug": @@ -278,15 +109,6 @@ func parseLevel(level string) zerolog.Level { } } -func isValidLevel(level string) bool { - switch strings.ToLower(strings.TrimSpace(level)) { - case "debug", "info", "warn", "error": - return true - default: - return false - } -} - func selectWriter(format string) io.Writer { format = strings.ToLower(strings.TrimSpace(format)) switch format { @@ -318,22 +140,6 @@ func isTerminal(file *os.File) bool { return term.IsTerminal(int(file.Fd())) } -func getBaseLogger() zerolog.Logger { - mu.RLock() - defer mu.RUnlock() - return baseLogger -} - -func enrichWithRequestID(logger zerolog.Logger, ctx context.Context) zerolog.Logger { - if ctx == nil { - return logger - } - if id := GetRequestID(ctx); id != "" { - return logger.With().Str("request_id", id).Logger() - } - return logger -} - type rollingFileWriter struct { mu sync.Mutex path string diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go index af1ae83..09d4aff 100644 --- a/internal/logging/logging_test.go +++ b/internal/logging/logging_test.go @@ -2,12 +2,8 @@ package logging import ( "bytes" - "context" - "encoding/json" - "io" "os" "reflect" - "strings" "sync" "testing" @@ -27,24 +23,6 @@ func resetLoggingState() { zerolog.SetGlobalLevel(zerolog.InfoLevel) } -func readJSONLine(t *testing.T, buf *bytes.Buffer) map[string]interface{} { - t.Helper() - - line := strings.TrimSpace(buf.String()) - if idx := strings.IndexByte(line, '\n'); idx >= 0 { - line = line[:idx] - } - if line == "" { - t.Fatalf("expected log output, got empty string") - } - - var event map[string]interface{} - if err := json.Unmarshal([]byte(line), &event); err != nil { - t.Fatalf("failed to unmarshal log line: %v", err) - } - return event -} - func TestInitJSONFormatSetsLevelAndComponent(t *testing.T) { t.Cleanup(resetLoggingState) @@ -118,58 +96,7 @@ func TestInitAutoFormatWithPipe(t *testing.T) { } } -func TestNewLoggerWithComponentAndFields(t *testing.T) { - t.Cleanup(resetLoggingState) - - Init(Config{ - Format: "json", - Level: "info", - Component: "root", - }) - - var buf bytes.Buffer - logger := New("worker", WithWriter(&buf), WithFields(map[string]interface{}{ - "request": "sync", - })) - - logger.Info().Msg("processing") - - event := readJSONLine(t, &buf) - - if event["component"] != "worker" { - t.Fatalf("expected component worker, got %v", event["component"]) - } - if event["request"] != "sync" { - t.Fatalf("expected request field, got %v", event["request"]) - } - if event["level"] != "info" { - t.Fatalf("expected level info, got %v", event["level"]) - } - if event["message"] != "processing" { - t.Fatalf("expected message processing, got %v", event["message"]) - } -} - -func TestNewLoggerInheritsComponentWhenEmpty(t *testing.T) { - t.Cleanup(resetLoggingState) - - Init(Config{ - Format: "json", - Level: "info", - Component: "core", - }) - - var buf bytes.Buffer - logger := New("", WithWriter(&buf)) - logger.Warn().Msg("warn") - - event := readJSONLine(t, &buf) - if event["component"] != "core" { - t.Fatalf("expected inherited component core, got %v", event["component"]) - } -} - -func TestNewLoggerWithCaller(t *testing.T) { +func TestWithRequestID(t *testing.T) { t.Cleanup(resetLoggingState) Init(Config{ @@ -177,104 +104,21 @@ func TestNewLoggerWithCaller(t *testing.T) { Level: "info", }) - var buf bytes.Buffer - logger := New("svc", WithWriter(&buf), WithCaller()) - logger.Error().Msg("boom") - - event := readJSONLine(t, &buf) - caller, ok := event["caller"].(string) - if !ok || !strings.Contains(caller, "logging_test.go") { - t.Fatalf("expected caller information, got %v", event["caller"]) - } -} - -func TestNewLoggerWithCustomWriter(t *testing.T) { - t.Cleanup(resetLoggingState) - - Init(Config{ - Format: "json", - Level: "info", - }) - - var buf bytes.Buffer - logger := New("custom", WithWriter(&buf)) - logger.Info().Msg("hello") - - if buf.Len() == 0 { - t.Fatal("expected output on custom writer") - } -} - -func TestContextHelpersWithRequestID(t *testing.T) { - t.Cleanup(resetLoggingState) - - Init(Config{ - Format: "json", - Level: "info", - }) - - ctx := context.Background() - ctx, generated := WithRequestID(ctx, "") + ctx, generated := WithRequestID(nil, "") if generated == "" { t.Fatal("expected generated request id") } - if got := GetRequestID(ctx); got != generated { - t.Fatalf("expected stored request id %s, got %s", generated, got) - } - - var buf bytes.Buffer - logger := New("api", WithWriter(&buf)) - ctx = WithLogger(ctx, logger) - - info := FromContext(ctx) - info.Info().Msg("ctx-log") - - event := readJSONLine(t, &buf) - if event["request_id"] != generated { - t.Fatalf("expected request_id %s, got %v", generated, event["request_id"]) - } -} - -func TestContextHelpersWithExistingLogger(t *testing.T) { - t.Cleanup(resetLoggingState) - - Init(Config{ - Format: "json", - Level: "debug", - }) - - var buf bytes.Buffer - base := New("svc", WithWriter(&buf)) - ctx := WithLogger(context.Background(), base) - ctx, id := WithRequestID(ctx, "custom-123") - - logger := FromContext(ctx) - logger.Debug().Msg("debug") - - event := readJSONLine(t, &buf) - if event["component"] != "svc" { - t.Fatalf("expected component svc, got %v", event["component"]) - } - if event["request_id"] != "custom-123" { - t.Fatalf("expected request_id custom-123, got %v", event["request_id"]) - } - if event["level"] != "debug" { - t.Fatalf("expected level debug, got %v", event["level"]) - } - if id != "custom-123" { - t.Fatalf("expected returned id to match input, got %s", id) - } -} - -func TestWithLoggerNilContext(t *testing.T) { - t.Cleanup(resetLoggingState) - - Init(Config{}) - - ctx := WithLogger(context.Background(), New("svc", WithWriter(io.Discard))) if ctx == nil { t.Fatal("expected non-nil context") } + + ctx2, id := WithRequestID(nil, "custom-123") + if id != "custom-123" { + t.Fatalf("expected custom-123, got %s", id) + } + if ctx2 == nil { + t.Fatal("expected non-nil context") + } } func TestWithRequestIDTrimsWhitespace(t *testing.T) { @@ -282,56 +126,10 @@ func TestWithRequestIDTrimsWhitespace(t *testing.T) { Init(Config{}) - ctx, id := WithRequestID(context.Background(), " ") + _, id := WithRequestID(nil, " ") if id == "" { t.Fatal("expected generated id for whitespace input") } - if GetRequestID(ctx) != id { - t.Fatalf("expected context request id %s, got %s", id, GetRequestID(ctx)) - } -} - -func TestFromContextWithoutRequestID(t *testing.T) { - t.Cleanup(resetLoggingState) - - Init(Config{ - Format: "json", - Level: "info", - }) - - var buf bytes.Buffer - mu.Lock() - baseLogger = zerolog.New(&buf).With().Timestamp().Logger() - baseWriter = &buf - baseComponent = "" - log.Logger = baseLogger - mu.Unlock() - - base := FromContext(context.Background()) - base.Info().Msg("no-request") - - event := readJSONLine(t, &buf) - if _, ok := event["request_id"]; ok { - t.Fatalf("did not expect request_id, got %v", event["request_id"]) - } -} - -func TestNewLoggerWithoutComponentOmitsField(t *testing.T) { - t.Cleanup(resetLoggingState) - - Init(Config{ - Format: "json", - Level: "info", - }) - - var buf bytes.Buffer - logger := New("", WithWriter(&buf)) - logger.Info().Msg("no-component") - - event := readJSONLine(t, &buf) - if _, exists := event["component"]; exists { - t.Fatalf("did not expect component field, got %v", event["component"]) - } } func TestInitThreadSafety(t *testing.T) { @@ -364,91 +162,6 @@ func TestInitThreadSafety(t *testing.T) { } } -func TestInitFromConfigWithDefaults(t *testing.T) { - t.Cleanup(resetLoggingState) - - logger, err := InitFromConfig(context.Background(), Config{ - Component: "test", - }) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - if reflect.DeepEqual(logger, zerolog.Logger{}) { - t.Fatal("expected initialized logger") - } - - if zerolog.GlobalLevel() != zerolog.InfoLevel { - t.Fatalf("expected default info level, got %s", zerolog.GlobalLevel()) - } -} - -func TestInitFromConfigWithEnvOverrides(t *testing.T) { - t.Cleanup(resetLoggingState) - t.Cleanup(func() { - os.Unsetenv("LOG_LEVEL") - os.Unsetenv("LOG_FORMAT") - }) - - os.Setenv("LOG_LEVEL", "debug") - os.Setenv("LOG_FORMAT", "json") - - logger, err := InitFromConfig(context.Background(), Config{ - Level: "info", - Format: "console", - Component: "test", - }) - if err != nil { - t.Fatalf("expected no error, got %v", err) - } - - if reflect.DeepEqual(logger, zerolog.Logger{}) { - t.Fatal("expected initialized logger") - } - - // Env override should set debug level - if zerolog.GlobalLevel() != zerolog.DebugLevel { - t.Fatalf("expected debug level from env override, got %s", zerolog.GlobalLevel()) - } - - // Format should be JSON (from env) - mu.RLock() - defer mu.RUnlock() - if _, ok := baseWriter.(zerolog.ConsoleWriter); ok { - t.Fatal("expected JSON writer from env override, got console writer") - } -} - -func TestInitFromConfigInvalidLevel(t *testing.T) { - t.Cleanup(resetLoggingState) - - _, err := InitFromConfig(context.Background(), Config{ - Level: "invalid", - Format: "json", - }) - if err == nil { - t.Fatal("expected error for invalid level") - } - if !strings.Contains(err.Error(), "invalid log level") { - t.Fatalf("expected invalid level error, got %v", err) - } -} - -func TestInitFromConfigInvalidFormat(t *testing.T) { - t.Cleanup(resetLoggingState) - - _, err := InitFromConfig(context.Background(), Config{ - Level: "info", - Format: "invalid", - }) - if err == nil { - t.Fatal("expected error for invalid format") - } - if !strings.Contains(err.Error(), "invalid log format") { - t.Fatalf("expected invalid format error, got %v", err) - } -} - func TestIsLevelEnabled(t *testing.T) { t.Cleanup(resetLoggingState) @@ -474,3 +187,108 @@ func TestIsLevelEnabled(t *testing.T) { t.Fatal("expected debug level to be enabled after setting global level") } } + +func TestRollingFileWriter(t *testing.T) { + t.Cleanup(resetLoggingState) + + dir := t.TempDir() + logFile := dir + "/test.log" + + cfg := Config{ + Format: "json", + Level: "info", + FilePath: logFile, + MaxSizeMB: 1, + MaxAgeDays: 7, + Compress: false, + } + + Init(cfg) + + // Write some log output + log.Info().Msg("test message") + + // Check file exists (this confirms rolling file writer was created) + if _, err := os.Stat(logFile); os.IsNotExist(err) { + t.Fatal("expected log file to be created") + } + + // Check file has content + data, err := os.ReadFile(logFile) + if err != nil { + t.Fatalf("failed to read log file: %v", err) + } + if len(data) == 0 { + t.Fatal("expected log file to have content") + } +} + +func TestParseLevelDefaults(t *testing.T) { + tests := []struct { + input string + want zerolog.Level + }{ + {"debug", zerolog.DebugLevel}, + {"DEBUG", zerolog.DebugLevel}, + {"info", zerolog.InfoLevel}, + {"INFO", zerolog.InfoLevel}, + {"warn", zerolog.WarnLevel}, + {"WARN", zerolog.WarnLevel}, + {"error", zerolog.ErrorLevel}, + {"ERROR", zerolog.ErrorLevel}, + {"unknown", zerolog.InfoLevel}, + {"", zerolog.InfoLevel}, + } + + for _, tc := range tests { + got := parseLevel(tc.input) + if got != tc.want { + t.Errorf("parseLevel(%q) = %v, want %v", tc.input, got, tc.want) + } + } +} + +func TestSelectWriter(t *testing.T) { + tests := []struct { + format string + isTTY bool + wantErr bool + }{ + {"json", false, false}, + {"console", false, false}, + {"auto", false, false}, + } + + for _, tc := range tests { + w := selectWriter(tc.format) + if w == nil { + t.Errorf("selectWriter(%q) returned nil", tc.format) + } + } +} + +// Test that the logging package doesn't panic under concurrent use +func TestConcurrentLogging(t *testing.T) { + t.Cleanup(resetLoggingState) + + var buf bytes.Buffer + mu.Lock() + baseWriter = &buf + baseLogger = zerolog.New(&buf).With().Timestamp().Logger() + log.Logger = baseLogger + mu.Unlock() + + var wg sync.WaitGroup + for i := 0; i < 100; i++ { + wg.Add(1) + go func(n int) { + defer wg.Done() + log.Info().Int("iteration", n).Msg("concurrent log") + }(i) + } + wg.Wait() + + if buf.Len() == 0 { + t.Fatal("expected log output from concurrent logging") + } +} diff --git a/internal/metrics/alert_metrics.go b/internal/metrics/alert_metrics.go index a8a4409..5bab05e 100644 --- a/internal/metrics/alert_metrics.go +++ b/internal/metrics/alert_metrics.go @@ -1,8 +1,6 @@ package metrics import ( - "fmt" - "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/rcourtman/pulse-go-rewrite/internal/alerts" @@ -50,87 +48,6 @@ var ( []string{"type"}, ) - // Notification metrics - NotificationsSentTotal = promauto.NewCounterVec( - prometheus.CounterOpts{ - Name: "pulse_notifications_sent_total", - Help: "Total number of notifications sent by method and status", - }, - []string{"method", "status"}, // method: email, webhook, apprise; status: success, failure - ) - - NotificationQueueDepth = promauto.NewGaugeVec( - prometheus.GaugeOpts{ - Name: "pulse_notification_queue_depth", - Help: "Number of notifications in queue by status", - }, - []string{"status"}, // pending, sending, dlq - ) - - NotificationDeliveryDuration = promauto.NewHistogramVec( - prometheus.HistogramOpts{ - Name: "pulse_notification_delivery_duration_seconds", - Help: "Time to deliver notifications", - Buckets: prometheus.DefBuckets, - }, - []string{"method"}, - ) - - NotificationRetriesTotal = promauto.NewCounterVec( - prometheus.CounterOpts{ - Name: "pulse_notification_retries_total", - Help: "Total number of notification retry attempts", - }, - []string{"method"}, - ) - - NotificationDLQTotal = promauto.NewCounter( - prometheus.CounterOpts{ - Name: "pulse_notification_dlq_total", - Help: "Total number of notifications moved to dead letter queue", - }, - ) - - // Alert grouping metrics - NotificationsGroupedTotal = promauto.NewCounter( - prometheus.CounterOpts{ - Name: "pulse_notifications_grouped_total", - Help: "Total number of grouped notifications sent", - }, - ) - - AlertsGroupedCount = promauto.NewHistogram( - prometheus.HistogramOpts{ - Name: "pulse_alerts_grouped_count", - Help: "Number of alerts in grouped notifications", - Buckets: []float64{1, 2, 5, 10, 20, 50, 100}, - }, - ) - - // Escalation metrics - AlertEscalationsTotal = promauto.NewCounterVec( - prometheus.CounterOpts{ - Name: "pulse_alert_escalations_total", - Help: "Total number of alert escalations by level", - }, - []string{"level"}, - ) - - // History metrics - AlertHistorySize = promauto.NewGauge( - prometheus.GaugeOpts{ - Name: "pulse_alert_history_size", - Help: "Number of alerts in history", - }, - ) - - AlertHistorySaveErrors = promauto.NewCounter( - prometheus.CounterOpts{ - Name: "pulse_alert_history_save_errors_total", - Help: "Total number of alert history save failures", - }, - ) - // Suppression and quiet hours metrics AlertsSuppressedTotal = promauto.NewCounterVec( prometheus.CounterOpts{ @@ -169,52 +86,7 @@ func RecordAlertAcknowledged() { AlertsAcknowledgedTotal.Inc() } -// RecordNotificationSent records a notification delivery attempt -func RecordNotificationSent(method string, success bool) { - status := "success" - if !success { - status = "failure" - } - NotificationsSentTotal.WithLabelValues(method, status).Inc() -} - -// RecordNotificationRetry records a notification retry -func RecordNotificationRetry(method string) { - NotificationRetriesTotal.WithLabelValues(method).Inc() -} - -// RecordNotificationDLQ records a notification moved to DLQ -func RecordNotificationDLQ() { - NotificationDLQTotal.Inc() -} - -// RecordGroupedNotification records a grouped notification -func RecordGroupedNotification(alertCount int) { - NotificationsGroupedTotal.Inc() - AlertsGroupedCount.Observe(float64(alertCount)) -} - -// RecordAlertEscalation records an alert escalation -func RecordAlertEscalation(level int) { - AlertEscalationsTotal.WithLabelValues(fmt.Sprintf("%d", level)).Inc() -} - // RecordAlertSuppressed records a suppressed alert func RecordAlertSuppressed(reason string) { AlertsSuppressedTotal.WithLabelValues(reason).Inc() } - -// UpdateQueueDepth updates the notification queue depth metric -func UpdateQueueDepth(status string, count int) { - NotificationQueueDepth.WithLabelValues(status).Set(float64(count)) -} - -// UpdateHistorySize updates the alert history size metric -func UpdateHistorySize(size int) { - AlertHistorySize.Set(float64(size)) -} - -// RecordHistorySaveError records a history save failure -func RecordHistorySaveError() { - AlertHistorySaveErrors.Inc() -} diff --git a/internal/metrics/alert_metrics_test.go b/internal/metrics/alert_metrics_test.go index 9c0db6c..b0e50bd 100644 --- a/internal/metrics/alert_metrics_test.go +++ b/internal/metrics/alert_metrics_test.go @@ -38,40 +38,6 @@ func TestRecordAlertAcknowledged(t *testing.T) { RecordAlertAcknowledged() } -func TestRecordNotificationSent_Success(t *testing.T) { - // Should not panic - RecordNotificationSent("email", true) -} - -func TestRecordNotificationSent_Failure(t *testing.T) { - // Should not panic - RecordNotificationSent("webhook", false) -} - -func TestRecordNotificationRetry(t *testing.T) { - // Should not panic - RecordNotificationRetry("email") -} - -func TestRecordNotificationDLQ(t *testing.T) { - // Should not panic - RecordNotificationDLQ() -} - -func TestRecordGroupedNotification(t *testing.T) { - // Should not panic with various counts - RecordGroupedNotification(1) - RecordGroupedNotification(5) - RecordGroupedNotification(50) -} - -func TestRecordAlertEscalation(t *testing.T) { - // Should not panic with various levels - RecordAlertEscalation(1) - RecordAlertEscalation(2) - RecordAlertEscalation(3) -} - func TestRecordAlertSuppressed(t *testing.T) { // Should not panic with various reasons RecordAlertSuppressed("quiet_hours") @@ -79,25 +45,6 @@ func TestRecordAlertSuppressed(t *testing.T) { RecordAlertSuppressed("duplicate") } -func TestUpdateQueueDepth(t *testing.T) { - // Should not panic with various statuses and counts - UpdateQueueDepth("pending", 10) - UpdateQueueDepth("sending", 5) - UpdateQueueDepth("dlq", 0) -} - -func TestUpdateHistorySize(t *testing.T) { - // Should not panic with various sizes - UpdateHistorySize(0) - UpdateHistorySize(100) - UpdateHistorySize(1000) -} - -func TestRecordHistorySaveError(t *testing.T) { - // Should not panic - RecordHistorySaveError() -} - func TestMetricVectors_NotNil(t *testing.T) { // Verify that metric vectors are properly initialized if AlertsActive == nil { @@ -115,36 +62,6 @@ func TestMetricVectors_NotNil(t *testing.T) { if AlertDurationSeconds == nil { t.Error("AlertDurationSeconds should not be nil") } - if NotificationsSentTotal == nil { - t.Error("NotificationsSentTotal should not be nil") - } - if NotificationQueueDepth == nil { - t.Error("NotificationQueueDepth should not be nil") - } - if NotificationDeliveryDuration == nil { - t.Error("NotificationDeliveryDuration should not be nil") - } - if NotificationRetriesTotal == nil { - t.Error("NotificationRetriesTotal should not be nil") - } - if NotificationDLQTotal == nil { - t.Error("NotificationDLQTotal should not be nil") - } - if NotificationsGroupedTotal == nil { - t.Error("NotificationsGroupedTotal should not be nil") - } - if AlertsGroupedCount == nil { - t.Error("AlertsGroupedCount should not be nil") - } - if AlertEscalationsTotal == nil { - t.Error("AlertEscalationsTotal should not be nil") - } - if AlertHistorySize == nil { - t.Error("AlertHistorySize should not be nil") - } - if AlertHistorySaveErrors == nil { - t.Error("AlertHistorySaveErrors should not be nil") - } if AlertsSuppressedTotal == nil { t.Error("AlertsSuppressedTotal should not be nil") }