Revert "[review-fix 2] Keep RC filters standard-only"

This reverts commit 9ac5b16.

Author: AlexandreYang

comp/networkpath/npcollector/impl/baseline_test.go

@@ -15,9 +15,9 @@ import (
 
 	model "github.com/DataDog/agent-payload/v5/process"
 	"github.com/DataDog/datadog-agent/comp/networkpath/npcollector/impl/common"
+	"github.com/DataDog/datadog-agent/comp/networkpath/npcollector/impl/connfilter"
 	npmodel "github.com/DataDog/datadog-agent/comp/networkpath/npcollector/model"
 	"github.com/DataDog/datadog-agent/pkg/networkpath/payload"
-	"github.com/DataDog/datadog-agent/pkg/remoteconfig/state"
 	"github.com/DataDog/datadog-agent/pkg/trace/teststatsd"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -150,28 +150,32 @@ func TestBaselineKeepsStrongestObservationPerPath(t *testing.T) {
 	assert.Equal(t, []string{"10.0.0.1", "10.0.0.2", "10.0.0.3"}, scheduledBaselineHosts(t, collector))
 }
 
-func TestBaselineIgnoresDynamicRemoteConfigFilters(t *testing.T) {
+func TestBaselinePreservesWinningRCProvenance(t *testing.T) {
 	_, collector := newTestNpCollector(t, map[string]any{
 		"network_path.connections_monitoring.baseline_tests_enabled": true,
 		"network_path.collector.monitor_ip_without_domain":           true,
-		"network_path.collector.filters": []map[string]any{{
-			"type":     "exclude",
-			"match_ip": "10.0.0.1",
-		}},
 	}, &teststatsd.Client{}, nil)
-	collector.UpdateRemoteConfig(map[string]state.RawConfig{
-		"dynamic": {Config: dynamicConfig("dynamic-a", `[
-			{"type":"include","match_ip":"10.0.0.1"},
-			{"type":"exclude","match_ip":"10.0.0.2"}
-		]`)},
-	}, func(string, state.ApplyStatus) {})
+	filter, errs := connfilter.NewConnFilter([]connfilter.Config{
+		{Type: connfilter.FilterTypeExclude, MatchIP: "10.0.0.1"},
+		{
+			Type:         connfilter.FilterTypeInclude,
+			MatchIP:      "10.0.0.1",
+			TestConfigID: "dynamic-a",
+			Tags:         []string{"team:payments"},
+		},
+	}, "", false)
+	require.Empty(t, errs)
+	collector.filter = filter
 
 	collector.ScheduleNetworkPathTests(slices.Values([]npmodel.NetworkPathConnection{
 		baselineConn("10.0.0.1", 1),
-		baselineConn("10.0.0.2", 2),
 	}))
 
-	assert.Equal(t, []string{"10.0.0.2"}, scheduledBaselineHosts(t, collector), "RC filters must neither admit nor exclude baseline candidates")
+	pathtest := <-collector.pathtestInputChan
+	assert.Equal(t, payload.DynamicTestProfileBaseline, pathtest.DynamicTestProfile)
+	assert.Equal(t, "dynamic-a", pathtest.TestConfigID)
+	assert.Equal(t, payload.TestConfigSourceRemote, pathtest.TestConfigSource)
+	assert.Equal(t, []string{"team:payments"}, pathtest.Tags)
 }
 
 func TestBaselineEmptySnapshotsSelectNothing(t *testing.T) {

comp/networkpath/npcollector/impl/npcollector.go

@@ -84,7 +84,6 @@ type npCollectorImpl struct {
 	networkDevicesNamespace string
 	filterMutex             sync.RWMutex
 	filter                  *connfilter.ConnFilter
-	localFilter             *connfilter.ConnFilter
 	localIPs                *localIPCache
 	remoteConfigState       dynamicRemoteConfigState
 }
@@ -134,8 +133,7 @@ func newNpCollectorImpl(epForwarder eventplatform.Forwarder, collectorConfigs *c
 		flushLoopDone:         make(chan struct{}),
 		workersDone:           make(chan struct{}),
 
-		filter:      filter,
-		localFilter: filter,
+		filter: filter,
 	}
 }
 
@@ -214,7 +212,7 @@ type pathEvaluation struct {
 	tags           []string
 }
 
-func (s *npCollectorImpl) evaluateNetworkPathForConn(conn npmodel.NetworkPathConnection, origin payload.PathOrigin, vpcSubnets []netip.Prefix, baselineMode bool) pathEvaluation {
+func (s *npCollectorImpl) evaluateNetworkPathForConn(conn npmodel.NetworkPathConnection, origin payload.PathOrigin, vpcSubnets []netip.Prefix) pathEvaluation {
 	if conn.IntraHost {
 		_ = s.statsdClient.Incr(netpathConnsSkippedMetricName, []string{"reason:skip_intra_host"}, 1)
 		return pathEvaluation{}
@@ -243,18 +241,9 @@ func (s *npCollectorImpl) evaluateNetworkPathForConn(conn npmodel.NetworkPathCon
 		return pathEvaluation{}
 	}
 
-	var included bool
-	var testConfigID string
-	var tags []string
-	if baselineMode {
-		// Dynamic Remote Configuration admits standard tests only. Baseline
-		// selection must remain governed by built-in and local filters.
-		included, testConfigID, tags = s.localFilter.EvaluateWithTags(conn.Domain, conn.Dest.Addr())
-	} else {
-		s.filterMutex.RLock()
-		included, testConfigID, tags = s.filter.EvaluateWithTags(conn.Domain, conn.Dest.Addr())
-		s.filterMutex.RUnlock()
-	}
+	s.filterMutex.RLock()
+	included, testConfigID, tags := s.filter.EvaluateWithTags(conn.Domain, conn.Dest.Addr())
+	s.filterMutex.RUnlock()
 	if !included {
 		_ = s.statsdClient.Incr(netpathConnsSkippedMetricName, []string{"reason:skip_not_matched_by_filters"}, 1)
 		return pathEvaluation{}
@@ -334,18 +323,23 @@ func (s *npCollectorImpl) scheduleNetworkPathTests(origin payload.PathOrigin, co
 	}
 	for conn := range conns {
 		connCount++
-		evaluation := s.evaluateNetworkPathForConn(conn, origin, vpcSubnets, baselineMode)
+		evaluation := s.evaluateNetworkPathForConn(conn, origin, vpcSubnets)
 		if !evaluation.shouldSchedule {
 			s.logger.Tracef("Skipped connection: addr=%s, protocol=%s", conn.Dest, conn.Type)
 			continue
 		}
 		pathtest := s.makePathtest(conn, origin)
+		pathtest.TestConfigID = evaluation.testConfigID
+		pathtest.Tags = evaluation.tags
+		if evaluation.testConfigID != "" {
+			pathtest.TestConfigSource = payload.TestConfigSourceRemote
+		}
 		if baselineMode {
 			selectedBaselineCandidates = addBaselinePath(selectedBaselineCandidates, pathtest, conn.Signals)
 			continue
 		}
 
-		if err := s.scheduleStandardNetworkPathTest(pathtest, evaluation); err != nil {
+		if err := s.scheduleOne(&pathtest); err != nil {
 			s.logger.Errorf("Error scheduling pathtests: %s", err)
 		}
 	}
@@ -355,16 +349,6 @@ func (s *npCollectorImpl) scheduleNetworkPathTests(origin payload.PathOrigin, co
 	_ = s.statsdClient.Count(common.NetworkPathCollectorMetricPrefix+"schedule.conns_received", int64(connCount), []string{}, 1)
 	_ = s.statsdClient.Gauge(common.NetworkPathCollectorMetricPrefix+"schedule.duration", s.TimeNowFn().Sub(startTime).Seconds(), nil, 1)
 }
-
-func (s *npCollectorImpl) scheduleStandardNetworkPathTest(pathtest common.Pathtest, evaluation pathEvaluation) error {
-	pathtest.TestConfigID = evaluation.testConfigID
-	pathtest.Tags = evaluation.tags
-	if evaluation.testConfigID != "" {
-		pathtest.TestConfigSource = payload.TestConfigSourceRemote
-	}
-	return s.scheduleOne(&pathtest)
-}
-
 func (s *npCollectorImpl) scheduleBaselinePaths(selected []baselineCandidate) {
 	for i := range selected {
 		if err := s.scheduleOne(&selected[i].path); err != nil {

comp/networkpath/npcollector/impl/npcollector_test.go

@@ -2358,7 +2358,7 @@ network_path:
 			stats := &teststatsd.Client{}
 			_, npCollector := newTestNpCollector(t, agentConfigs, stats, nil)
 
-			require.Equal(t, tt.shouldSchedule, npCollector.evaluateNetworkPathForConn(tt.conn, payload.PathOriginNetworkTraffic, tt.vpcSubnets, false).shouldSchedule)
+			require.Equal(t, tt.shouldSchedule, npCollector.evaluateNetworkPathForConn(tt.conn, payload.PathOriginNetworkTraffic, tt.vpcSubnets).shouldSchedule)
 
 			if tt.subnetSkipped {
 				require.Contains(t, stats.CountCalls, subnetSkippedStat)
@@ -2424,7 +2424,7 @@ func Test_npCollectorImpl_evaluateNetworkPathForConn_subnets(t *testing.T) {
 			stats := &teststatsd.Client{}
 			_, npCollector := newTestNpCollector(t, agentConfigs, stats, nil)
 
-			assert.Equal(t, tt.shouldSchedule, npCollector.evaluateNetworkPathForConn(tt.conn, payload.PathOriginNetworkTraffic, nil, false).shouldSchedule)
+			assert.Equal(t, tt.shouldSchedule, npCollector.evaluateNetworkPathForConn(tt.conn, payload.PathOriginNetworkTraffic, nil).shouldSchedule)
 
 			if tt.subnetSkipped {
 				require.Contains(t, stats.CountCalls, subnetSkippedStat)