Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 0 additions & 26 deletions comp/haagent/helpers/helpers.go

This file was deleted.

33 changes: 0 additions & 33 deletions comp/haagent/helpers/helpers_test.go

This file was deleted.

5 changes: 2 additions & 3 deletions comp/haagent/impl/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"),
}
}
5 changes: 2 additions & 3 deletions comp/metadata/host/hostimpl/hosttags/tags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
Loading
Loading