Merge #97035

97035: kvserver: log circuit breaker trip events at error severity r=erikgrinaker a=tbg

Informs #94691.

Epic: CRDB-23087
Release note: None


Co-authored-by: Tobias Grieger <[email protected]>

pkg/kv/kvserver/replica_circuit_breaker.go

@@ -145,6 +145,7 @@ func newReplicaCircuitBreaker(
 		Name:       "breaker", // log bridge has ctx tags
 		AsyncProbe: br.asyncProbe,
 		EventHandler: &replicaCircuitBreakerLogger{
+			ambientCtx: ambientCtx,
 			EventHandler: &circuit.EventLogger{
 				Log: func(buf redact.StringBuilder) {
 					log.Infof(ambientCtx.AnnotateCtx(context.Background()), "%s", buf)
@@ -160,15 +161,19 @@ func newReplicaCircuitBreaker(
 
 type replicaCircuitBreakerLogger struct {
 	circuit.EventHandler
-	onTrip  func()
-	onReset func()
+	ambientCtx log.AmbientContext
+	onTrip     func()
+	onReset    func()
 }
 
-func (r replicaCircuitBreakerLogger) OnTrip(br *circuit.Breaker, prev, cur error) {
+func (r replicaCircuitBreakerLogger) OnTrip(b *circuit.Breaker, prev, cur error) {
 	if prev == nil {
 		r.onTrip()
 	}
-	r.EventHandler.OnTrip(br, prev, cur)
+	// Log directly from this method via log.Errorf.
+	var buf redact.StringBuilder
+	circuit.EventFormatter{}.OnTrip(b, prev, cur, &buf)
+	log.Errorf(r.ambientCtx.AnnotateCtx(context.Background()), "%s", buf)
 }
 
 func (r replicaCircuitBreakerLogger) OnReset(br *circuit.Breaker) {

pkg/util/circuit/event_handler.go

@@ -30,29 +30,73 @@ type EventLogger struct {
 	Log func(redact.StringBuilder)
 }
 
+func (d *EventLogger) maybeLog(buf redact.StringBuilder) {
+	if buf.Len() == 0 {
+		return
+	}
+	d.Log(buf)
+}
+
 // OnTrip implements EventHandler. If the previous error is nil, it logs the
 // error. If the previous error is not nil and has a different cause than the
 // current error, logs a message indicating the old and new error.
 func (d *EventLogger) OnTrip(b *Breaker, prev, cur error) {
 	var buf redact.StringBuilder
+	EventFormatter{}.OnTrip(b, prev, cur, &buf)
+	d.maybeLog(buf)
+}
+
+// OnProbeLaunched implements EventHandler. It is a no-op.
+func (d *EventLogger) OnProbeLaunched(b *Breaker) {
+	var buf redact.StringBuilder
+	EventFormatter{}.OnProbeLaunched(b, &buf)
+	d.maybeLog(buf)
+}
+
+// OnProbeDone implements EventHandler. It is a no-op.
+func (d *EventLogger) OnProbeDone(b *Breaker) {
+	var buf redact.StringBuilder
+	EventFormatter{}.OnProbeDone(b, &buf)
+	d.maybeLog(buf)
+}
+
+// OnReset implements EventHandler. It logs a message.
+func (d *EventLogger) OnReset(b *Breaker) {
+	var buf redact.StringBuilder
+	EventFormatter{}.OnReset(b, &buf)
+	d.maybeLog(buf)
+}
+
+// EventFormatter allows direct access to the translation of
+// EventHandler inputs to log line suitable for formatted output.
+// Its methods mirror the `EventHandler` interface, but with a
+// StringBuilder as the last argument, which will be written to.
+// Not all methods produce output. The caller should check if
+// anything was written to the buffer before logging it.
+type EventFormatter struct{}
+
+// OnTrip mirrors EventHandler.OnTrip.
+func (EventFormatter) OnTrip(b *Breaker, prev, cur error, buf *redact.StringBuilder) {
 	opts := b.Opts()
 	if prev != nil && !errors.Is(errors.Cause(prev), errors.Cause(cur)) {
 		buf.Printf("%s: now tripped with error: %s (previously: %s)", opts.Name, cur, prev)
 	} else {
 		buf.Printf("%s: tripped with error: %s", opts.Name, cur)
 	}
-	d.Log(buf)
+
 }
 
-// OnProbeLaunched implements EventHandler. It is a no-op.
-func (d *EventLogger) OnProbeLaunched(*Breaker) {}
+// OnProbeLaunched mirrors EventHandler.OnProbeLaunched.
+func (EventFormatter) OnProbeLaunched(*Breaker, *redact.StringBuilder) {
+	// No-op.
+}
 
-// OnProbeDone implements EventHandler. It is a no-op.
-func (d *EventLogger) OnProbeDone(*Breaker) {}
+// OnProbeDone mirrors EventHandler.OnProbeDone.
+func (EventFormatter) OnProbeDone(*Breaker, *redact.StringBuilder) {
+	// No-op.
+}
 
-// OnReset implements EventHandler. It logs a message.
-func (d *EventLogger) OnReset(b *Breaker) {
-	var buf redact.StringBuilder
+// OnReset mirrors EventHandler.OnReset.
+func (EventFormatter) OnReset(b *Breaker, buf *redact.StringBuilder) {
 	buf.Printf("%s: breaker reset", b.Opts().Name)
-	d.Log(buf)
 }