Refactor prometheusreceiver config to avoid some custom unmarshal work

.chloggen/refactorprom.yaml

@@ -0,0 +1,22 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: 'breaking'
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: prometheusreceiver
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: Consolidate Config members and remove the need of placeholders.
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [29901]
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: [api]

cmd/otelcontribcol/receivers_test.go

@@ -312,7 +312,7 @@ func TestDefaultReceivers(t *testing.T) {
 			receiver: "prometheus",
 			getConfigFn: func() component.Config {
 				cfg := rcvrFactories["prometheus"].CreateDefaultConfig().(*prometheusreceiver.Config)
-				cfg.PrometheusConfig = &promconfig.Config{
+				cfg.PrometheusConfig = &prometheusreceiver.PromConfig{
 					ScrapeConfigs: []*promconfig.ScrapeConfig{
 						{JobName: "test"},
 					},

exporter/prometheusexporter/end_to_end_test.go

@@ -16,7 +16,6 @@ import (
 	"testing"
 	"time"
 
-	promconfig "github.com/prometheus/prometheus/config"
 	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/exporter/exportertest"
@@ -88,7 +87,7 @@ func TestEndToEndSummarySupport(t *testing.T) {
               static_configs:
                 - targets: ['%s']
         `, srvURL.Host))
-	receiverConfig := new(promconfig.Config)
+	receiverConfig := new(prometheusreceiver.PromConfig)
 	if err = yaml.Unmarshal(yamlConfig, receiverConfig); err != nil {
 		t.Fatal(err)
 	}

exporter/prometheusexporter/go.mod

@@ -10,7 +10,6 @@ require (
 	github.com/prometheus/client_golang v1.18.0
 	github.com/prometheus/client_model v0.5.0
 	github.com/prometheus/common v0.46.0
-	github.com/prometheus/prometheus v0.48.1
 	github.com/stretchr/testify v1.8.4
 	go.opentelemetry.io/collector/component v0.92.1-0.20240112172857-83d463ceba06
 	go.opentelemetry.io/collector/config/confighttp v0.92.1-0.20240112172857-83d463ceba06
@@ -132,6 +131,7 @@ require (
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/prometheus/common/sigv4 v0.1.0 // indirect
 	github.com/prometheus/procfs v0.12.0 // indirect
+	github.com/prometheus/prometheus v0.48.1 // indirect
 	github.com/prometheus/statsd_exporter v0.22.7 // indirect
 	github.com/rs/cors v1.10.1 // indirect
 	github.com/scaleway/scaleway-sdk-go v1.0.0-beta.21 // indirect

receiver/prometheusreceiver/config.go

@@ -16,25 +16,14 @@ import (
 	promconfig "github.com/prometheus/prometheus/config"
 	promHTTP "github.com/prometheus/prometheus/discovery/http"
 	"github.com/prometheus/prometheus/discovery/kubernetes"
-	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/confmap"
-	"go.uber.org/zap"
 	"gopkg.in/yaml.v2"
 )
 
-const (
-	// The key for Prometheus scraping configs.
-	prometheusConfigKey = "config"
-
-	// keys to access the http_sd_config from config root
-	targetAllocatorConfigKey       = "target_allocator"
-	targetAllocatorHTTPSDConfigKey = "http_sd_config"
-)
-
 // Config defines configuration for Prometheus receiver.
 type Config struct {
-	PrometheusConfig   *promconfig.Config `mapstructure:"-"`
-	TrimMetricSuffixes bool               `mapstructure:"trim_metric_suffixes"`
+	PrometheusConfig   *PromConfig `mapstructure:"config"`
+	TrimMetricSuffixes bool        `mapstructure:"trim_metric_suffixes"`
 	// UseStartTimeMetric enables retrieving the start time of all counter metrics
 	// from the process_start_time_seconds metric. This is only correct if all counters on that endpoint
 	// started after the process start time, and the process is the only actor exporting the metric after
@@ -47,94 +36,74 @@ type Config struct {
 	// ReportExtraScrapeMetrics - enables reporting of additional metrics for Prometheus client like scrape_body_size_bytes
 	ReportExtraScrapeMetrics bool `mapstructure:"report_extra_scrape_metrics"`
 
-	TargetAllocator *targetAllocator `mapstructure:"target_allocator"`
-
-	// ConfigPlaceholder is just an entry to make the configuration pass a check
-	// that requires that all keys present in the config actually exist on the
-	// structure, ie.: it will error if an unknown key is present.
-	ConfigPlaceholder any `mapstructure:"config"`
+	TargetAllocator *TargetAllocator `mapstructure:"target_allocator"`
 
 	// EnableProtobufNegotiation allows the collector to set the scraper option for
 	// protobuf negotiation when conferring with a prometheus client.
 	EnableProtobufNegotiation bool `mapstructure:"enable_protobuf_negotiation"`
 }
 
-type targetAllocator struct {
-	Endpoint    string        `mapstructure:"endpoint"`
-	Interval    time.Duration `mapstructure:"interval"`
-	CollectorID string        `mapstructure:"collector_id"`
-	// ConfigPlaceholder is just an entry to make the configuration pass a check
-	// that requires that all keys present in the config actually exist on the
-	// structure, ie.: it will error if an unknown key is present.
-	ConfigPlaceholder any                `mapstructure:"http_sd_config"`
-	HTTPSDConfig      *promHTTP.SDConfig `mapstructure:"-"`
+// Validate checks the receiver configuration is valid.
+func (cfg *Config) Validate() error {
+	if (cfg.PrometheusConfig == nil || len(cfg.PrometheusConfig.ScrapeConfigs) == 0) && cfg.TargetAllocator == nil {
+		return errors.New("no Prometheus scrape_configs or target_allocator set")
+	}
+	return nil
 }
 
-var _ component.Config = (*Config)(nil)
-var _ confmap.Unmarshaler = (*Config)(nil)
-
-func checkFile(fn string) error {
-	// Nothing set, nothing to error on.
-	if fn == "" {
-		return nil
-	}
-	_, err := os.Stat(fn)
-	return err
+type TargetAllocator struct {
+	Endpoint     string            `mapstructure:"endpoint"`
+	Interval     time.Duration     `mapstructure:"interval"`
+	CollectorID  string            `mapstructure:"collector_id"`
+	HTTPSDConfig *PromHTTPSDConfig `mapstructure:"http_sd_config"`
 }
 
-func checkTLSConfig(tlsConfig commonconfig.TLSConfig) error {
-	if err := checkFile(tlsConfig.CertFile); err != nil {
-		return fmt.Errorf("error checking client cert file %q: %w", tlsConfig.CertFile, err)
+func (cfg *TargetAllocator) Validate() error {
+	// ensure valid endpoint
+	if _, err := url.ParseRequestURI(cfg.Endpoint); err != nil {
+		return fmt.Errorf("TargetAllocator endpoint is not valid: %s", cfg.Endpoint)
 	}
-	if err := checkFile(tlsConfig.KeyFile); err != nil {
-		return fmt.Errorf("error checking client key file %q: %w", tlsConfig.KeyFile, err)
+	// ensure valid collectorID without variables
+	if cfg.CollectorID == "" || strings.Contains(cfg.CollectorID, "${") {
+		return fmt.Errorf("CollectorID is not a valid ID")
 	}
+
 	return nil
 }
 
-// Validate checks the receiver configuration is valid.
-func (cfg *Config) Validate() error {
-	promConfig := cfg.PrometheusConfig
-	if (promConfig == nil || len(promConfig.ScrapeConfigs) == 0) && cfg.TargetAllocator == nil {
-		return errors.New("no Prometheus scrape_configs or target_allocator set")
-	}
+// PromConfig is a redeclaration of promconfig.Config because we need custom unmarshaling
+// as prometheus "config" uses `yaml` tags.
+type PromConfig promconfig.Config
 
-	err := validatePromConfig(promConfig)
-	if err != nil {
-		return err
-	}
+var _ confmap.Unmarshaler = (*PromConfig)(nil)
 
-	if cfg.TargetAllocator != nil {
-		err := cfg.validateTargetAllocatorConfig()
-		if err != nil {
-			return err
-		}
+func (cfg *PromConfig) Unmarshal(componentParser *confmap.Conf) error {
+	cfgMap := componentParser.ToStringMap()
+	if len(cfgMap) == 0 {
+		return nil
 	}
-	return nil
+	return unmarshalYAML(cfgMap, (*promconfig.Config)(cfg))
 }
 
-func validatePromConfig(promConfig *promconfig.Config) error {
-	if promConfig == nil {
-		return nil
-	}
+func (cfg *PromConfig) Validate() error {
 	// Reject features that Prometheus supports but that the receiver doesn't support:
 	// See:
 	// * https://github.com/open-telemetry/opentelemetry-collector/issues/3863
 	// * https://github.com/open-telemetry/wg-prometheus/issues/3
 	unsupportedFeatures := make([]string, 0, 4)
-	if len(promConfig.RemoteWriteConfigs) != 0 {
+	if len(cfg.RemoteWriteConfigs) != 0 {
 		unsupportedFeatures = append(unsupportedFeatures, "remote_write")
 	}
-	if len(promConfig.RemoteReadConfigs) != 0 {
+	if len(cfg.RemoteReadConfigs) != 0 {
 		unsupportedFeatures = append(unsupportedFeatures, "remote_read")
 	}
-	if len(promConfig.RuleFiles) != 0 {
+	if len(cfg.RuleFiles) != 0 {
 		unsupportedFeatures = append(unsupportedFeatures, "rule_files")
 	}
-	if len(promConfig.AlertingConfig.AlertRelabelConfigs) != 0 {
+	if len(cfg.AlertingConfig.AlertRelabelConfigs) != 0 {
 		unsupportedFeatures = append(unsupportedFeatures, "alert_config.relabel_configs")
 	}
-	if len(promConfig.AlertingConfig.AlertmanagerConfigs) != 0 {
+	if len(cfg.AlertingConfig.AlertmanagerConfigs) != 0 {
 		unsupportedFeatures = append(unsupportedFeatures, "alert_config.alertmanagers")
 	}
 	if len(unsupportedFeatures) != 0 {
@@ -143,7 +112,7 @@ func validatePromConfig(promConfig *promconfig.Config) error {
 		return fmt.Errorf("unsupported features:\n\t%s", strings.Join(unsupportedFeatures, "\n\t"))
 	}
 
-	for _, sc := range promConfig.ScrapeConfigs {
+	for _, sc := range cfg.ScrapeConfigs {
 		if sc.HTTPClientConfig.Authorization != nil {
 			if err := checkFile(sc.HTTPClientConfig.Authorization.CredentialsFile); err != nil {
 				return fmt.Errorf("error checking authorization credentials file %q: %w", sc.HTTPClientConfig.Authorization.CredentialsFile, err)
@@ -165,84 +134,49 @@ func validatePromConfig(promConfig *promconfig.Config) error {
 	return nil
 }
 
-func (cfg *Config) validateTargetAllocatorConfig() error {
-	// validate targetAllocator
-	targetAllocatorConfig := cfg.TargetAllocator
-	if targetAllocatorConfig == nil {
-		return nil
-	}
-	// ensure valid endpoint
-	if _, err := url.ParseRequestURI(targetAllocatorConfig.Endpoint); err != nil {
-		return fmt.Errorf("TargetAllocator endpoint is not valid: %s", targetAllocatorConfig.Endpoint)
-	}
-	// ensure valid collectorID without variables
-	if targetAllocatorConfig.CollectorID == "" || strings.Contains(targetAllocatorConfig.CollectorID, "${") {
-		return fmt.Errorf("CollectorID is not a valid ID")
-	}
+// PromHTTPSDConfig is a redeclaration of promHTTP.SDConfig because we need custom unmarshaling
+// as prometheus "config" uses `yaml` tags.
+type PromHTTPSDConfig promHTTP.SDConfig
 
-	return nil
-}
+var _ confmap.Unmarshaler = (*PromHTTPSDConfig)(nil)
 
-// Unmarshal a config.Parser into the config struct.
-func (cfg *Config) Unmarshal(componentParser *confmap.Conf) error {
-	if componentParser == nil {
+func (cfg *PromHTTPSDConfig) Unmarshal(componentParser *confmap.Conf) error {
+	cfgMap := componentParser.ToStringMap()
+	if len(cfgMap) == 0 {
 		return nil
 	}
-	// We need custom unmarshaling because prometheus "config" subkey defines its own
-	// YAML unmarshaling routines so we need to do it explicitly.
+	cfgMap["url"] = "http://placeholder" // we have to set it as else marshaling will fail
+	return unmarshalYAML(cfgMap, (*promHTTP.SDConfig)(cfg))
+}
 
-	err := componentParser.Unmarshal(cfg)
+func unmarshalYAML(in map[string]any, out any) error {
+	yamlOut, err := yaml.Marshal(in)
 	if err != nil {
-		return fmt.Errorf("prometheus receiver failed to parse config: %w", err)
+		return fmt.Errorf("prometheus receiver: failed to marshal config to yaml: %w", err)
 	}
 
-	// Unmarshal prometheus's config values. Since prometheus uses `yaml` tags, so use `yaml`.
-	promCfg, err := componentParser.Sub(prometheusConfigKey)
-	if err != nil || len(promCfg.ToStringMap()) == 0 {
-		return err
-	}
-	out, err := yaml.Marshal(promCfg.ToStringMap())
+	err = yaml.UnmarshalStrict(yamlOut, out)
 	if err != nil {
-		return fmt.Errorf("prometheus receiver failed to marshal config to yaml: %w", err)
+		return fmt.Errorf("prometheus receiver: failed to unmarshal yaml to prometheus config object: %w", err)
 	}
+	return nil
+}
 
-	err = yaml.UnmarshalStrict(out, &cfg.PrometheusConfig)
-	if err != nil {
-		return fmt.Errorf("prometheus receiver failed to unmarshal yaml to prometheus config: %w", err)
+func checkFile(fn string) error {
+	// Nothing set, nothing to error on.
+	if fn == "" {
+		return nil
 	}
+	_, err := os.Stat(fn)
+	return err
+}
 
-	// Unmarshal targetAllocator configs
-	targetAllocatorCfg, err := componentParser.Sub(targetAllocatorConfigKey)
-	if err != nil {
-		return err
-	}
-	targetAllocatorHTTPSDCfg, err := targetAllocatorCfg.Sub(targetAllocatorHTTPSDConfigKey)
-	if err != nil {
-		return err
+func checkTLSConfig(tlsConfig commonconfig.TLSConfig) error {
+	if err := checkFile(tlsConfig.CertFile); err != nil {
+		return fmt.Errorf("error checking client cert file %q: %w", tlsConfig.CertFile, err)
 	}
-
-	targetAllocatorHTTPSDMap := targetAllocatorHTTPSDCfg.ToStringMap()
-	if len(targetAllocatorHTTPSDMap) != 0 {
-		targetAllocatorHTTPSDMap["url"] = "http://placeholder" // we have to set it as else the marshal will fail
-		httpSDConf, err := yaml.Marshal(targetAllocatorHTTPSDMap)
-		if err != nil {
-			return fmt.Errorf("prometheus receiver failed to marshal config to yaml: %w", err)
-		}
-		err = yaml.UnmarshalStrict(httpSDConf, &cfg.TargetAllocator.HTTPSDConfig)
-		if err != nil {
-			return fmt.Errorf("prometheus receiver failed to unmarshal yaml to prometheus config: %w", err)
-		}
+	if err := checkFile(tlsConfig.KeyFile); err != nil {
+		return fmt.Errorf("error checking client key file %q: %w", tlsConfig.KeyFile, err)
 	}
-
 	return nil
 }
-
-func configWarnings(logger *zap.Logger, cfg *Config) {
-	for _, sc := range cfg.PrometheusConfig.ScrapeConfigs {
-		for _, rc := range sc.MetricRelabelConfigs {
-			if rc.TargetLabel == "__name__" {
-				logger.Warn("metric renaming using metric_relabel_configs will result in unknown-typed metrics without a unit or description", zap.String("job", sc.JobName))
-			}
-		}
-	}
-}