From 729bcf2b01b0a50f5fcd326394c43df33c9ab2b2 Mon Sep 17 00:00:00 2001 From: Pascal Fischer <32096965+pascal-fischer@users.noreply.github.com> Date: Wed, 30 Oct 2024 16:53:23 +0100 Subject: [PATCH] [management] add metrics to network map diff (#2811) --- .../server/telemetry/updatechannel_metrics.go | 12 ++++++ management/server/updatechannel.go | 15 +++++-- management/server/updatechannel_test.go | 42 +++++++++++-------- 3 files changed, 47 insertions(+), 22 deletions(-) diff --git a/management/server/telemetry/updatechannel_metrics.go b/management/server/telemetry/updatechannel_metrics.go index 2582006e5..fb33b663c 100644 --- a/management/server/telemetry/updatechannel_metrics.go +++ b/management/server/telemetry/updatechannel_metrics.go @@ -18,6 +18,7 @@ type UpdateChannelMetrics struct { getAllConnectedPeersDurationMicro metric.Int64Histogram getAllConnectedPeers metric.Int64Histogram hasChannelDurationMicro metric.Int64Histogram + networkMapDiffDurationMicro metric.Int64Histogram ctx context.Context } @@ -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, @@ -72,6 +78,7 @@ func NewUpdateChannelMetrics(ctx context.Context, meter metric.Meter) (*UpdateCh getAllConnectedPeersDurationMicro: getAllConnectedPeersDurationMicro, getAllConnectedPeers: getAllConnectedPeers, hasChannelDurationMicro: hasChannelDurationMicro, + networkMapDiffDurationMicro: networkMapDiffDurationMicro, ctx: ctx, }, nil } @@ -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()) +} diff --git a/management/server/updatechannel.go b/management/server/updatechannel.go index 6fb96c971..7c7300222 100644 --- a/management/server/updatechannel.go +++ b/management/server/updatechannel.go @@ -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" ) @@ -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) @@ -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()) @@ -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 } diff --git a/management/server/updatechannel_test.go b/management/server/updatechannel_test.go index 52b715e95..b8a0ce45f 100644 --- a/management/server/updatechannel_test.go +++ b/management/server/updatechannel_test.go @@ -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 @@ -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 { @@ -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) }) @@ -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) }) @@ -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) @@ -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) }) @@ -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) }) @@ -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) @@ -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) @@ -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) }) @@ -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) }) @@ -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) }) @@ -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) }) @@ -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) }) @@ -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) }) @@ -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) }) @@ -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) })