amadan
console: unit-correct clear mail body; panic-safe sweep in-flight flag
Two re-review nits: the alert-clear mail rendered its value with %g while the firing mail had moved to alertValueText (a lag clear read "902" with no unit); and LoadMetrics set the sweeping flag without a defer, so a panic in sweepAll — recovered by net/http — would have wedged the metrics cache for the life of the process. err starts non-nil so a panic can't cache nils as a successful sweep.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2 files changed,
+28
−15
internal/console/alertwatch.go+2 −2internal/console/metricsview.go+26 −13
diff --git a/internal/console/alertwatch.go b/internal/console/alertwatch.go| index f34212779..e3352610e 100644 |
| --- a/internal/console/alertwatch.go |
| +++ b/internal/console/alertwatch.go |
| @@ -282,7 +282,7 @@ func alertMessage(box, rule string, rs metrics.RuleState, updated time.Time) (su |
| return subject, body |
| } |
| subject = fmt.Sprintf("[carlos] %s: %s cleared", box, rule) |
| - body = fmt.Sprintf("The %s alert has cleared on %s.\n\nValue at the transition: %g\nTime (UTC): %s\n\n"+ |
| - "You are getting this because you hold the CARLOS fleet-operator role.\n", rule, box, rs.Value, stamp) |
| + body = fmt.Sprintf("The %s alert has cleared on %s.\n\nValue at the transition: %s\nTime (UTC): %s\n\n"+ |
| + "You are getting this because you hold the CARLOS fleet-operator role.\n", rule, box, alertValueText(rule, rs.Value), stamp) |
| return subject, body |
| } |
diff --git a/internal/console/metricsview.go b/internal/console/metricsview.go| index 4f01c8ec6..a6defb365 100644 |
| --- a/internal/console/metricsview.go |
| +++ b/internal/console/metricsview.go |
| @@ -159,19 +159,32 @@ func (s *Server) LoadMetrics(ctx context.Context) ([]BoxMetricsView, []AlertRow, |
| done := c.done |
| c.mu.Unlock() |
| - boxes, alerts, err := s.sweepAll(ctx) |
| - |
| - c.mu.Lock() |
| - if err == nil { |
| - // Only a fully successful sweep is cached; a failed one |
| - // leaves the previous cache exactly as it was. |
| - c.at = time.Now() |
| - c.boxes = boxes |
| - c.alerts = alerts |
| - } |
| - c.sweeping = false |
| - close(done) |
| - c.mu.Unlock() |
| + // err starts non-nil so that if sweepAll panics (unwinding past the |
| + // assignment below), the deferred bookkeeping still sees a failed |
| + // sweep: nothing gets cached and the in-flight flag is cleared |
| + // instead of wedging every future caller. net/http's per-connection |
| + // recover keeps the process up; this keeps the cache usable. |
| + var ( |
| + boxes []BoxMetricsView |
| + alerts []AlertRow |
| + err = errors.New("console: metrics sweep did not complete") |
| + ) |
| + func() { |
| + defer func() { |
| + c.mu.Lock() |
| + if err == nil { |
| + // Only a fully successful sweep is cached; a failed |
| + // one leaves the previous cache exactly as it was. |
| + c.at = time.Now() |
| + c.boxes = boxes |
| + c.alerts = alerts |
| + } |
| + c.sweeping = false |
| + close(done) |
| + c.mu.Unlock() |
| + }() |
| + boxes, alerts, err = s.sweepAll(ctx) |
| + }() |
| if err != nil { |
| return nil, nil, err |