Fix prometheusreceiver TA/scrape_config validation logic

Signed-off-by: Anthony J Mirabella <a9@aneurysm9.com>
open-telemetry · Dec 19, 2023 · e2c168e · e2c168e
1 parent 1b56c29
commit e2c168e
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 6 deletions.
diff --git a/.chloggen/fix_promTACfgValidation.yaml b/.chloggen/fix_promTACfgValidation.yaml
@@ -0,0 +1,6 @@
+change_type: 'bug_fix'
+component: 'prometheusreceiver'
+note: Fix configuration validation to allow specification of Target Allocator configuration without providing scrape configurations
+issues: [30135]
+subtext:
+change_logs: []
diff --git a/receiver/prometheusreceiver/config.go b/receiver/prometheusreceiver/config.go
@@ -95,11 +95,9 @@ func checkTLSConfig(tlsConfig commonconfig.TLSConfig) error {
 // Validate checks the receiver configuration is valid.
 func (cfg *Config) Validate() error {
 	promConfig := cfg.PrometheusConfig
-	if promConfig != nil {
-		err := cfg.validatePromConfig(promConfig)
-		if err != nil {
-			return err
-		}
+	err := cfg.validatePromConfig(promConfig)
+	if err != nil {
+		return err
 	}
 
 	if cfg.TargetAllocator != nil {
@@ -112,8 +110,11 @@ func (cfg *Config) Validate() error {
 }
 
 func (cfg *Config) validatePromConfig(promConfig *promconfig.Config) error {
-	if len(promConfig.ScrapeConfigs) == 0 && cfg.TargetAllocator == nil {
+	if (promConfig == nil || len(promConfig.ScrapeConfigs) == 0) && cfg.TargetAllocator == nil {
 		return errors.New("no Prometheus scrape_configs or target_allocator set")
+	} else if promConfig == nil {
+		// nothing to validate
+		return nil
 	}
 
 	// Reject features that Prometheus supports but that the receiver doesn't support:

diff --git a/receiver/prometheusreceiver/config_test.go b/receiver/prometheusreceiver/config_test.go
@@ -66,6 +66,7 @@ func TestLoadTargetAllocatorConfig(t *testing.T) {
 	sub, err := cm.Sub(component.NewIDWithName(metadata.Type, "").String())
 	require.NoError(t, err)
 	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
 
 	r0 := cfg.(*Config)
 	assert.NotNil(t, r0.PrometheusConfig)
@@ -77,6 +78,7 @@ func TestLoadTargetAllocatorConfig(t *testing.T) {
 	require.NoError(t, err)
 	cfg = factory.CreateDefaultConfig()
 	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
 
 	r1 := cfg.(*Config)
 	assert.NotNil(t, r0.PrometheusConfig)
@@ -92,6 +94,7 @@ func TestLoadTargetAllocatorConfig(t *testing.T) {
 	require.NoError(t, err)
 	cfg = factory.CreateDefaultConfig()
 	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
 
 	r2 := cfg.(*Config)
 	assert.Equal(t, 1, len(r2.PrometheusConfig.ScrapeConfigs))
@@ -110,6 +113,36 @@ func TestLoadConfigFailsOnUnknownSection(t *testing.T) {
 	require.Error(t, component.UnmarshalConfig(sub, cfg))
 }
 
+func TestLoadConfigFailsOnNoPrometheusOrTAConfig(t *testing.T) {
+	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "invalid-config-prometheus-non-existent-scrape-config.yaml"))
+	require.NoError(t, err)
+	factory := NewFactory()
+
+	cfg := factory.CreateDefaultConfig()
+	sub, err := cm.Sub(component.NewIDWithName(metadata.Type, "").String())
+	require.NoError(t, err)
+	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.ErrorContains(t, component.ValidateConfig(cfg), "no Prometheus scrape_configs or target_allocator set")
+
+	cfg = factory.CreateDefaultConfig()
+	sub, err = cm.Sub(component.NewIDWithName(metadata.Type, "withConfigAndTA").String())
+	require.NoError(t, err)
+	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
+
+	cfg = factory.CreateDefaultConfig()
+	sub, err = cm.Sub(component.NewIDWithName(metadata.Type, "withOnlyTA").String())
+	require.NoError(t, err)
+	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
+
+	cfg = factory.CreateDefaultConfig()
+	sub, err = cm.Sub(component.NewIDWithName(metadata.Type, "withOnlyScrape").String())
+	require.NoError(t, err)
+	require.NoError(t, component.UnmarshalConfig(sub, cfg))
+	require.NoError(t, component.ValidateConfig(cfg))
+}
+
 // As one of the config parameters is consuming prometheus
 // configuration as a subkey, ensure that invalid configuration
 // within the subkey will also raise an error.

diff --git a/...ver/prometheusreceiver/testdata/invalid-config-prometheus-non-existent-scrape-config.yaml b/...ver/prometheusreceiver/testdata/invalid-config-prometheus-non-existent-scrape-config.yaml
@@ -0,0 +1,19 @@
+prometheus:
+prometheus/withConfigAndTA:
+  target_allocator:
+    endpoint: http://localhost:8080
+    interval: 30s
+    collector_id: collector-1
+  config:
+    global:
+      scrape_interval: 30s
+prometheus/withOnlyTA:
+  target_allocator:
+    endpoint: http://localhost:8080
+    interval: 30s
+    collector_id: collector-1
+prometheus/withOnlyScrape:
+  config:
+    scrape_configs:
+      - job_name: 'demo'
+        scrape_interval: 5s