check if monitor accepts endpoints before setting host/port (#1107)

Only select smart-agent monitors can be created via endpoints. This is
defined as a struct tag on the monitor configs "acceptsEndpoints". In
the case where we attempt to set Host/Port on a monitor that has it's
"acceptsEndpoints" value set to false, an error occurs and we do not
create the monitor.

This change would create the monitor but avoid setting the
Host/Port fields. This allows the receiver creator to
create any smart-agent monitor.

CHANGELOG.md

@@ -13,6 +13,10 @@ This Splunk OpenTelemetry Collector release includes changes from the [opentelem
 - [`docker_observer`](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/extension/observer/dockerobserver) to detect and create container endpoints, to be used with the [`receiver_creator`](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/receivercreator) (#1044)
 - [`ecs_task_observer`](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/extension/observer/ecstaskobserver) to detect and create ECS task container endpoints, to be used with the [`receiver_creator`](https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/receivercreator) (#1125)
 
+### 🧰 Bug fixes 🧰
+
+- [`smartagent` receiver](https://github.com/signalfx/splunk-otel-collector/tree/main/internal/receiver/smartagentreceiver) will now attempt to create _any_ monitor from a Receiver Creator instance, disregarding its provided `endpoint`. Previously would error out if a monitor did not accept endpoints ([#1107](https://github.com/signalfx/splunk-otel-collector/pull/1107))
+
 ## v0.41.0
 
 This Splunk OpenTelemetry Collector release includes changes from the [opentelemetry-collector v0.41.0](https://github.com/open-telemetry/opentelemetry-collector/releases/tag/v0.41.0) and the [opentelemetry-collector-contrib v0.41.0](https://github.com/open-telemetry/opentelemetry-collector-contrib/releases/tag/v0.41.0) releases.

internal/receiver/smartagentreceiver/config.go

@@ -42,6 +42,7 @@ type Config struct {
 	// Will expand to MonitorCustomConfig Host and Port values if unset.
 	Endpoint         string   `mapstructure:"endpoint"`
 	DimensionClients []string `mapstructure:"dimensionClients"`
+	acceptsEndpoints bool
 }
 
 func (cfg *Config) validate() error {
@@ -110,11 +111,18 @@ func (cfg *Config) Unmarshal(componentParser *config.Map) error {
 		return fmt.Errorf("failed setting Smart Agent Monitor config defaults: %w", err)
 	}
 
-	err = setHostAndPortViaEndpoint(cfg.Endpoint, monitorConfig)
+	cfg.acceptsEndpoints, err = monitorAcceptsEndpoints(monitorConfig)
 	if err != nil {
 		return err
 	}
 
+	if cfg.acceptsEndpoints {
+		err = setHostAndPortViaEndpoint(cfg.Endpoint, monitorConfig)
+		if err != nil {
+			return err
+		}
+	}
+
 	cfg.monitorConfig = monitorConfig.(saconfig.MonitorCustomConfig)
 	return nil
 }
@@ -178,3 +186,13 @@ func setHostAndPortViaEndpoint(endpoint string, monitorConfig interface{}) error
 
 	return nil
 }
+
+// Monitors can only set the "Host" and "Port" fields if they accept endpoints,
+// which is defined as a struct tag for each monitor config.
+func monitorAcceptsEndpoints(monitorConfig interface{}) (bool, error) {
+	field, ok := reflect.TypeOf(monitorConfig).Elem().FieldByName("MonitorConfig")
+	if !ok {
+		return false, fmt.Errorf("could not reflect monitor config, top level MonitorConfig does not exist")
+	}
+	return field.Tag.Get("acceptsEndpoints") == strconv.FormatBool(true), nil
+}

internal/receiver/smartagentreceiver/config_linux_test.go

@@ -61,6 +61,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
 			Port: 6379,
 			URL:  "http://{{.Host}}:{{.Port}}/mod_status?auto",
 		},
+		acceptsEndpoints: true,
 	}, apacheCfg)
 	require.NoError(t, apacheCfg.validate())
 
@@ -80,6 +81,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
 			},
 			ClusterName: "somecluster",
 		},
+		acceptsEndpoints: true,
 	}, kafkaCfg)
 	require.NoError(t, kafkaCfg.validate())
 
@@ -95,6 +97,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
 			Host: "localhost",
 			Port: 5309,
 		},
+		acceptsEndpoints: true,
 	}, memcachedCfg)
 	require.NoError(t, memcachedCfg.validate())
 
@@ -109,6 +112,7 @@ func TestLoadConfigWithLinuxOnlyMonitors(t *testing.T) {
 			},
 			Path: "/status",
 		},
+		acceptsEndpoints: true,
 	}, phpCfg)
 	require.NoError(t, phpCfg.validate())
 }

internal/receiver/smartagentreceiver/config_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/signalfx/signalfx-agent/pkg/monitors/filesystems"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/haproxy"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/kubernetes/volumes"
+	"github.com/signalfx/signalfx-agent/pkg/monitors/nagios"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/prometheusexporter"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/telegraf/common/parser"
 	"github.com/signalfx/signalfx-agent/pkg/monitors/telegraf/monitors/exec"
@@ -74,6 +75,7 @@ func TestLoadConfig(t *testing.T) {
 			SSLVerify: true,
 			Timeout:   timeutil.Duration(5 * time.Second),
 		},
+		acceptsEndpoints: true,
 	}, haproxyCfg)
 	require.NoError(t, haproxyCfg.validate())
 
@@ -90,6 +92,7 @@ func TestLoadConfig(t *testing.T) {
 			Host: "localhost",
 			Port: 6379,
 		},
+		acceptsEndpoints: true,
 	}, redisCfg)
 	require.NoError(t, redisCfg.validate())
 
@@ -106,6 +109,7 @@ func TestLoadConfig(t *testing.T) {
 			Host:         "localhost",
 			Port:         8088,
 		},
+		acceptsEndpoints: true,
 	}, hadoopCfg)
 	require.NoError(t, hadoopCfg.validate())
 
@@ -125,6 +129,7 @@ func TestLoadConfig(t *testing.T) {
 			Port:       5309,
 			MetricPath: "/metrics",
 		},
+		acceptsEndpoints: true,
 	}, etcdCfg)
 	require.NoError(t, etcdCfg.validate())
 
@@ -140,6 +145,7 @@ func TestLoadConfig(t *testing.T) {
 			},
 			DNSLookup: &tr,
 		},
+		acceptsEndpoints: true,
 	}, ntpqCfg)
 	require.NoError(t, ntpqCfg.validate())
 }
@@ -213,6 +219,7 @@ func TestLoadInvalidConfigs(t *testing.T) {
 				DatapointsToExclude: []saconfig.MetricFilter{},
 			},
 		},
+		acceptsEndpoints: true,
 	}, negativeIntervalCfg)
 	err = negativeIntervalCfg.validate()
 	require.Error(t, err)
@@ -231,6 +238,7 @@ func TestLoadInvalidConfigs(t *testing.T) {
 			TelemetryHost: "0.0.0.0",
 			TelemetryPort: 8125,
 		},
+		acceptsEndpoints: true,
 	}, missingRequiredCfg)
 	err = missingRequiredCfg.validate()
 	require.Error(t, err)
@@ -270,6 +278,7 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
 			SSLVerify: true,
 			Timeout:   timeutil.Duration(5 * time.Second),
 		},
+		acceptsEndpoints: true,
 	}, haproxyCfg)
 	require.NoError(t, haproxyCfg.validate())
 
@@ -286,6 +295,7 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
 			Host: "redishost",
 			Port: 6379,
 		},
+		acceptsEndpoints: true,
 	}, redisCfg)
 	require.NoError(t, redisCfg.validate())
 
@@ -303,6 +313,7 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
 			Host:         "localhost",
 			Port:         8088,
 		},
+		acceptsEndpoints: true,
 	}, hadoopCfg)
 	require.NoError(t, hadoopCfg.validate())
 
@@ -323,6 +334,7 @@ func TestLoadConfigWithEndpoints(t *testing.T) {
 			Port:       5555,
 			MetricPath: "/metrics",
 		},
+		acceptsEndpoints: true,
 	}, etcdCfg)
 	require.NoError(t, etcdCfg.validate())
 }
@@ -342,7 +354,9 @@ func TestLoadInvalidConfigWithInvalidEndpoint(t *testing.T) {
 	require.Nil(t, cfg)
 }
 
-func TestLoadInvalidConfigWithUnsupportedEndpoint(t *testing.T) {
+// Even though this smart-agent monitor does not accept endpoints,
+// we can create it without setting Host/Port fields.
+func TestLoadConfigWithUnsupportedEndpoint(t *testing.T) {
 	factories, err := componenttest.NopFactories()
 	assert.Nil(t, err)
 
@@ -351,10 +365,25 @@ func TestLoadInvalidConfigWithUnsupportedEndpoint(t *testing.T) {
 	cfg, err := servicetest.LoadConfig(
 		path.Join(".", "testdata", "unsupported_endpoint.yaml"), factories,
 	)
-	require.Error(t, err)
-	require.EqualError(t, err,
-		`error reading receivers configuration for "smartagent/nagios": unable to set monitor Host field using Endpoint-derived value of localhost: no field Host of type string detected`)
-	require.Nil(t, cfg)
+	require.NoError(t, err)
+	require.NotNil(t, cfg)
+
+	nagiosCfg := cfg.Receivers[config.NewComponentIDWithName(typeStr, "nagios")].(*Config)
+	require.Equal(t, &Config{
+		Endpoint:         "localhost:12345",
+		ReceiverSettings: config.NewReceiverSettings(config.NewComponentIDWithName(typeStr, "nagios")),
+		monitorConfig: &nagios.Config{
+			MonitorConfig: saconfig.MonitorConfig{
+				Type:                "nagios",
+				DatapointsToExclude: []saconfig.MetricFilter{},
+			},
+			Command: "some_command",
+			Service: "some_service",
+			Timeout: 9,
+		},
+		acceptsEndpoints: false,
+	}, nagiosCfg)
+	require.NoError(t, nagiosCfg.validate())
 }
 
 func TestLoadInvalidConfigWithNonArrayDimensionClients(t *testing.T) {

internal/receiver/smartagentreceiver/receiver.go

@@ -112,6 +112,9 @@ func (r *Receiver) Start(_ context.Context, host component.Host) error {
 		monitorType: r.config.monitorConfig.MonitorConfigCore().Type,
 	}, r.logger)
 
+	if !r.config.acceptsEndpoints {
+		r.logger.Info("This Smart Agent monitor does not use Host/Port config fields. If either are set, they will be ignored.", zap.String("monitor_type", monitorType))
+	}
 	r.monitor, err = r.createMonitor(monitorType, host)
 	if err != nil {
 		return fmt.Errorf("failed creating monitor %q: %w", monitorType, err)