Skip to content

Commit

Permalink
[management] add metrics to network map diff (#2811)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pascal-fischer
pascal-fischer authored Oct 30, 2024
1 parent a0cdb58 commit 729bcf2
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 22 deletions.
12 changes: 12 additions & 0 deletions management/server/telemetry/updatechannel_metrics.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type UpdateChannelMetrics struct {
getAllConnectedPeersDurationMicro metric.Int64Histogram
getAllConnectedPeers metric.Int64Histogram
hasChannelDurationMicro metric.Int64Histogram
networkMapDiffDurationMicro metric.Int64Histogram
ctx context.Context
}

Expand Down Expand Up @@ -63,6 +64,11 @@ func NewUpdateChannelMetrics(ctx context.Context, meter metric.Meter) (*UpdateCh
return nil, err
}

networkMapDiffDurationMicro, err := meter.Int64Histogram("management.updatechannel.networkmap.diff.duration.micro")
if err != nil {
return nil, err
}

return &UpdateChannelMetrics{
createChannelDurationMicro: createChannelDurationMicro,
closeChannelDurationMicro: closeChannelDurationMicro,
Expand All @@ -72,6 +78,7 @@ func NewUpdateChannelMetrics(ctx context.Context, meter metric.Meter) (*UpdateCh
getAllConnectedPeersDurationMicro: getAllConnectedPeersDurationMicro,
getAllConnectedPeers: getAllConnectedPeers,
hasChannelDurationMicro: hasChannelDurationMicro,
networkMapDiffDurationMicro: networkMapDiffDurationMicro,
ctx: ctx,
}, nil
}
Expand Down Expand Up @@ -111,3 +118,8 @@ func (metrics *UpdateChannelMetrics) CountGetAllConnectedPeersDuration(duration
func (metrics *UpdateChannelMetrics) CountHasChannelDuration(duration time.Duration) {
metrics.hasChannelDurationMicro.Record(metrics.ctx, duration.Microseconds())
}

// CountNetworkMapDiffDurationMicro counts the duration of the NetworkMapDiff method
func (metrics *UpdateChannelMetrics) CountNetworkMapDiffDurationMicro(duration time.Duration) {
metrics.networkMapDiffDurationMicro.Record(metrics.ctx, duration.Microseconds())
}
15 changes: 11 additions & 4 deletions management/server/updatechannel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import (
"sync"
"time"

"github.com/netbirdio/netbird/management/server/differs"
"github.com/r3labs/diff/v3"
log "github.com/sirupsen/logrus"

"github.com/netbirdio/netbird/management/proto"
"github.com/netbirdio/netbird/management/server/differs"
"github.com/netbirdio/netbird/management/server/telemetry"
)

Expand Down Expand Up @@ -208,10 +208,10 @@ func (p *PeersUpdateManager) handlePeerMessageUpdate(ctx context.Context, peerID
p.channelsMux.RUnlock()

if lastSentUpdate != nil {
updated, err := isNewPeerUpdateMessage(ctx, lastSentUpdate, update)
updated, err := isNewPeerUpdateMessage(ctx, lastSentUpdate, update, p.metrics)
if err != nil {
log.WithContext(ctx).Errorf("error checking for SyncResponse updates: %v", err)
return false
return true
}
if !updated {
log.WithContext(ctx).Debugf("peer %s network map is not updated, skip sending update", peerID)
Expand All @@ -223,7 +223,9 @@ func (p *PeersUpdateManager) handlePeerMessageUpdate(ctx context.Context, peerID
}

// isNewPeerUpdateMessage checks if the given current update message is a new update that should be sent.
func isNewPeerUpdateMessage(ctx context.Context, lastSentUpdate, currUpdateToSend *UpdateMessage) (isNew bool, err error) {
func isNewPeerUpdateMessage(ctx context.Context, lastSentUpdate, currUpdateToSend *UpdateMessage, metric telemetry.AppMetrics) (isNew bool, err error) {
startTime := time.Now()

defer func() {
if r := recover(); r != nil {
log.WithContext(ctx).Panicf("comparing peer update messages. Trace: %s", debug.Stack())
Expand Down Expand Up @@ -258,6 +260,11 @@ func isNewPeerUpdateMessage(ctx context.Context, lastSentUpdate, currUpdateToSen
if err != nil {
return false, fmt.Errorf("failed to diff network map: %v", err)
}

if metric != nil {
metric.UpdateChannelMetrics().CountNetworkMapDiffDurationMicro(time.Since(startTime))
}

return len(changelog) > 0, nil
}

Expand Down
42 changes: 24 additions & 18 deletions management/server/updatechannel_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,16 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"

nbdns "github.com/netbirdio/netbird/dns"
"github.com/netbirdio/netbird/management/domain"
"github.com/netbirdio/netbird/management/proto"
nbpeer "github.com/netbirdio/netbird/management/server/peer"
"github.com/netbirdio/netbird/management/server/posture"
"github.com/netbirdio/netbird/management/server/telemetry"
nbroute "github.com/netbirdio/netbird/route"
"github.com/netbirdio/netbird/util"
"github.com/stretchr/testify/assert"
)

// var peersUpdater *PeersUpdateManager
Expand Down Expand Up @@ -175,8 +177,12 @@ func TestHandlePeerMessageUpdate(t *testing.T) {
}

for _, tt := range tests {
metrics, err := telemetry.NewDefaultAppMetrics(context.Background())
if err != nil {
t.Fatal(err)
}
t.Run(tt.name, func(t *testing.T) {
p := NewPeersUpdateManager(nil)
p := NewPeersUpdateManager(metrics)
ctx := context.Background()

if tt.existingUpdate != nil {
Expand All @@ -194,7 +200,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage1 := createMockUpdateMessage(t)
newUpdateMessage2 := createMockUpdateMessage(t)

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.False(t, message)
})
Expand All @@ -205,7 +211,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {

newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.False(t, message)
})
Expand All @@ -217,7 +223,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.Routes[0].Network = netip.MustParsePrefix("1.1.1.1/32")
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.True(t, message)

Expand All @@ -230,7 +236,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.Routes[0].Groups = []string{"randomGroup1"}
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -249,7 +255,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.Peers = append(newUpdateMessage2.NetworkMap.Peers, newPeer)
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -259,14 +265,14 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {

newUpdateMessage2 := createMockUpdateMessage(t)
newUpdateMessage2.Update.NetworkMap.Serial++
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.False(t, message)

newUpdateMessage3 := createMockUpdateMessage(t)
newUpdateMessage3.Update.Checks = []*proto.Checks{}
newUpdateMessage3.Update.NetworkMap.Serial++
message, err = isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage3)
message, err = isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage3, nil)
assert.NoError(t, err)
assert.True(t, message)

Expand All @@ -285,7 +291,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
}
newUpdateMessage4.Update.Checks = []*proto.Checks{toProtocolCheck(check)}
newUpdateMessage4.Update.NetworkMap.Serial++
message, err = isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage4)
message, err = isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage4, nil)
assert.NoError(t, err)
assert.True(t, message)

Expand All @@ -305,7 +311,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
}
newUpdateMessage5.Update.Checks = []*proto.Checks{toProtocolCheck(check)}
newUpdateMessage5.Update.NetworkMap.Serial++
message, err = isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage5)
message, err = isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage5, nil)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -321,7 +327,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
)
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -333,7 +339,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.Peers[0].IP = net.ParseIP("192.168.1.10")
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -345,7 +351,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.FirewallRules[0].Port = "443"
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -364,7 +370,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.FirewallRules = append(newUpdateMessage2.NetworkMap.FirewallRules, newRule)
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -376,7 +382,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.DNSConfig.NameServerGroups[0].NameServers = make([]nbdns.NameServer, 0)
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -388,7 +394,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.DNSConfig.NameServerGroups[0].NameServers[0].IP = netip.MustParseAddr("8.8.4.4")
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -400,7 +406,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.DNSConfig.CustomZones[0].Records[0].RData = "100.64.0.2"
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2, nil)
assert.NoError(t, err)
assert.True(t, message)
})
Expand Down

0 comments on commit 729bcf2

Please sign in to comment.