refactor: remove unused error handling and improve remote address val…

…idation in socket stats

pkg/plugin/linuxutil/netstat_stats_linux.go

@@ -13,7 +13,6 @@ import (
 	"github.com/microsoft/retina/pkg/log"
 	"github.com/microsoft/retina/pkg/metrics"
 	"github.com/microsoft/retina/pkg/utils"
-	"github.com/pkg/errors"
 	"go.uber.org/zap"
 )
 
@@ -188,17 +187,6 @@ func (nr *NetstatReader) readSockStats() error {
 		return err
 	} else {
 		sockStats := processSocks(socks)
-		// Compare existing tcp socket connections with updated ones, remove the ones that are not seen in the new sockStats map
-		// Log the socketByRemoteAddr map
-		if nr.opts.PrevTCPSockStats != nil {
-			for remoteAddr := range nr.opts.PrevTCPSockStats.socketByRemoteAddr {
-				_, err := netip.ParseAddrPort(remoteAddr)
-				if err != nil {
-					return errors.Wrapf(err, "failed to parse remote address %s", remoteAddr)
-				}
-			}
-		}
-
 		nr.connStats.TcpSockets = *sockStats
 	}
 
@@ -246,7 +234,12 @@ func (nr *NetstatReader) updateMetrics() {
 	}
 
 	totalCount := 0
-	for _, v := range nr.connStats.TcpSockets.socketByRemoteAddr {
+	for remoteAddr, v := range nr.connStats.TcpSockets.socketByRemoteAddr {
+		// only count valid remote addresses
+		if _, err := netip.ParseAddrPort(remoteAddr); err != nil {
+			nr.l.Error("failed to parse remote address", zap.String("remoteAddr", remoteAddr), zap.Error(err))
+			continue
+		}
 		totalCount += v
 	}
 	metrics.TCPConnectionRemoteGauge.WithLabelValues(addrDefaultTCPRemote).Set(float64(totalCount))

pkg/plugin/linuxutil/netstat_stats_linux_test.go

@@ -12,7 +12,6 @@ import (
 	"github.com/microsoft/retina/pkg/utils"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	gomock "go.uber.org/mock/gomock"
 )
 
@@ -310,6 +309,9 @@ func TestReadSockStats(t *testing.T) {
 		},
 	}, nil).Times(1)
 
+	// We are expecting the gauges to be called once.
+	MockGaugeVec.EXPECT().WithLabelValues(addrDefaultTCPRemote).Return(testmetric).Times(1)
+	MockGaugeVec.EXPECT().WithLabelValues(utils.Active).Return(testmetric).Times(1)
 	ns.EXPECT().TCPSocks(gomock.Any()).Return([]netstat.SockTabEntry{
 		{
 			LocalAddr: &netstat.SockAddr{
@@ -337,46 +339,3 @@ func TestReadSockStats(t *testing.T) {
 
 	nr.updateMetrics()
 }
-
-// Test IP that belongs to a closed connection being removed from the metrics
-func TestReadSockStatsRemoveClosedConnection(t *testing.T) {
-	_, err := log.SetupZapLogger(log.GetDefaultLogOpts())
-	require.NoError(t, err)
-	opts := &NetstatOpts{
-		CuratedKeys: false,
-		AddZeroVal:  false,
-		ListenSock:  false,
-	}
-
-	ctrl := gomock.NewController(t)
-	defer ctrl.Finish()
-
-	ns := NewMockNetstatInterface(ctrl)
-	nr := NewNetstatReader(opts, ns)
-	assert.NotNil(t, nr)
-	InitalizeMetricsForTesting(ctrl)
-
-	testmetric := prometheus.NewGauge(prometheus.GaugeOpts{
-		Name: "testmetric",
-		Help: "testmetric",
-	})
-
-	// Initial value
-	nr.opts.PrevTCPSockStats = &SocketStats{
-		socketByRemoteAddr: map[string]int{
-			"192.168.1.100:80": 1,
-		},
-	}
-
-	// Latest read would not contain the IP in PrevTCPSockStats
-	ns.EXPECT().TCPSocks(gomock.Any()).Return([]netstat.SockTabEntry{}, nil).Times(1)
-	ns.EXPECT().UDPSocks(gomock.Any()).Return([]netstat.SockTabEntry{}, nil).Times(1)
-
-	// We are expecting the gauges to be called once.
-	MockGaugeVec.EXPECT().WithLabelValues(addrDefaultTCPRemote).Return(testmetric).Times(1)
-	MockGaugeVec.EXPECT().WithLabelValues(utils.Active).Return(testmetric).Times(1)
-
-	err = nr.readSockStats()
-	nr.updateMetrics()
-	require.NoError(t, err)
-}