Simplify event storing with one generic method (#662)

Use the generic storeEvent() funcion to store all activity events.

.github/pull_request_template.md

@@ -6,5 +6,6 @@
 - [ ] Is it a bug fix
 - [ ] Is a typo/documentation fix
 - [ ] Is a feature enhancement
+- [ ] It is a refactor
 - [ ] Created tests that fail without the change (if possible)
 - [ ] Extended the README / documentation, if necessary

management/server/account.go

@@ -544,16 +544,7 @@ func (am *DefaultAccountManager) newAccount(userID, domain string) (*Account, er
 			continue
 		} else if statusErr.Type() == status.NotFound {
 			newAccount := newAccountWithId(accountId, userID, domain)
-			_, err = am.eventStore.Save(&activity.Event{
+			am.storeEvent(userID, newAccount.Id, accountId, activity.AccountCreated, nil)
-				Timestamp:   time.Now(),
-				Activity:    activity.AccountCreated,
-				AccountID:   newAccount.Id,
-				TargetID:    newAccount.Id,
-				InitiatorID: userID,
-			})
-			if err != nil {
-				return nil, err
-			}
 			return newAccount, nil
 		} else {
 			return nil, err
@@ -841,18 +832,7 @@ func (am *DefaultAccountManager) handleNewUserAccount(domainAcc *Account, claims
 		return nil, err
 	}
 
-	event := &activity.Event{
+	am.storeEvent(claims.UserId, claims.UserId, account.Id, activity.UserJoined, nil)
-		Timestamp:   time.Now(),
-		Activity:    activity.UserJoined,
-		AccountID:   account.Id,
-		TargetID:    claims.UserId,
-		InitiatorID: claims.UserId,
-	}
-
-	_, err = am.eventStore.Save(event)
-	if err != nil {
-		return nil, err
-	}
 
 	return account, nil
 }
@@ -885,17 +865,7 @@ func (am *DefaultAccountManager) redeemInvite(account *Account, userID string) e
 				return
 			}
 			log.Debugf("user %s of account %s redeemed invite", user.ID, account.Id)
-			_, err = am.eventStore.Save(&activity.Event{
+			am.storeEvent(userID, userID, account.Id, activity.UserJoined, nil)
-				Timestamp:   time.Now(),
-				Activity:    activity.UserJoined,
-				AccountID:   account.Id,
-				TargetID:    userID,
-				InitiatorID: userID,
-			})
-			if err != nil {
-				log.Warnf("failed saving activity event %v", err)
-				return
-			}
 		}()
 	}
 

management/server/account_test.go

@@ -9,6 +9,7 @@ import (
 	"reflect"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/netbirdio/netbird/management/server/jwtclaims"
 	"github.com/stretchr/testify/assert"
@@ -134,17 +135,7 @@ func TestAccountManager_GetOrCreateAccountByUser(t *testing.T) {
 	}
 
 	// check the corresponding events that should have been generated
-	events, err := manager.GetEvents(account.Id, userID)
+	ev := getEvent(t, account.Id, manager, activity.AccountCreated)
-	if err != nil {
-		return
-	}
-
-	var ev *activity.Event
-	for _, event := range events {
-		if event.Activity == activity.AccountCreated {
-			ev = event
-		}
-	}
 
 	assert.NotNil(t, ev)
 	assert.Equal(t, account.Id, ev.AccountID)
@@ -582,19 +573,7 @@ func TestAccountManager_AddPeer(t *testing.T) {
 	if account.Network.CurrentSerial() != 1 {
 		t.Errorf("expecting Network Serial=%d to be incremented by 1 and be equal to %d when adding new peer to account", serial, account.Network.CurrentSerial())
 	}
-
+	ev := getEvent(t, account.Id, manager, activity.PeerAddedWithSetupKey)
-	// check the corresponding events that should have been generated
-	events, err := manager.GetEvents(account.Id, userID)
-	if err != nil {
-		return
-	}
-
-	var ev *activity.Event
-	for _, event := range events {
-		if event.Activity == activity.PeerAddedWithSetupKey {
-			ev = event
-		}
-	}
 
 	assert.NotNil(t, ev)
 	assert.Equal(t, account.Id, ev.AccountID)
@@ -664,18 +643,7 @@ func TestAccountManager_AddPeerWithUserID(t *testing.T) {
 		t.Errorf("expecting Network Serial=%d to be incremented by 1 and be equal to %d when adding new peer to account", serial, account.Network.CurrentSerial())
 	}
 
-	// check the corresponding events that should have been generated
+	ev := getEvent(t, account.Id, manager, activity.PeerAddedByUser)
-	events, err := manager.GetEvents(account.Id, userID)
-	if err != nil {
-		return
-	}
-
-	var ev *activity.Event
-	for _, event := range events {
-		if event.Activity == activity.PeerAddedByUser {
-			ev = event
-		}
-	}
 
 	assert.NotNil(t, ev)
 	assert.Equal(t, account.Id, ev.AccountID)
@@ -921,18 +889,7 @@ func TestAccountManager_DeletePeer(t *testing.T) {
 		t.Errorf("expecting Network Serial=%d to be incremented and be equal to 2 after adding and deleteing a peer", account.Network.CurrentSerial())
 	}
 
-	// check the corresponding events that should have been generated
+	ev := getEvent(t, account.Id, manager, activity.PeerRemovedByUser)
-	events, err := manager.GetEvents(account.Id, userID)
-	if err != nil {
-		return
-	}
-
-	var ev *activity.Event
-	for _, event := range events {
-		if event.Activity == activity.PeerRemovedByUser {
-			ev = event
-		}
-	}
 
 	assert.NotNil(t, ev)
 	assert.Equal(t, account.Id, ev.AccountID)
@@ -942,6 +899,24 @@ func TestAccountManager_DeletePeer(t *testing.T) {
 	assert.Equal(t, peer.IP.String(), ev.TargetID)
 	assert.Equal(t, peer.IP.String(), fmt.Sprint(ev.Meta["ip"]))
 }
+func getEvent(t *testing.T, accountID string, manager AccountManager, eventType activity.Activity) *activity.Event {
+	for {
+		select {
+		case <-time.After(time.Second):
+			t.Fatal("no PeerAddedWithSetupKey event was generated")
+		default:
+			events, err := manager.GetEvents(accountID, userID)
+			if err != nil {
+				t.Fatal(err)
+			}
+			for _, event := range events {
+				if event.Activity == eventType {
+					return event
+				}
+			}
+		}
+	}
+}
 
 func TestGetUsersFromAccount(t *testing.T) {
 	manager, err := createManager(t)

management/server/dns.go

@@ -106,7 +106,6 @@ func (am *DefaultAccountManager) SaveDNSSettings(accountID string, userID string
 		return err
 	}
 
-	go func() {
 	addedGroups := difference(dnsSettingsToSave.DisabledManagementGroups, oldSettings.DisabledManagementGroups)
 	for _, id := range addedGroups {
 		group := account.GetGroup(id)
@@ -120,7 +119,6 @@ func (am *DefaultAccountManager) SaveDNSSettings(accountID string, userID string
 		meta := map[string]any{"group": group.Name, "group_id": group.ID}
 		am.storeEvent(userID, accountID, accountID, activity.GroupRemovedFromDisabledManagementGroups, meta)
 	}
-	}()
 
 	return am.updateAccountPeers(account)
 }

management/server/event.go

@@ -34,7 +34,10 @@ func (am *DefaultAccountManager) GetEvents(accountID, userID string) ([]*activit
 	return filtered, nil
 }
 
-func (am *DefaultAccountManager) storeEvent(initiatorID, targetID, accountID string, activityID activity.Activity, meta map[string]any) {
+func (am *DefaultAccountManager) storeEvent(initiatorID, targetID, accountID string, activityID activity.Activity,
+	meta map[string]any) {
+
+	go func() {
 		_, err := am.eventStore.Save(&activity.Event{
 			Timestamp:   time.Now(),
 			Activity:    activityID,
@@ -44,6 +47,9 @@ func (am *DefaultAccountManager) storeEvent(initiatorID, targetID, accountID str
 			Meta:        meta,
 		})
 		if err != nil {
+			//todo add metric
 			log.Errorf("received an error while storing an activity event, error: %s", err)
 		}
+	}()
+
 }

management/server/group.go

@@ -4,7 +4,6 @@ import (
 	"github.com/netbirdio/netbird/management/server/activity"
 	"github.com/netbirdio/netbird/management/server/status"
 	log "github.com/sirupsen/logrus"
-	"time"
 )
 
 // Group of the peers for ACL
@@ -89,27 +88,14 @@ func (am *DefaultAccountManager) SaveGroup(accountID, userID string, newGroup *G
 		return err
 	}
 
-	if !exists {
-		_, err = am.eventStore.Save(&activity.Event{
-			Timestamp:   time.Now(),
-			Activity:    activity.GroupCreated,
-			InitiatorID: userID,
-			TargetID:    newGroup.ID,
-			AccountID:   accountID,
-			Meta:        newGroup.EventMeta(),
-		})
-		if err != nil {
-			return err
-		}
-	}
-
 	addedPeers := make([]string, 0)
 	removedPeers := make([]string, 0)
-	if !exists {
+	if exists {
-		addedPeers = append(addedPeers, newGroup.Peers...)
-	} else {
 		addedPeers = difference(newGroup.Peers, oldGroup.Peers)
 		removedPeers = difference(oldGroup.Peers, newGroup.Peers)
+	} else {
+		addedPeers = append(addedPeers, newGroup.Peers...)
+		am.storeEvent(userID, newGroup.ID, accountID, activity.GroupCreated, newGroup.EventMeta())
 	}
 
 	for _, p := range addedPeers {
@@ -118,18 +104,9 @@ func (am *DefaultAccountManager) SaveGroup(accountID, userID string, newGroup *G
 			log.Errorf("peer %s not found under account %s while saving group", p, accountID)
 			continue
 		}
-		_, err = am.eventStore.Save(&activity.Event{
+		am.storeEvent(userID, peer.IP.String(), accountID, activity.GroupAddedToPeer,
-			Timestamp:   time.Now(),
+			map[string]any{"group": newGroup.Name, "group_id": newGroup.ID, "peer_ip": peer.IP.String(),
-			Activity:    activity.GroupAddedToPeer,
+				"peer_fqdn": peer.FQDN(am.GetDNSDomain())})
-			InitiatorID: userID,
-			TargetID:    peer.IP.String(),
-			AccountID:   accountID,
-			Meta: map[string]any{"group": newGroup.Name, "group_id": newGroup.ID, "peer_ip": peer.IP.String(),
-				"peer_fqdn": peer.FQDN(am.GetDNSDomain())},
-		})
-		if err != nil {
-			return err
-		}
 	}
 
 	for _, p := range removedPeers {
@@ -138,18 +115,9 @@ func (am *DefaultAccountManager) SaveGroup(accountID, userID string, newGroup *G
 			log.Errorf("peer %s not found under account %s while saving group", p, accountID)
 			continue
 		}
-		_, err = am.eventStore.Save(&activity.Event{
+		am.storeEvent(userID, peer.IP.String(), accountID, activity.GroupRemovedFromPeer,
-			Timestamp:   time.Now(),
+			map[string]any{"group": newGroup.Name, "group_id": newGroup.ID, "peer_ip": peer.IP.String(),
-			Activity:    activity.GroupRemovedFromPeer,
+				"peer_fqdn": peer.FQDN(am.GetDNSDomain())})
-			InitiatorID: userID,
-			TargetID:    peer.IP.String(),
-			AccountID:   accountID,
-			Meta: map[string]any{"group": newGroup.Name, "group_id": newGroup.ID, "peer_ip": peer.IP.String(),
-				"peer_fqdn": peer.FQDN(am.GetDNSDomain())},
-		})
-		if err != nil {
-			return err
-		}
 	}
 
 	return am.updateAccountPeers(account)

management/server/peer.go

@@ -278,18 +278,7 @@ func (am *DefaultAccountManager) DeletePeer(accountID, peerPubKey, userID string
 	}
 
 	am.peersUpdateManager.CloseChannel(peerPubKey)
-	event := &activity.Event{
+	am.storeEvent(userID, peer.IP.String(), account.Id, activity.PeerRemovedByUser, peer.EventMeta(am.GetDNSDomain()))
-		Timestamp:   time.Now(),
-		AccountID:   account.Id,
-		InitiatorID: userID,
-		TargetID:    peer.IP.String(),
-		Activity:    activity.PeerRemovedByUser,
-		Meta:        peer.EventMeta(am.GetDNSDomain()),
-	}
-	_, err = am.eventStore.Save(event)
-	if err != nil {
-		return nil, err
-	}
 	return peer, nil
 }
 
@@ -479,10 +468,7 @@ func (am *DefaultAccountManager) AddPeer(setupKey, userID string, peer *Peer) (*
 
 	opEvent.TargetID = newPeer.IP.String()
 	opEvent.Meta = newPeer.EventMeta(am.GetDNSDomain())
-	_, err = am.eventStore.Save(opEvent)
+	am.storeEvent(opEvent.InitiatorID, opEvent.TargetID, opEvent.AccountID, opEvent.Activity, opEvent.Meta)
-	if err != nil {
-		return nil, err
-	}
 
 	return newPeer, nil
 }

management/server/rule.go

@@ -4,7 +4,6 @@ import (
 	"github.com/netbirdio/netbird/management/server/activity"
 	"github.com/netbirdio/netbird/management/server/status"
 	"strings"
-	"time"
 )
 
 // TrafficFlowType defines allowed direction of the traffic in the rule
@@ -144,19 +143,7 @@ func (am *DefaultAccountManager) SaveRule(accountID, userID string, rule *Rule)
 	if exists {
 		action = activity.RuleUpdated
 	}
-
+	am.storeEvent(userID, rule.ID, accountID, action, rule.EventMeta())
-	_, err = am.eventStore.Save(&activity.Event{
-		Timestamp:   time.Now(),
-		Activity:    action,
-		InitiatorID: userID,
-		TargetID:    rule.ID,
-		AccountID:   accountID,
-		Meta:        rule.EventMeta(),
-	})
-
-	if err != nil {
-		return err
-	}
 
 	return am.updateAccountPeers(account)
 }
@@ -257,18 +244,7 @@ func (am *DefaultAccountManager) DeleteRule(accountID, ruleID, userID string) er
 		return err
 	}
 
-	_, err = am.eventStore.Save(&activity.Event{
+	am.storeEvent(userID, rule.ID, accountID, activity.RuleRemoved, rule.EventMeta())
-		Timestamp:   time.Now(),
-		Activity:    activity.RuleRemoved,
-		InitiatorID: userID,
-		TargetID:    ruleID,
-		AccountID:   accountID,
-		Meta:        rule.EventMeta(),
-	})
-
-	if err != nil {
-		return err
-	}
 
 	return am.updateAccountPeers(account)
 }

management/server/setupkey.go

@@ -226,34 +226,13 @@ func (am *DefaultAccountManager) CreateSetupKey(accountID string, keyName string
 		return nil, status.Errorf(status.Internal, "failed adding account key")
 	}
 
-	_, err = am.eventStore.Save(&activity.Event{
+	am.storeEvent(userID, setupKey.Id, accountID, activity.SetupKeyCreated, setupKey.EventMeta())
-		Timestamp:   time.Now(),
-		Activity:    activity.SetupKeyCreated,
-		InitiatorID: userID,
-		TargetID:    setupKey.Id,
-		AccountID:   accountID,
-		Meta:        setupKey.EventMeta(),
-	})
-	if err != nil {
-		return nil, err
-	}
 
 	for _, g := range setupKey.AutoGroups {
 		group := account.GetGroup(g)
 		if group != nil {
-			_, err := am.eventStore.Save(&activity.Event{
+			am.storeEvent(userID, setupKey.Id, accountID, activity.GroupAddedToSetupKey,
-				Timestamp:   time.Now(),
+				map[string]any{"group": group.Name, "group_id": group.ID, "setupkey": setupKey.Name})
-				Activity:    activity.GroupAddedToSetupKey,
-				InitiatorID: userID,
-				TargetID:    setupKey.Id,
-				AccountID:   accountID,
-				Meta:        map[string]any{"group": group.Name, "group_id": group.ID, "setupkey": setupKey.Name},
-			})
-			if err != nil {
-				log.Errorf("failed saving setup key activity event %s: %v",
-					activity.GroupAddedToSetupKey.StringCode(), err)
-				continue
-			}
 		} else {
 			log.Errorf("group %s not found while saving setup key activity event of account %s", g, account.Id)
 		}
@@ -304,17 +283,7 @@ func (am *DefaultAccountManager) SaveSetupKey(accountID string, keyToSave *Setup
 	}
 
 	if !oldKey.Revoked && newKey.Revoked {
-		_, err = am.eventStore.Save(&activity.Event{
+		am.storeEvent(userID, newKey.Id, accountID, activity.SetupKeyRevoked, newKey.EventMeta())
-			Timestamp:   time.Now(),
-			Activity:    activity.SetupKeyRevoked,
-			InitiatorID: userID,
-			TargetID:    newKey.Id,
-			AccountID:   accountID,
-			Meta:        newKey.EventMeta(),
-		})
-		if err != nil {
-			return nil, err
-		}
 	}
 
 	defer func() {
@@ -323,19 +292,8 @@ func (am *DefaultAccountManager) SaveSetupKey(accountID string, keyToSave *Setup
 		for _, g := range removedGroups {
 			group := account.GetGroup(g)
 			if group != nil {
-				_, err := am.eventStore.Save(&activity.Event{
+				am.storeEvent(userID, oldKey.Id, accountID, activity.GroupRemovedFromSetupKey,
-					Timestamp:   time.Now(),
+					map[string]any{"group": group.Name, "group_id": group.ID, "setupkey": newKey.Name})
-					Activity:    activity.GroupRemovedFromSetupKey,
-					InitiatorID: userID,
-					TargetID:    oldKey.Id,
-					AccountID:   accountID,
-					Meta:        map[string]any{"group": group.Name, "group_id": group.ID, "setupkey": newKey.Name},
-				})
-				if err != nil {
-					log.Errorf("failed saving setup key activity event %s: %v",
-						activity.GroupRemovedFromSetupKey.StringCode(), err)
-					continue
-				}
 			} else {
 				log.Errorf("group %s not found while saving setup key activity event of account %s", g, account.Id)
 			}
@@ -345,19 +303,8 @@ func (am *DefaultAccountManager) SaveSetupKey(accountID string, keyToSave *Setup
 		for _, g := range addedGroups {
 			group := account.GetGroup(g)
 			if group != nil {
-				_, err := am.eventStore.Save(&activity.Event{
+				am.storeEvent(userID, oldKey.Id, accountID, activity.GroupAddedToSetupKey,
-					Timestamp:   time.Now(),
+					map[string]any{"group": group.Name, "group_id": group.ID, "setupkey": newKey.Name})
-					Activity:    activity.GroupAddedToSetupKey,
-					InitiatorID: userID,
-					TargetID:    oldKey.Id,
-					AccountID:   accountID,
-					Meta:        map[string]any{"group": group.Name, "group_id": group.ID, "setupkey": newKey.Name},
-				})
-				if err != nil {
-					log.Errorf("failed saving setup key activity event %s: %v",
-						activity.GroupAddedToSetupKey.StringCode(), err)
-					continue
-				}
 			} else {
 				log.Errorf("group %s not found while saving setup key activity event of account %s", g, account.Id)
 			}

management/server/setupkey_test.go

@@ -56,17 +56,8 @@ func TestDefaultAccountManager_SaveSetupKey(t *testing.T) {
 	assertKey(t, newKey, newKeyName, revoked, "reusable", 0, key.CreatedAt, key.ExpiresAt,
 		key.Id, time.Now(), autoGroups)
 
-	events, err := manager.GetEvents(account.Id, userID)
+	// check the corresponding events that should have been generated
-	if err != nil {
+	ev := getEvent(t, account.Id, manager, activity.SetupKeyRevoked)
-		return
-	}
-
-	var ev *activity.Event
-	for _, event := range events {
-		if event.Activity == activity.SetupKeyRevoked {
-			ev = event
-		}
-	}
 
 	assert.NotNil(t, ev)
 	assert.Equal(t, account.Id, ev.AccountID)
@@ -160,17 +151,8 @@ func TestDefaultAccountManager_CreateSetupKey(t *testing.T) {
 				tCase.expectedCreatedAt, tCase.expectedExpiresAt, strconv.Itoa(int(Hash(key.Key))),
 				tCase.expectedUpdatedAt, tCase.expectedGroups)
 
-			events, err := manager.GetEvents(account.Id, userID)
+			// check the corresponding events that should have been generated
-			if err != nil {
+			ev := getEvent(t, account.Id, manager, activity.SetupKeyCreated)
-				return
-			}
-
-			var ev *activity.Event
-			for _, event := range events {
-				if event.Activity == activity.SetupKeyCreated {
-					ev = event
-				}
-			}
 
 			assert.NotNil(t, ev)
 			assert.Equal(t, account.Id, ev.AccountID)

management/server/user.go

@@ -8,7 +8,6 @@ import (
 	"github.com/netbirdio/netbird/management/server/status"
 	log "github.com/sirupsen/logrus"
 	"strings"
-	"time"
 )
 
 const (
@@ -178,18 +177,7 @@ func (am *DefaultAccountManager) CreateUser(accountID, userID string, invite *Us
 		return nil, err
 	}
 
-	event := &activity.Event{
+	am.storeEvent(userID, newUser.Id, accountID, activity.UserInvited, nil)
-		Timestamp:   time.Now(),
-		Activity:    activity.UserInvited,
-		AccountID:   account.Id,
-		TargetID:    newUser.Id,
-		InitiatorID: userID,
-	}
-
-	_, err = am.eventStore.Save(event)
-	if err != nil {
-		return nil, err
-	}
 
 	return newUser.toUserInfo(idpUser)
 
@@ -235,18 +223,7 @@ func (am *DefaultAccountManager) SaveUser(accountID, userID string, update *User
 
 	defer func() {
 		if oldUser.Role != newUser.Role {
-			_, err := am.eventStore.Save(&activity.Event{
+			am.storeEvent(userID, oldUser.Id, accountID, activity.UserRoleUpdated, map[string]any{"role": newUser.Role})
-				Timestamp:   time.Now(),
-				Activity:    activity.UserRoleUpdated,
-				InitiatorID: userID,
-				TargetID:    oldUser.Id,
-				AccountID:   accountID,
-				Meta:        map[string]any{"role": newUser.Role},
-			})
-			if err != nil {
-				log.Errorf("failed saving user activity event %v", err)
-				return
-			}
 		}
 
 		removedGroups := difference(oldUser.AutoGroups, update.AutoGroups)
@@ -254,19 +231,8 @@ func (am *DefaultAccountManager) SaveUser(accountID, userID string, update *User
 		for _, g := range removedGroups {
 			group := account.GetGroup(g)
 			if group != nil {
-				_, err := am.eventStore.Save(&activity.Event{
+				am.storeEvent(userID, oldUser.Id, accountID, activity.GroupRemovedFromUser,
-					Timestamp:   time.Now(),
+					map[string]any{"group": group.Name, "group_id": group.ID})
-					Activity:    activity.GroupRemovedFromUser,
-					InitiatorID: userID,
-					TargetID:    oldUser.Id,
-					AccountID:   accountID,
-					Meta:        map[string]any{"group": group.Name, "group_id": group.ID},
-				})
-				if err != nil {
-					log.Errorf("failed saving user activity event %s %v",
-						activity.GroupRemovedFromUser.StringCode(), err)
-					continue
-				}
 			} else {
 				log.Errorf("group %s not found while saving user activity event of account %s", g, account.Id)
 			}
@@ -276,19 +242,8 @@ func (am *DefaultAccountManager) SaveUser(accountID, userID string, update *User
 		for _, g := range addedGroups {
 			group := account.GetGroup(g)
 			if group != nil {
-				_, err := am.eventStore.Save(&activity.Event{
+				am.storeEvent(userID, oldUser.Id, accountID, activity.GroupAddedToUser,
-					Timestamp:   time.Now(),
+					map[string]any{"group": group.Name, "group_id": group.ID})
-					Activity:    activity.GroupAddedToUser,
-					InitiatorID: userID,
-					TargetID:    oldUser.Id,
-					AccountID:   accountID,
-					Meta:        map[string]any{"group": group.Name, "group_id": group.ID},
-				})
-				if err != nil {
-					log.Errorf("failed saving user activity event %s: %v",
-						activity.GroupAddedToUser.StringCode(), err)
-					continue
-				}
 			} else {
 				log.Errorf("group %s not found while saving user activity event of account %s", g, account.Id)
 			}