Revert "[snmp] Add agent_group tag for snmp integration" (#32164)

comp/haagent/impl/config.go

@@ -7,7 +7,6 @@ package haagentimpl
 
 import (
 	"github.com/DataDog/datadog-agent/comp/core/config"
-	helpers "github.com/DataDog/datadog-agent/comp/haagent/helpers"
 )
 
 // validHaIntegrations represent the list of integrations that will be considered as
@@ -31,7 +30,7 @@ type haAgentConfigs struct {
 
 func newHaAgentConfigs(agentConfig config.Component) *haAgentConfigs {
 	return &haAgentConfigs{
-		enabled: helpers.IsEnabled(agentConfig),
-		group:   helpers.GetGroup(agentConfig),
+		enabled: agentConfig.GetBool("ha_agent.enabled"),
+		group:   agentConfig.GetString("ha_agent.group"),
 	}
 }

comp/metadata/host/hostimpl/hosttags/tags.go

@@ -12,7 +12,6 @@ import (
 	"strings"
 	"time"
 
-	haagenthelpers "github.com/DataDog/datadog-agent/comp/haagent/helpers"
 	"github.com/DataDog/datadog-agent/pkg/config/env"
 	"github.com/DataDog/datadog-agent/pkg/config/model"
 	configUtils "github.com/DataDog/datadog-agent/pkg/config/utils"
@@ -134,8 +133,8 @@ func Get(ctx context.Context, cached bool, conf model.Reader) *Tags {
 		hostTags = appendToHostTags(hostTags, clusterNameTags)
 	}
 
-	if haagenthelpers.IsEnabled(conf) {
-		hostTags = appendToHostTags(hostTags, haagenthelpers.GetHaAgentTags(conf))
+	if conf.GetBool("ha_agent.enabled") {
+		hostTags = appendToHostTags(hostTags, []string{"agent_group:" + conf.GetString("ha_agent.group")})
 	}
 
 	gceTags := []string{}

pkg/collector/corechecks/snmp/internal/devicecheck/devicecheck.go

@@ -17,9 +17,8 @@ import (
 
 	"go.uber.org/atomic"
 
-	"github.com/DataDog/datadog-agent/comp/core/config"
-	haagenthelpers "github.com/DataDog/datadog-agent/comp/haagent/helpers"
 	"github.com/DataDog/datadog-agent/pkg/collector/externalhost"
+	pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup"
 	configUtils "github.com/DataDog/datadog-agent/pkg/config/utils"
 	"github.com/DataDog/datadog-agent/pkg/metrics/servicecheck"
 	"github.com/DataDog/datadog-agent/pkg/util/hostname/validate"
@@ -67,13 +66,12 @@ type DeviceCheck struct {
 	diagnoses               *diagnoses.Diagnoses
 	interfaceBandwidthState report.InterfaceBandwidthState
 	cacheKey                string
-	agentConfig             config.Component
 }
 
 const cacheKeyPrefix = "snmp-tags"
 
 // NewDeviceCheck returns a new DeviceCheck
-func NewDeviceCheck(config *checkconfig.CheckConfig, ipAddress string, sessionFactory session.Factory, agentConfig config.Component) (*DeviceCheck, error) {
+func NewDeviceCheck(config *checkconfig.CheckConfig, ipAddress string, sessionFactory session.Factory) (*DeviceCheck, error) {
 	newConfig := config.CopyWithNewIP(ipAddress)
 
 	var devicePinger pinger.Pinger
@@ -96,7 +94,6 @@ func NewDeviceCheck(config *checkconfig.CheckConfig, ipAddress string, sessionFa
 		diagnoses:               diagnoses.NewDeviceDiagnoses(newConfig.DeviceID),
 		interfaceBandwidthState: report.MakeInterfaceBandwidthState(),
 		cacheKey:                cacheKey,
-		agentConfig:             agentConfig,
 	}
 
 	d.readTagsFromCache()
@@ -247,19 +244,11 @@ func (d *DeviceCheck) setDeviceHostExternalTags() {
 	if deviceHostname == "" || err != nil {
 		return
 	}
-	agentTags := d.buildExternalTags()
+	agentTags := configUtils.GetConfiguredTags(pkgconfigsetup.Datadog(), false)
 	log.Debugf("Set external tags for device host, host=`%s`, agentTags=`%v`", deviceHostname, agentTags)
 	externalhost.SetExternalTags(deviceHostname, common.SnmpExternalTagsSourceType, agentTags)
 }
 
-func (d *DeviceCheck) buildExternalTags() []string {
-	agentTags := configUtils.GetConfiguredTags(d.agentConfig, false)
-	if haagenthelpers.IsEnabled(d.agentConfig) {
-		agentTags = append(agentTags, haagenthelpers.GetHaAgentTags(d.agentConfig)...)
-	}
-	return agentTags
-}
-
 func (d *DeviceCheck) getValuesAndTags() (bool, []string, *valuestore.ResultValueStore, error) {
 	var deviceReachable bool
 	var checkErrors []string

pkg/collector/corechecks/snmp/internal/devicecheck/devicecheck_test.go

@@ -17,7 +17,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 
-	agentconfig "github.com/DataDog/datadog-agent/comp/core/config"
 	"github.com/DataDog/datadog-agent/pkg/aggregator/mocksender"
 	"github.com/DataDog/datadog-agent/pkg/metrics/servicecheck"
 	"github.com/DataDog/datadog-agent/pkg/version"
@@ -58,7 +57,7 @@ profiles:
 	config, err := checkconfig.NewCheckConfig(rawInstanceConfig, rawInitConfig)
 	assert.Nil(t, err)
 
-	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory, agentconfig.NewMock(t))
+	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory)
 	assert.Nil(t, err)
 
 	sender := mocksender.NewMockSender("123") // required to initiate aggregator
@@ -198,7 +197,7 @@ global_metrics:
 	config, err := checkconfig.NewCheckConfig(rawInstanceConfig, rawInitConfig)
 	assert.Nil(t, err)
 
-	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory, agentconfig.NewMock(t))
+	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory)
 	assert.Nil(t, err)
 
 	sender := mocksender.NewMockSender("123") // required to initiate aggregator
@@ -247,7 +246,7 @@ community_string: public
 	config, err := checkconfig.NewCheckConfig(rawInstanceConfig, rawInitConfig)
 	assert.Nil(t, err)
 
-	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", session.NewMockSession, agentconfig.NewMock(t))
+	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", session.NewMockSession)
 	assert.Nil(t, err)
 
 	sender := mocksender.NewMockSender("123") // required to initiate aggregator
@@ -278,7 +277,7 @@ community_string: public
 	config, err := checkconfig.NewCheckConfig(rawInstanceConfig, rawInitConfig)
 	assert.Nil(t, err)
 
-	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", session.NewMockSession, agentconfig.NewMock(t))
+	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", session.NewMockSession)
 	assert.Nil(t, err)
 
 	hostname, err := deviceCk.GetDeviceHostname()
@@ -344,7 +343,7 @@ profiles:
 	config, err := checkconfig.NewCheckConfig(rawInstanceConfig, rawInitConfig)
 	assert.Nil(t, err)
 
-	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory, agentconfig.NewMock(t))
+	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory)
 	assert.Nil(t, err)
 
 	snmpTags := []string{"snmp_device:1.2.3.4", "device_ip:1.2.3.4", "device_id:default:1.2.3.4", "snmp_profile:f5-big-ip", "device_vendor:f5", "snmp_host:foo_sys_name",
@@ -649,7 +648,7 @@ profiles:
 	config, err := checkconfig.NewCheckConfig(rawInstanceConfig, rawInitConfig)
 	assert.Nil(t, err)
 
-	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory, agentconfig.NewMock(t))
+	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory)
 	assert.Nil(t, err)
 
 	sender := mocksender.NewMockSender("123") // required to initiate aggregator
@@ -696,7 +695,7 @@ profiles:
 	config, err := checkconfig.NewCheckConfig(rawInstanceConfig, rawInitConfig)
 	assert.Nil(t, err)
 
-	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory, agentconfig.NewMock(t))
+	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory)
 	assert.Nil(t, err)
 
 	// override pinger with mock pinger
@@ -847,7 +846,7 @@ profiles:
 	config, err := checkconfig.NewCheckConfig(rawInstanceConfig, rawInitConfig)
 	assert.Nil(t, err)
 
-	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory, agentconfig.NewMock(t))
+	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory)
 	assert.Nil(t, err)
 
 	// override pinger with mock pinger
@@ -968,37 +967,3 @@ profiles:
 	sender.AssertNotCalled(t, "Gauge", pingAvgRttMetric, mock.Anything, mock.Anything, mock.Anything)
 	sender.AssertNotCalled(t, "Gauge", pingPacketLoss, mock.Anything, mock.Anything, mock.Anything)
 }
-
-func TestDeviceCheck_buildExternalTags(t *testing.T) {
-	// GIVEN
-	profile.SetConfdPathAndCleanProfiles()
-	sess := session.CreateFakeSession()
-	sessionFactory := func(*checkconfig.CheckConfig) (session.Session, error) {
-		return sess, nil
-	}
-
-	// language=yaml
-	rawInstanceConfig := []byte(`
-ip_address: 1.2.3.4
-community_string: public
-collect_topology: false
-`)
-	// language=yaml
-	rawInitConfig := []byte(``)
-
-	config, err := checkconfig.NewCheckConfig(rawInstanceConfig, rawInitConfig)
-	assert.Nil(t, err)
-
-	cfg := agentconfig.NewMock(t)
-	cfg.SetWithoutSource("ha_agent.enabled", true)
-	cfg.SetWithoutSource("ha_agent.group", "my-group")
-
-	deviceCk, err := NewDeviceCheck(config, "1.2.3.4", sessionFactory, cfg)
-	assert.Nil(t, err)
-
-	// WHEN
-	externalTags := deviceCk.buildExternalTags()
-
-	// THEN
-	assert.Equal(t, []string{"agent_group:my-group"}, externalTags)
-}

pkg/collector/corechecks/snmp/internal/discovery/discovery.go

@@ -14,7 +14,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/DataDog/datadog-agent/comp/core/config"
 	"github.com/DataDog/datadog-agent/pkg/persistentcache"
 	"github.com/DataDog/datadog-agent/pkg/util/log"
 	"go.uber.org/atomic"
@@ -44,7 +43,6 @@ type Discovery struct {
 	discoveredDevices map[checkconfig.DeviceDigest]Device
 
 	sessionFactory session.Factory
-	agentConfig    config.Component
 }
 
 // Device implements and store results from the Service interface for the SNMP listener
@@ -239,7 +237,7 @@ func (d *Discovery) getDevicesFound() []string {
 }
 
 func (d *Discovery) createDevice(deviceDigest checkconfig.DeviceDigest, subnet *snmpSubnet, deviceIP string, writeCache bool) {
-	deviceCk, err := devicecheck.NewDeviceCheck(subnet.config, deviceIP, d.sessionFactory, d.agentConfig)
+	deviceCk, err := devicecheck.NewDeviceCheck(subnet.config, deviceIP, d.sessionFactory)
 	if err != nil {
 		// should not happen since the deviceCheck is expected to be valid at this point
 		// and are only changing the device ip
@@ -337,12 +335,11 @@ func (d *Discovery) writeCache(subnet *snmpSubnet) {
 }
 
 // NewDiscovery return a new Discovery instance
-func NewDiscovery(config *checkconfig.CheckConfig, sessionFactory session.Factory, agentConfig config.Component) *Discovery {
+func NewDiscovery(config *checkconfig.CheckConfig, sessionFactory session.Factory) *Discovery {
 	return &Discovery{
 		discoveredDevices: make(map[checkconfig.DeviceDigest]Device),
 		stop:              make(chan struct{}),
 		config:            config,
 		sessionFactory:    sessionFactory,
-		agentConfig:       agentConfig,
 	}
 }

pkg/collector/corechecks/snmp/internal/discovery/discovery_test.go

@@ -15,7 +15,6 @@ import (
 	"github.com/gosnmp/gosnmp"
 	"github.com/stretchr/testify/assert"
 
-	agentconfig "github.com/DataDog/datadog-agent/comp/core/config"
 	"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/internal/checkconfig"
 	"github.com/DataDog/datadog-agent/pkg/collector/corechecks/snmp/internal/session"
 	pkgconfigsetup "github.com/DataDog/datadog-agent/pkg/config/setup"
@@ -60,7 +59,7 @@ func TestDiscovery(t *testing.T) {
 		DiscoveryWorkers:   1,
 		IgnoredIPAddresses: map[string]bool{"192.168.0.5": true},
 	}
-	discovery := NewDiscovery(checkConfig, sessionFactory, agentconfig.NewMock(t))
+	discovery := NewDiscovery(checkConfig, sessionFactory)
 	discovery.Start()
 	assert.NoError(t, waitForDiscoveredDevices(discovery, 7, 2*time.Second))
 	discovery.Stop()
@@ -110,7 +109,7 @@ func TestDiscoveryCache(t *testing.T) {
 		DiscoveryInterval: 3600,
 		DiscoveryWorkers:  1,
 	}
-	discovery := NewDiscovery(checkConfig, sessionFactory, agentconfig.NewMock(t))
+	discovery := NewDiscovery(checkConfig, sessionFactory)
 	discovery.Start()
 	assert.NoError(t, waitForDiscoveredDevices(discovery, 4, 2*time.Second))
 	discovery.Stop()
@@ -142,7 +141,7 @@ func TestDiscoveryCache(t *testing.T) {
 		DiscoveryInterval: 3600,
 		DiscoveryWorkers:  0, // no workers, the devices will be loaded from cache
 	}
-	discovery2 := NewDiscovery(checkConfig, sessionFactory, agentconfig.NewMock(t))
+	discovery2 := NewDiscovery(checkConfig, sessionFactory)
 	discovery2.Start()
 	assert.NoError(t, waitForDiscoveredDevices(discovery2, 4, 2*time.Second))
 	discovery2.Stop()
@@ -181,7 +180,7 @@ func TestDiscoveryTicker(t *testing.T) {
 		DiscoveryInterval: 1,
 		DiscoveryWorkers:  1,
 	}
-	discovery := NewDiscovery(checkConfig, sessionFactory, agentconfig.NewMock(t))
+	discovery := NewDiscovery(checkConfig, sessionFactory)
 	discovery.Start()
 	time.Sleep(1500 * time.Millisecond)
 	discovery.Stop()
@@ -228,7 +227,7 @@ func TestDiscovery_checkDevice(t *testing.T) {
 	}
 
 	var sess *session.MockSession
-	discovery := NewDiscovery(checkConfig, session.NewMockSession, agentconfig.NewMock(t))
+	discovery := NewDiscovery(checkConfig, session.NewMockSession)
 
 	checkDeviceOnce := func() {
 		sess = session.CreateMockSession()
@@ -316,7 +315,7 @@ func TestDiscovery_createDevice(t *testing.T) {
 		DiscoveryAllowedFailures: 3,
 		Namespace:                "default",
 	}
-	discovery := NewDiscovery(checkConfig, session.NewMockSession, agentconfig.NewMock(t))
+	discovery := NewDiscovery(checkConfig, session.NewMockSession)
 	ipAddr, ipNet, err := net.ParseCIDR(checkConfig.Network)
 	assert.Nil(t, err)
 	startingIP := ipAddr.Mask(ipNet.Mask)