Code review changes

Signed-off-by: Nick Pillitteri <[email protected]>

Author: 56quarters

pkg/alertmanager/state_replication.go

@@ -71,7 +71,7 @@ type state struct {
 func newReplicatedStates(userID string, rf int, re Replicator, st alertstore.AlertStore, l log.Logger, r prometheus.Registerer) *state {
 
 	s := &state{
-		logger:            l,
+		logger:            log.With(l, "user", userID),
 		userID:            userID,
 		replicationFactor: rf,
 		replicator:        re,
@@ -226,16 +226,17 @@ func (s *state) starting(ctx context.Context) error {
 			s.fetchReplicaStateFailed.Inc()
 		}
 
-		level.Info(s.logger).Log("msg", "state not settled; trying to read from storage", "err", err)
+		level.Info(s.logger).Log("msg", "unable to read state from other Alertmanager replicas; trying to read from storage", "err", err)
 	}
 
+	level.Info(s.logger).Log("msg", "reading state from storage")
 	// Attempt to read the state from persistent storage instead.
 	storeReadCtx, cancel := context.WithTimeout(ctx, s.storeReadTimeout)
 	defer cancel()
 
 	fullState, err := s.store.GetFullState(storeReadCtx, s.userID)
 	if errors.Is(err, alertspb.ErrNotFound) {
-		level.Info(s.logger).Log("msg", "no state for user in storage; proceeding", "user", s.userID)
+		level.Info(s.logger).Log("msg", "no state for user in storage; proceeding")
 		s.initialSyncCompleted.WithLabelValues(syncUserNotFound).Inc()
 		return nil
 	}
@@ -269,11 +270,11 @@ func (s *state) mergeFullStates(fs []*clusterpb.FullState) error {
 
 	for _, f := range fs {
 		for _, p := range f.Parts {
-			level.Debug(s.logger).Log("msg", "merging full state", "user", s.userID, "key", p.Key, "bytes", len(p.Data))
+			level.Debug(s.logger).Log("msg", "merging full state", "key", p.Key, "bytes", len(p.Data))
 
 			st, ok := s.states[p.Key]
 			if !ok {
-				level.Error(s.logger).Log("msg", "key not found while merging full state", "user", s.userID, "key", p.Key)
+				level.Error(s.logger).Log("msg", "key not found while merging full state", "key", p.Key)
 				continue
 			}
 
@@ -298,7 +299,7 @@ func (s *state) running(ctx context.Context) error {
 			s.stateReplicationTotal.WithLabelValues(p.Key).Inc()
 			if err := s.replicator.ReplicateStateForUser(ctx, s.userID, p); err != nil {
 				s.stateReplicationFailed.WithLabelValues(p.Key).Inc()
-				level.Error(s.logger).Log("msg", "failed to replicate state to other alertmanagers", "user", s.userID, "key", p.Key, "err", err)
+				level.Error(s.logger).Log("msg", "failed to replicate state to other alertmanagers", "key", p.Key, "err", err)
 			}
 		case <-ctx.Done():
 			return nil

pkg/alertmanager/state_replication_test.go

@@ -29,6 +29,8 @@ import (
 	"github.com/grafana/mimir/pkg/alertmanager/alertstore"
 )
 
+const testUserID = "user-1"
+
 type fakeState struct {
 	binary []byte
 	merges [][]byte
@@ -73,8 +75,8 @@ func (f *fakeReplicator) GetPositionForUser(_ string) int {
 }
 
 func (f *fakeReplicator) ReadFullStateForUser(ctx context.Context, userID string) ([]*clusterpb.FullState, error) {
-	if userID != "user-1" {
-		return nil, errors.New("Unexpected userID")
+	if userID != testUserID {
+		return nil, errors.New("unexpected userID")
 	}
 
 	if f.read.blocking {
@@ -121,13 +123,13 @@ func TestStateReplication(t *testing.T) {
 			replicationFactor:  1,
 			message:            &clusterpb.Part{Key: "nflog", Data: []byte("OK")},
 			replicationResults: map[string]clusterpb.Part{},
-			storeResults:       map[string]clusterpb.Part{"user-1": {Key: "nflog", Data: []byte("OK")}},
+			storeResults:       map[string]clusterpb.Part{testUserID: {Key: "nflog", Data: []byte("OK")}},
 		},
 		{
 			name:               "with a replication factor of > 1, state is broadcasted for replication.",
 			replicationFactor:  3,
 			message:            &clusterpb.Part{Key: "nflog", Data: []byte("OK")},
-			replicationResults: map[string]clusterpb.Part{"user-1": {Key: "nflog", Data: []byte("OK")}},
+			replicationResults: map[string]clusterpb.Part{testUserID: {Key: "nflog", Data: []byte("OK")}},
 			storeResults:       map[string]clusterpb.Part{},
 		},
 	}
@@ -145,7 +147,7 @@ func TestStateReplication(t *testing.T) {
 				}))
 			}
 
-			s := newReplicatedStates("user-1", tt.replicationFactor, replicator, store, log.NewNopLogger(), reg)
+			s := newReplicatedStates(testUserID, tt.replicationFactor, replicator, store, log.NewNopLogger(), reg)
 			require.False(t, s.Ready())
 			{
 				ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)