From 50f77da13153954b6fbacc02945b76d8062302ad Mon Sep 17 00:00:00 2001 From: Tyler Koenig Date: Sat, 27 Jun 2026 15:44:03 -0400 Subject: [PATCH] refactor: extract magic numbers to named constants Replace inline numeric literals with named constants across 14 files: server timeouts/rate limits, cluster thresholds/intervals, DB pool sizes, alert/dial timeouts, TUI uptime thresholds, node status thresholds, and state history limits. --- internal/alert/alert.go | 4 +++- internal/cluster/cluster.go | 14 ++++++++++---- internal/cluster/probe.go | 12 +++++++++--- internal/monitor/monitor.go | 5 +++-- internal/monitor/safedial.go | 4 +++- internal/server/ratelimit.go | 9 +++++++-- internal/server/server.go | 28 ++++++++++++++++++++-------- internal/store/sqlstore.go | 12 ++++++++---- internal/tui/const.go | 20 ++++++++++++++++++++ internal/tui/data.go | 6 +++--- internal/tui/errclass.go | 16 ++++++++-------- internal/tui/tab_nodes.go | 6 +++--- internal/tui/uptime_timeline.go | 8 ++++---- internal/tui/view_dashboard.go | 4 ++-- internal/tui/view_sla.go | 10 +++++----- 15 files changed, 108 insertions(+), 50 deletions(-) create mode 100644 internal/tui/const.go diff --git a/internal/alert/alert.go b/internal/alert/alert.go index 4e8e348..12bf2b2 100644 --- a/internal/alert/alert.go +++ b/internal/alert/alert.go @@ -18,7 +18,9 @@ import ( "gitea.lerkolabs.com/lerkolabs/uptop/internal/models" ) -var alertClient = &http.Client{Timeout: 10 * time.Second} +const alertHTTPTimeout = 10 * time.Second + +var alertClient = &http.Client{Timeout: alertHTTPTimeout} // sanitizeError strips the request URL from transport errors before they are // stored or displayed. *url.Error embeds the full URL, which for several diff --git a/internal/cluster/cluster.go b/internal/cluster/cluster.go index 382aad5..223d2c9 100644 --- a/internal/cluster/cluster.go +++ b/internal/cluster/cluster.go @@ -38,14 +38,20 @@ func Start(ctx context.Context, cfg Config, eng *monitor.Engine) { // "probe" mode is handled directly in main.go before cluster.Start is called } +const ( + followerTimeout = 2 * time.Second + leaderFailureThreshold = 3 + followerRetryInterval = 5 * time.Second +) + func runFollowerLoop(ctx context.Context, cfg Config, eng *monitor.Engine) { - client := http.Client{Timeout: 2 * time.Second} + client := http.Client{Timeout: followerTimeout} failures := 0 - threshold := 3 + threshold := leaderFailureThreshold for { select { - case <-time.After(5 * time.Second): + case <-time.After(followerRetryInterval): case <-ctx.Done(): return } @@ -59,7 +65,7 @@ func runFollowerLoop(ctx context.Context, cfg Config, eng *monitor.Engine) { isLeaderHealthy := false if err == nil { - isLeaderHealthy = resp.StatusCode == 200 + isLeaderHealthy = resp.StatusCode == http.StatusOK _ = resp.Body.Close() } diff --git a/internal/cluster/probe.go b/internal/cluster/probe.go index 44a8105..55aa0a5 100644 --- a/internal/cluster/probe.go +++ b/internal/cluster/probe.go @@ -26,12 +26,18 @@ type ProbeConfig struct { AllowPrivateTargets bool } +const ( + probeMinInterval = 10 + probeDefaultInterval = 30 + probeAPITimeout = 10 * time.Second +) + func RunProbe(ctx context.Context, cfg ProbeConfig) error { - if cfg.Interval < 10 { - cfg.Interval = 30 + if cfg.Interval < probeMinInterval { + cfg.Interval = probeDefaultInterval } - apiClient := &http.Client{Timeout: 10 * time.Second} + apiClient := &http.Client{Timeout: probeAPITimeout} dial := monitor.SafeDialContext(cfg.AllowPrivateTargets) strictClient := &http.Client{ Transport: &http.Transport{ diff --git a/internal/monitor/monitor.go b/internal/monitor/monitor.go index 929f99e..04c4caf 100644 --- a/internal/monitor/monitor.go +++ b/internal/monitor/monitor.go @@ -24,6 +24,7 @@ const ( maintPruneInterval = 15 * time.Minute defaultMaintRetention = 7 * 24 * time.Hour dbWriteBuffer = 4096 + alertSendTimeout = 30 * time.Second dbPruneInterval = 10 * time.Minute ) @@ -900,7 +901,7 @@ func (e *Engine) triggerAlert(alertID int, title, message string) { provider := alert.GetProvider(cfg) if provider != nil { go func() { - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), alertSendTimeout) defer cancel() if err := provider.Send(ctx, title, message); err != nil { e.AddLog(fmt.Sprintf("Alert send failed (%s): %v", cfg.Name, err)) @@ -953,7 +954,7 @@ func (e *Engine) TestAlert(alertID int) error { if provider == nil { return fmt.Errorf("no provider for type %q", cfg.Type) } - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), alertSendTimeout) defer cancel() err = provider.Send(ctx, "🧪 Test Alert", fmt.Sprintf("Test notification from uptop for channel '%s'.", cfg.Name)) if err != nil { diff --git a/internal/monitor/safedial.go b/internal/monitor/safedial.go index 5e85ffa..6311051 100644 --- a/internal/monitor/safedial.go +++ b/internal/monitor/safedial.go @@ -7,6 +7,8 @@ import ( "time" ) +const dialTimeout = 10 * time.Second + var privateRanges []*net.IPNet func init() { @@ -60,7 +62,7 @@ func SafeDialContext(allowPrivate bool) func(ctx context.Context, network, addr } } - dialer := &net.Dialer{Timeout: 10 * time.Second} + dialer := &net.Dialer{Timeout: dialTimeout} for _, ip := range ips { target := net.JoinHostPort(ip.IP.String(), port) conn, err := dialer.DialContext(ctx, network, target) diff --git a/internal/server/ratelimit.go b/internal/server/ratelimit.go index e963b26..74351a6 100644 --- a/internal/server/ratelimit.go +++ b/internal/server/ratelimit.go @@ -14,6 +14,11 @@ import ( // guard. const maxVisitors = 10000 +const ( + visitorCleanupInterval = 5 * time.Minute + visitorIdleCutoff = 10 * time.Minute +) + type visitor struct { tokens float64 lastSeen time.Time @@ -90,13 +95,13 @@ func (rl *RateLimiter) evictOldest() { } func (rl *RateLimiter) cleanup() { - ticker := time.NewTicker(5 * time.Minute) + ticker := time.NewTicker(visitorCleanupInterval) defer ticker.Stop() for { select { case <-ticker.C: rl.mu.Lock() - cutoff := time.Now().Add(-10 * time.Minute) + cutoff := time.Now().Add(-visitorIdleCutoff) for ip, v := range rl.visitors { if v.lastSeen.Before(cutoff) { delete(rl.visitors, ip) diff --git a/internal/server/server.go b/internal/server/server.go index 6f349c8..8fa9f16 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -45,15 +45,27 @@ type Server struct { statusRL *RateLimiter } +const ( + pushRateLimit = 60 + probeRateLimit = 30 + backupRateLimit = 10 + statusRateLimit = 120 + + httpReadHeaderTimeout = 10 * time.Second + httpReadTimeout = 30 * time.Second + httpWriteTimeout = 60 * time.Second + httpIdleTimeout = 120 * time.Second +) + func NewServer(cfg ServerConfig, s store.Store, eng *monitor.Engine) *Server { return &Server{ cfg: cfg, store: s, eng: eng, - pushRL: NewRateLimiter(60, cfg.TrustedProxies), - probeRL: NewRateLimiter(30, cfg.TrustedProxies), - backupRL: NewRateLimiter(10, cfg.TrustedProxies), - statusRL: NewRateLimiter(120, cfg.TrustedProxies), + pushRL: NewRateLimiter(pushRateLimit, cfg.TrustedProxies), + probeRL: NewRateLimiter(probeRateLimit, cfg.TrustedProxies), + backupRL: NewRateLimiter(backupRateLimit, cfg.TrustedProxies), + statusRL: NewRateLimiter(statusRateLimit, cfg.TrustedProxies), } } @@ -77,10 +89,10 @@ func (s *Server) Start() *http.Server { httpSrv := &http.Server{ Addr: addr, Handler: handler, - ReadHeaderTimeout: 10 * time.Second, - ReadTimeout: 30 * time.Second, - WriteTimeout: 60 * time.Second, - IdleTimeout: 120 * time.Second, + ReadHeaderTimeout: httpReadHeaderTimeout, + ReadTimeout: httpReadTimeout, + WriteTimeout: httpWriteTimeout, + IdleTimeout: httpIdleTimeout, } go func() { if s.cfg.TLSCert != "" && s.cfg.TLSKey != "" { diff --git a/internal/store/sqlstore.go b/internal/store/sqlstore.go index 1ba7c3d..c803f2c 100644 --- a/internal/store/sqlstore.go +++ b/internal/store/sqlstore.go @@ -18,6 +18,10 @@ const ( maxLogRows = 200 maxStateChangesPerSite = 5000 maxMaintenanceExport = 1000 + maxOpenConns = 25 + maxIdleConns = 5 + connMaxLifetime = 5 * time.Minute + tokenByteLen = 16 ) type SQLStore struct { @@ -32,9 +36,9 @@ func NewSQLStore(driverName, dsn string, dialect Dialect) (*SQLStore, error) { if err != nil { return nil, err } - db.SetMaxOpenConns(25) - db.SetMaxIdleConns(5) - db.SetConnMaxLifetime(5 * time.Minute) + db.SetMaxOpenConns(maxOpenConns) + db.SetMaxIdleConns(maxIdleConns) + db.SetConnMaxLifetime(connMaxLifetime) _, isDollar := dialect.(*PostgresDialect) return &SQLStore{db: db, dialect: dialect, dollar: isDollar}, nil } @@ -62,7 +66,7 @@ func (s *SQLStore) q(query string) string { } func generateToken() (string, error) { - b := make([]byte, 16) + b := make([]byte, tokenByteLen) if _, err := rand.Read(b); err != nil { return "", fmt.Errorf("crypto/rand failed: %w", err) } diff --git a/internal/tui/const.go b/internal/tui/const.go new file mode 100644 index 0000000..0999e88 --- /dev/null +++ b/internal/tui/const.go @@ -0,0 +1,20 @@ +package tui + +import "time" + +const ( + nodeOnlineThreshold = 60 * time.Second + nodeStaleThreshold = 5 * time.Minute + + uptimeGoodPct = 99.0 + uptimeExcellentPct = 99.9 + uptimeWarnPct = 95.0 + uptimePrecisionPct = 99.99 + + stateHistoryLookback = 30 * 24 * time.Hour + stateHistoryDays = 30 + stateHistoryLimit = 100 + + httpErrorThreshold = 400 + errorDetailMaxLen = 30 +) diff --git a/internal/tui/data.go b/internal/tui/data.go index c27a642..5731134 100644 --- a/internal/tui/data.go +++ b/internal/tui/data.go @@ -202,8 +202,8 @@ func (m *Model) loadDetailCmd(siteID int) tea.Cmd { return func() tea.Msg { changes := eng.GetStateChanges(siteID, 5) now := time.Now() - allChanges := eng.GetStateChangesSince(siteID, now.Add(-30*24*time.Hour)) - daily := monitor.ComputeDailyBreakdown(allChanges, currentStatus, 30, now) + allChanges := eng.GetStateChangesSince(siteID, now.Add(-stateHistoryLookback)) + daily := monitor.ComputeDailyBreakdown(allChanges, currentStatus, stateHistoryDays, now) return detailDataMsg{siteID: siteID, changes: changes, dailyDays: daily} } } @@ -213,7 +213,7 @@ func (m *Model) loadDetailCmd(siteID int) tea.Cmd { func (m *Model) loadHistoryCmd(siteID int) tea.Cmd { eng := m.engine return func() tea.Msg { - return historyDataMsg{siteID: siteID, changes: eng.GetStateChanges(siteID, 100)} + return historyDataMsg{siteID: siteID, changes: eng.GetStateChanges(siteID, stateHistoryLimit)} } } diff --git a/internal/tui/errclass.go b/internal/tui/errclass.go index 7b505bc..566a0aa 100644 --- a/internal/tui/errclass.go +++ b/internal/tui/errclass.go @@ -58,7 +58,7 @@ func classifyError(errorReason string, siteType string, statusCode int) ErrorCat if strings.HasPrefix(lower, "http ") || strings.Contains(lower, "keyword not found") { return ErrCatHTTP } - if statusCode >= 400 { + if statusCode >= httpErrorThreshold { return ErrCatHTTP } @@ -145,7 +145,7 @@ func extractDetail(errorReason string, cat ErrorCategory, statusCode int) string } } if detail == "" { - detail = limitStr(errorReason, 30) + detail = limitStr(errorReason, errorDetailMaxLen) } case ErrCatTCP: for _, keyword := range []string{"connection refused", "connection reset", "no route to host", "network unreachable"} { @@ -155,7 +155,7 @@ func extractDetail(errorReason string, cat ErrorCategory, statusCode int) string } } if detail == "" { - detail = limitStr(errorReason, 30) + detail = limitStr(errorReason, errorDetailMaxLen) } case ErrCatTLS: for _, keyword := range []string{"certificate expired", "certificate has expired", "handshake failure", "unknown authority"} { @@ -166,16 +166,16 @@ func extractDetail(errorReason string, cat ErrorCategory, statusCode int) string } if detail == "" && strings.Contains(lower, "x509:") { idx := strings.Index(lower, "x509:") - detail = limitStr(errorReason[idx:], 30) + detail = limitStr(errorReason[idx:], errorDetailMaxLen) } if detail == "" { - detail = limitStr(errorReason, 30) + detail = limitStr(errorReason, errorDetailMaxLen) } case ErrCatHTTP: if statusCode > 0 { detail = fmt.Sprintf("HTTP %d", statusCode) } else { - detail = limitStr(errorReason, 30) + detail = limitStr(errorReason, errorDetailMaxLen) } case ErrCatTimeout: if strings.Contains(lower, "i/o timeout") { @@ -187,12 +187,12 @@ func extractDetail(errorReason string, cat ErrorCategory, statusCode int) string if strings.Contains(lower, "no icmp response") { detail = "no response" } else { - detail = limitStr(errorReason, 30) + detail = limitStr(errorReason, errorDetailMaxLen) } case ErrCatPrivate: detail = "private IP blocked" default: - detail = limitStr(errorReason, 30) + detail = limitStr(errorReason, errorDetailMaxLen) } return detail diff --git a/internal/tui/tab_nodes.go b/internal/tui/tab_nodes.go index 6aebe26..f79372f 100644 --- a/internal/tui/tab_nodes.go +++ b/internal/tui/tab_nodes.go @@ -55,7 +55,7 @@ func (m Model) viewNodesTab() string { regions := make(map[string]bool) var leader string for _, n := range m.nodes { - if time.Since(n.LastSeen) < 60*time.Second { + if time.Since(n.LastSeen) < nodeOnlineThreshold { online++ } if n.Region != "" { @@ -81,10 +81,10 @@ func (m Model) fmtNodeStatus(lastSeen time.Time) string { return m.st.subtleStyle.Render("UNKNOWN") } ago := time.Since(lastSeen) - if ago < 60*time.Second { + if ago < nodeOnlineThreshold { return m.st.specialStyle.Render("ONLINE") } - if ago < 5*time.Minute { + if ago < nodeStaleThreshold { return m.st.warnStyle.Render("STALE") } return m.st.dangerStyle.Render("OFFLINE") diff --git a/internal/tui/uptime_timeline.go b/internal/tui/uptime_timeline.go index c071c0c..06ee036 100644 --- a/internal/tui/uptime_timeline.go +++ b/internal/tui/uptime_timeline.go @@ -26,9 +26,9 @@ func (m Model) uptimeTimeline(days []monitor.DayReport, width int) string { for _, d := range display { ch := "█" switch { - case d.UptimePct >= 99.0: + case d.UptimePct >= uptimeGoodPct: sb.WriteString(m.st.specialStyle.Render(ch)) - case d.UptimePct >= 95.0: + case d.UptimePct >= uptimeWarnPct: sb.WriteString(m.st.warnStyle.Render(ch)) case d.UptimePct > 0: sb.WriteString(m.st.dangerStyle.Render(ch)) @@ -39,9 +39,9 @@ func (m Model) uptimeTimeline(days []monitor.DayReport, width int) string { pct := days[len(days)-1].UptimePct pctStyle := m.st.specialStyle - if pct < 99.0 { + if pct < uptimeGoodPct { pctStyle = m.st.dangerStyle - } else if pct < 99.9 { + } else if pct < uptimeExcellentPct { pctStyle = m.st.warnStyle } sb.WriteString(" " + pctStyle.Render(fmt.Sprintf("%.2f%%", pct))) diff --git a/internal/tui/view_dashboard.go b/internal/tui/view_dashboard.go index c54ece7..f43c9d4 100644 --- a/internal/tui/view_dashboard.go +++ b/internal/tui/view_dashboard.go @@ -130,7 +130,7 @@ func (m Model) computeStats() dashboardStats { } } for _, n := range m.nodes { - if !n.LastSeen.IsZero() && time.Since(n.LastSeen) > 5*time.Minute { + if !n.LastSeen.IsZero() && time.Since(n.LastSeen) > nodeStaleThreshold { s.offlineNodes++ } } @@ -294,7 +294,7 @@ func (m Model) renderFooter(stats dashboardStats) string { if len(m.nodes) > 0 { online := 0 for _, n := range m.nodes { - if !n.LastSeen.IsZero() && time.Since(n.LastSeen) < 60*time.Second { + if !n.LastSeen.IsZero() && time.Since(n.LastSeen) < nodeOnlineThreshold { online++ } } diff --git a/internal/tui/view_sla.go b/internal/tui/view_sla.go index eff6583..b9d7abd 100644 --- a/internal/tui/view_sla.go +++ b/internal/tui/view_sla.go @@ -40,10 +40,10 @@ func (m Model) viewSLAPanel() string { } bar := m.uptimeBar(r.UptimePct, barWidth) uptimeColor := m.st.specialStyle - if r.UptimePct < 99.9 { + if r.UptimePct < uptimeExcellentPct { uptimeColor = m.st.warnStyle } - if r.UptimePct < 99.0 { + if r.UptimePct < uptimeGoodPct { uptimeColor = m.st.dangerStyle } fmt.Fprintf(&b, " %-16s %s %s\n", m.st.subtleStyle.Render("Uptime"), uptimeColor.Render(fmt.Sprintf("%s%%", fmtPct(r.UptimePct))), bar) @@ -94,10 +94,10 @@ func (m Model) buildSLADailyContent() string { pctStr := fmtPct(day.UptimePct) + "%" color := m.st.specialStyle - if day.UptimePct < 99.9 { + if day.UptimePct < uptimeExcellentPct { color = m.st.warnStyle } - if day.UptimePct < 99.0 { + if day.UptimePct < uptimeGoodPct { color = m.st.dangerStyle } @@ -128,7 +128,7 @@ func fmtPct(pct float64) string { if pct == 100 { return "100.00" } - if pct >= 99.99 { + if pct >= uptimePrecisionPct { return fmt.Sprintf("%.3f", pct) } return fmt.Sprintf("%.2f", pct) -- 2.52.0