From c57013b5e4cda78ce2cd4a30325c529cfb5e8dae Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Fri, 17 Dec 2021 15:20:21 -0500 Subject: [PATCH 1/2] pkg/integrations: decouple common config from integrations --- pkg/integrations/agent/agent.go | 9 +- pkg/integrations/cadvisor/common.go | 9 -- .../consul_exporter/consul_exporter.go | 8 -- .../dnsmasq_exporter/dnsmasq_exporter.go | 8 -- .../elasticsearch_exporter.go | 11 -- .../github_exporter/github_exporter.go | 9 -- pkg/integrations/integration.go | 8 +- .../kafka_exporter/kafka_exporter.go | 9 -- pkg/integrations/manager.go | 16 +-- pkg/integrations/manager_test.go | 42 ++++--- .../memcached_exporter/memcached_exporter.go | 8 -- .../mongodb_exporter/mongodb_exporter.go | 9 -- .../mysqld_exporter/mysqld-exporter.go | 8 -- pkg/integrations/node_exporter/config.go | 8 -- .../postgres_exporter/postgres_exporter.go | 9 -- pkg/integrations/process_exporter/config.go | 7 -- .../redis_exporter/redis_exporter.go | 9 -- pkg/integrations/register.go | 115 ++++++++++------- pkg/integrations/register_test.go | 9 +- .../statsd_exporter/statsd_exporter.go | 7 -- pkg/integrations/windows_exporter/config.go | 8 -- pkg/util/yaml.go | 119 ++++++++++++++++++ 22 files changed, 225 insertions(+), 220 deletions(-) create mode 100644 pkg/util/yaml.go diff --git a/pkg/integrations/agent/agent.go b/pkg/integrations/agent/agent.go index 4457485d998d..3976f827953c 100644 --- a/pkg/integrations/agent/agent.go +++ b/pkg/integrations/agent/agent.go @@ -14,20 +14,13 @@ import ( ) // Config controls the Agent integration. -type Config struct { - Common config.Common `yaml:",inline"` -} +type Config struct{} // Name returns the name of the integration that this config represents. func (c *Config) Name() string { return "agent" } -// CommonConfig returns the common settings shared across all integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the hostname of the machine. func (c *Config) InstanceKey(agentKey string) (string, error) { return agentKey, nil diff --git a/pkg/integrations/cadvisor/common.go b/pkg/integrations/cadvisor/common.go index 6edf9a699534..6d5d0b5eb70a 100644 --- a/pkg/integrations/cadvisor/common.go +++ b/pkg/integrations/cadvisor/common.go @@ -4,7 +4,6 @@ import ( "time" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" ) const name = "cadvisor" @@ -34,8 +33,6 @@ var DefaultConfig Config = Config{ // Config controls cadvisor type Config struct { - Common config.Common `yaml:",inline"` - // Common cadvisor config options // StoreContainerLabels converts container labels and environment variables into labels on prometheus metrics for each container. If false, then only metrics exported are container name, first alias, and image name. StoreContainerLabels bool `yaml:"store_container_labels,omitempty"` @@ -106,12 +103,6 @@ func (c *Config) Name() string { return name } -// CommonConfig returns the common settings shared across all configs for -// integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the agentKey func (c *Config) InstanceKey(agentKey string) (string, error) { return agentKey, nil diff --git a/pkg/integrations/consul_exporter/consul_exporter.go b/pkg/integrations/consul_exporter/consul_exporter.go index 488af593e309..b8f59406508d 100644 --- a/pkg/integrations/consul_exporter/consul_exporter.go +++ b/pkg/integrations/consul_exporter/consul_exporter.go @@ -8,7 +8,6 @@ import ( "github.com/go-kit/log" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" consul_api "github.com/hashicorp/consul/api" "github.com/prometheus/consul_exporter/pkg/exporter" ) @@ -24,8 +23,6 @@ var DefaultConfig = Config{ // Config controls the consul_exporter integration. type Config struct { - Common config.Common `yaml:",inline"` - Server string `yaml:"server,omitempty"` CAFile string `yaml:"ca_file,omitempty"` CertFile string `yaml:"cert_file,omitempty"` @@ -55,11 +52,6 @@ func (c *Config) Name() string { return "consul_exporter" } -// CommonConfig returns the common set of settings for this integration. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the hostname:port of the Consul server. func (c *Config) InstanceKey(agentKey string) (string, error) { u, err := url.Parse(c.Server) diff --git a/pkg/integrations/dnsmasq_exporter/dnsmasq_exporter.go b/pkg/integrations/dnsmasq_exporter/dnsmasq_exporter.go index b8a6dc058dc8..6df60bd5c7ca 100644 --- a/pkg/integrations/dnsmasq_exporter/dnsmasq_exporter.go +++ b/pkg/integrations/dnsmasq_exporter/dnsmasq_exporter.go @@ -5,7 +5,6 @@ import ( "github.com/go-kit/log" "github.com/google/dnsmasq_exporter/collector" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" "github.com/miekg/dns" ) @@ -17,8 +16,6 @@ var DefaultConfig Config = Config{ // Config controls the dnsmasq_exporter integration. type Config struct { - Common config.Common `yaml:",inline"` - // DnsmasqAddress is the address of the dnsmasq server (host:port). DnsmasqAddress string `yaml:"dnsmasq_address,omitempty"` @@ -31,11 +28,6 @@ func (c *Config) Name() string { return "dnsmasq_exporter" } -// CommonConfig returns the set of common settings shared across all integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the address of the dnsmasq server. func (c *Config) InstanceKey(agentKey string) (string, error) { return c.DnsmasqAddress, nil diff --git a/pkg/integrations/elasticsearch_exporter/elasticsearch_exporter.go b/pkg/integrations/elasticsearch_exporter/elasticsearch_exporter.go index d97923a3e135..8e404dde8df6 100644 --- a/pkg/integrations/elasticsearch_exporter/elasticsearch_exporter.go +++ b/pkg/integrations/elasticsearch_exporter/elasticsearch_exporter.go @@ -12,7 +12,6 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus-community/elasticsearch_exporter/collector" @@ -30,10 +29,6 @@ var DefaultConfig = Config{ // Config controls the elasticsearch_exporter integration. type Config struct { - Common config.Common `yaml:",inline"` - - // Exporter configuration - // HTTP API address of an Elasticsearch node. Address string `yaml:"address,omitempty"` // Timeout for trying to get stats from Elasticsearch. @@ -77,12 +72,6 @@ func (c *Config) Name() string { return "elasticsearch_exporter" } -// CommonConfig returns the common settings shared across all configs for -// integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the hostname:port of the elasticsearch node being queried. func (c *Config) InstanceKey(agentKey string) (string, error) { u, err := url.Parse(c.Address) diff --git a/pkg/integrations/github_exporter/github_exporter.go b/pkg/integrations/github_exporter/github_exporter.go index a448524f473f..ba016f42fa9a 100644 --- a/pkg/integrations/github_exporter/github_exporter.go +++ b/pkg/integrations/github_exporter/github_exporter.go @@ -7,7 +7,6 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" gh_config "github.com/infinityworks/github-exporter/config" "github.com/infinityworks/github-exporter/exporter" config_util "github.com/prometheus/common/config" @@ -20,8 +19,6 @@ var DefaultConfig Config = Config{ // Config controls github_exporter type Config struct { - Common config.Common `yaml:",inline"` - // URL for the github API APIURL string `yaml:"api_url,omitempty"` @@ -54,12 +51,6 @@ func (c *Config) Name() string { return "github_exporter" } -// CommonConfig returns the common settings shared across all configs for -// integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the hostname:port of the GitHub API server. func (c *Config) InstanceKey(agentKey string) (string, error) { u, err := url.Parse(c.APIURL) diff --git a/pkg/integrations/integration.go b/pkg/integrations/integration.go index 7ba62378381f..6f26e7a79a5f 100644 --- a/pkg/integrations/integration.go +++ b/pkg/integrations/integration.go @@ -14,10 +14,6 @@ type Config interface { // pull the configuration from the Agent config YAML. Name() string - // CommonConfig returns the set of common configuration values present across - // all integrations. - CommonConfig() config.Common - // InstanceKey should return the key the reprsents the config, which will be // used to populate the value of the `instance` label for metrics. // @@ -25,8 +21,8 @@ type Config interface { // may be used if the integration being configured applies to an entire // machine. // - // This method is only used if the common config does not have an override for - // InstanceKey. + // This method may not be invoked if the instance key for a Config is + // overridden. InstanceKey(agentKey string) (string, error) // NewIntegration returns an integration for the given with the given logger. diff --git a/pkg/integrations/kafka_exporter/kafka_exporter.go b/pkg/integrations/kafka_exporter/kafka_exporter.go index 132f78e338f0..53b5610421ce 100644 --- a/pkg/integrations/kafka_exporter/kafka_exporter.go +++ b/pkg/integrations/kafka_exporter/kafka_exporter.go @@ -9,7 +9,6 @@ import ( kafka_exporter "github.com/davidmparrott/kafka_exporter/v2/exporter" "github.com/go-kit/log" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" ) // DefaultConfig holds the default settings for the kafka_lag_exporter @@ -27,8 +26,6 @@ var DefaultConfig = Config{ // Config controls kafka_exporter type Config struct { - Common config.Common `yaml:",inline"` - // Address array (host:port) of Kafka server KafkaURIs []string `yaml:"kafka_uris,omitempty"` @@ -106,12 +103,6 @@ func (c *Config) Name() string { return "kafka_exporter" } -// CommonConfig returns the common settings shared across all configs for -// integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the hostname:port of the first Kafka node, if any. If // there is not exactly one Kafka node, the user must manually provide // their own value for instance key in the common config. diff --git a/pkg/integrations/manager.go b/pkg/integrations/manager.go index af62088bb318..47275e0ba8e1 100644 --- a/pkg/integrations/manager.go +++ b/pkg/integrations/manager.go @@ -117,12 +117,12 @@ func (c *ManagerConfig) DefaultRelabelConfigs(instanceKey string) []*relabel.Con // Prometheus configuration must have a WAL directory configured. func (c *ManagerConfig) ApplyDefaults(cfg *metrics.Config) error { for _, ic := range c.Integrations { - if !ic.CommonConfig().Enabled { + if !ic.Common.Enabled { continue } scrapeIntegration := c.ScrapeIntegrations - if common := ic.CommonConfig(); common.ScrapeIntegration != nil { + if common := ic.Common; common.ScrapeIntegration != nil { scrapeIntegration = *common.ScrapeIntegration } @@ -213,7 +213,7 @@ func (m *Manager) ApplyConfig(cfg ManagerConfig) error { // Iterate over our integrations. New or changed integrations will be // started, with their existing counterparts being shut down. for _, ic := range cfg.Integrations { - if !ic.CommonConfig().Enabled { + if !ic.Common.Enabled { continue } // Key is used to identify the instance of this integration within the @@ -245,7 +245,7 @@ func (m *Manager) ApplyConfig(cfg ManagerConfig) error { // Find what instance label should be used to represent this integration. var instanceKey string - if kp := ic.CommonConfig().InstanceKey; kp != nil { + if kp := ic.Common.InstanceKey; kp != nil { // Common config takes precedence. instanceKey = strings.TrimSpace(*kp) } else { @@ -286,7 +286,7 @@ func (m *Manager) ApplyConfig(cfg ManagerConfig) error { for _, ic := range cfg.Integrations { if integrationKey(ic.Name()) == key { // If this is disabled then we should delete from integrations - if !ic.CommonConfig().Enabled { + if !ic.Common.Enabled { break } foundConfig = true @@ -307,7 +307,7 @@ func (m *Manager) ApplyConfig(cfg ManagerConfig) error { // if the configs for the integration didn't. for key, p := range m.integrations { shouldCollect := cfg.ScrapeIntegrations - if common := p.cfg.CommonConfig(); common.ScrapeIntegration != nil { + if common := p.cfg.Common; common.ScrapeIntegration != nil { shouldCollect = *common.ScrapeIntegration } @@ -345,7 +345,7 @@ type integrationProcess struct { log log.Logger ctx context.Context stop context.CancelFunc - cfg Config + cfg UnmarshaledConfig instanceKey string // Value for the `instance` label i Integration @@ -386,7 +386,7 @@ func (m *Manager) instanceBackoff(cfg Config, err error) { } func (m *Manager) instanceConfigForIntegration(p *integrationProcess, cfg ManagerConfig) instance.Config { - common := p.cfg.CommonConfig() + common := p.cfg.Common relabelConfigs := append(cfg.DefaultRelabelConfigs(p.instanceKey), common.RelabelConfigs...) schema := "http" diff --git a/pkg/integrations/manager_test.go b/pkg/integrations/manager_test.go index 20ea53ed1042..88325dfb874f 100644 --- a/pkg/integrations/manager_test.go +++ b/pkg/integrations/manager_test.go @@ -48,7 +48,6 @@ use_hostname_label: true outBytes, err := yaml.Marshal(cfg) require.NoError(t, err, "Failed creating integration") - fmt.Println(string(outBytes)) require.YAMLEq(t, cfgText, string(outBytes)) } @@ -79,7 +78,6 @@ test: outBytes, err := yaml.Marshal(cfg) require.NoError(t, err, "Failed creating integration") - fmt.Println(string(outBytes)) require.YAMLEq(t, cfgText, string(outBytes)) } @@ -120,7 +118,7 @@ func TestManager_instanceConfigForIntegration(t *testing.T) { require.NoError(t, err) defer m.Stop() - p := &integrationProcess{instanceKey: "key", cfg: icfg, i: mock} + p := &integrationProcess{instanceKey: "key", cfg: makeUnmarshaledConfig(icfg, true), i: mock} cfg := m.instanceConfigForIntegration(p, mockManagerConfig()) // Validate that the generated MetricsPath is a valid URL path @@ -128,6 +126,10 @@ func TestManager_instanceConfigForIntegration(t *testing.T) { require.Equal(t, "/integrations/mock/metrics", cfg.ScrapeConfigs[0].MetricsPath) } +func makeUnmarshaledConfig(cfg Config, enabled bool) UnmarshaledConfig { + return UnmarshaledConfig{Config: cfg, Common: config.Common{Enabled: enabled}} +} + // TestManager_NoIntegrationsScrape ensures that configs don't get generates // when the ScrapeIntegrations flag is disabled. func TestManager_NoIntegrationsScrape(t *testing.T) { @@ -138,7 +140,7 @@ func TestManager_NoIntegrationsScrape(t *testing.T) { cfg := mockManagerConfig() cfg.ScrapeIntegrations = false - cfg.Integrations = append(cfg.Integrations, &icfg) + cfg.Integrations = append(cfg.Integrations, makeUnmarshaledConfig(&icfg, true)) m, err := NewManager(cfg, log.NewNopLogger(), im, noOpValidator) require.NoError(t, err) @@ -156,14 +158,15 @@ func TestManager_NoIntegrationsScrape(t *testing.T) { func TestManager_NoIntegrationScrape(t *testing.T) { mock := newMockIntegration() icfg := mockConfig{Integration: mock} - noScrape := false - mock.CommonCfg.ScrapeIntegration = &noScrape im := instance.NewBasicManager(instance.DefaultBasicManagerConfig, log.NewNopLogger(), mockInstanceFactory) cfg := mockManagerConfig() - cfg.Integrations = append(cfg.Integrations, icfg) + cfg.Integrations = append(cfg.Integrations, UnmarshaledConfig{ + Config: icfg, + Common: config.Common{ScrapeIntegration: &noScrape}, + }) m, err := NewManager(cfg, log.NewNopLogger(), im, noOpValidator) require.NoError(t, err) @@ -180,7 +183,7 @@ func TestManager_StartsIntegrations(t *testing.T) { icfg := mockConfig{Integration: mock} cfg := mockManagerConfig() - cfg.Integrations = append(cfg.Integrations, icfg) + cfg.Integrations = append(cfg.Integrations, makeUnmarshaledConfig(icfg, true)) im := instance.NewBasicManager(instance.DefaultBasicManagerConfig, log.NewNopLogger(), mockInstanceFactory) m, err := NewManager(cfg, log.NewNopLogger(), im, noOpValidator) @@ -202,7 +205,7 @@ func TestManager_RestartsIntegrations(t *testing.T) { icfg := mockConfig{Integration: mock} cfg := mockManagerConfig() - cfg.Integrations = append(cfg.Integrations, icfg) + cfg.Integrations = append(cfg.Integrations, makeUnmarshaledConfig(icfg, true)) im := instance.NewBasicManager(instance.DefaultBasicManagerConfig, log.NewNopLogger(), mockInstanceFactory) m, err := NewManager(cfg, log.NewNopLogger(), im, noOpValidator) @@ -221,7 +224,7 @@ func TestManager_GracefulStop(t *testing.T) { icfg := mockConfig{Integration: mock} cfg := mockManagerConfig() - cfg.Integrations = append(cfg.Integrations, icfg) + cfg.Integrations = append(cfg.Integrations, makeUnmarshaledConfig(icfg, true)) im := instance.NewBasicManager(instance.DefaultBasicManagerConfig, log.NewNopLogger(), mockInstanceFactory) m, err := NewManager(cfg, log.NewNopLogger(), im, noOpValidator) @@ -245,7 +248,7 @@ func TestManager_IntegrationEnabledToDisabledReload(t *testing.T) { mock := newMockIntegration() icfg := mockConfig{Integration: mock} cfg := mockManagerConfig() - cfg.Integrations = append(cfg.Integrations, icfg) + cfg.Integrations = append(cfg.Integrations, makeUnmarshaledConfig(icfg, true)) im := instance.NewBasicManager(instance.DefaultBasicManagerConfig, log.NewNopLogger(), mockInstanceFactory) m, err := NewManager(cfg, log.NewNopLogger(), im, noOpValidator) @@ -267,11 +270,13 @@ func TestManager_IntegrationEnabledToDisabledReload(t *testing.T) { func TestManager_IntegrationDisabledToEnabledReload(t *testing.T) { mock := newMockIntegration() - mock.CommonCfg.Enabled = false icfg := mockConfig{Integration: mock} cfg := mockManagerConfig() - cfg.Integrations = append(cfg.Integrations, icfg) + cfg.Integrations = append(cfg.Integrations, UnmarshaledConfig{ + Config: icfg, + Common: config.Common{Enabled: false}, + }) im := instance.NewBasicManager(instance.DefaultBasicManagerConfig, log.NewNopLogger(), mockInstanceFactory) m, err := NewManager(cfg, log.NewNopLogger(), im, noOpValidator) @@ -305,7 +310,7 @@ func TestManager_PromConfigChangeReloads(t *testing.T) { icfg := mockConfig{Integration: mock} cfg := mockManagerConfig() - cfg.Integrations = append(cfg.Integrations, icfg) + cfg.Integrations = append(cfg.Integrations, makeUnmarshaledConfig(icfg, true)) im := instance.NewBasicManager(instance.DefaultBasicManagerConfig, log.NewNopLogger(), mockInstanceFactory) @@ -343,10 +348,12 @@ func TestManager_PromConfigChangeReloads(t *testing.T) { func generateMockConfigWithEnabledFlag(enabled bool) ManagerConfig { enabledMock := newMockIntegration() - enabledMock.CommonCfg.Enabled = enabled enabledConfig := mockConfig{Integration: enabledMock} enabledManagerConfig := mockManagerConfig() - enabledManagerConfig.Integrations = append(enabledManagerConfig.Integrations, enabledConfig) + enabledManagerConfig.Integrations = append( + enabledManagerConfig.Integrations, + makeUnmarshaledConfig(enabledConfig, enabled), + ) return enabledManagerConfig } @@ -358,7 +365,6 @@ type mockConfig struct { func (c mockConfig) Equal(other mockConfig) bool { return c.Integration == other.Integration } func (c mockConfig) Name() string { return "mock" } -func (c mockConfig) CommonConfig() config.Common { return c.Integration.CommonCfg } func (c mockConfig) InstanceKey(agentKey string) (string, error) { return agentKey, nil } func (c mockConfig) NewIntegration(_ log.Logger) (Integration, error) { @@ -366,7 +372,6 @@ func (c mockConfig) NewIntegration(_ log.Logger) (Integration, error) { } type mockIntegration struct { - CommonCfg config.Common `yaml:",inline"` startedCount *atomic.Uint32 running *atomic.Bool err chan error @@ -377,7 +382,6 @@ func newMockIntegration() *mockIntegration { running: atomic.NewBool(true), startedCount: atomic.NewUint32(0), err: make(chan error), - CommonCfg: config.Common{Enabled: true}, } } diff --git a/pkg/integrations/memcached_exporter/memcached_exporter.go b/pkg/integrations/memcached_exporter/memcached_exporter.go index 4d85e11603cd..727b835de3d6 100644 --- a/pkg/integrations/memcached_exporter/memcached_exporter.go +++ b/pkg/integrations/memcached_exporter/memcached_exporter.go @@ -6,7 +6,6 @@ import ( "github.com/go-kit/log" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" "github.com/prometheus/memcached_exporter/pkg/exporter" ) @@ -18,8 +17,6 @@ var DefaultConfig Config = Config{ // Config controls the memcached_exporter integration. type Config struct { - Common config.Common `yaml:",inline"` - // MemcachedAddress is the address of the memcached server (host:port). MemcachedAddress string `yaml:"memcached_address,omitempty"` @@ -40,11 +37,6 @@ func (c *Config) Name() string { return "memcached_exporter" } -// CommonConfig returns the common settings shared across all integratons. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the address:port of the memcached server. func (c *Config) InstanceKey(agentKey string) (string, error) { return c.MemcachedAddress, nil diff --git a/pkg/integrations/mongodb_exporter/mongodb_exporter.go b/pkg/integrations/mongodb_exporter/mongodb_exporter.go index e5aa5629bc0c..16d8459c903a 100644 --- a/pkg/integrations/mongodb_exporter/mongodb_exporter.go +++ b/pkg/integrations/mongodb_exporter/mongodb_exporter.go @@ -6,15 +6,12 @@ import ( "github.com/go-kit/log" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" "github.com/percona/mongodb_exporter/exporter" config_util "github.com/prometheus/common/config" ) // Config controls mongodb_exporter type Config struct { - Common config.Common `yaml:",inline"` - // MongoDB connection URI. example:mongodb://user:pass@127.0.0.1:27017/admin?ssl=true" URI config_util.Secret `yaml:"mongodb_uri"` } @@ -31,12 +28,6 @@ func (c *Config) Name() string { return "mongodb_exporter" } -// CommonConfig returns the common settings shared across all configs for -// integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the address:port of the mongodb server being queried. func (c *Config) InstanceKey(_ string) (string, error) { u, err := url.Parse(string(c.URI)) diff --git a/pkg/integrations/mysqld_exporter/mysqld-exporter.go b/pkg/integrations/mysqld_exporter/mysqld-exporter.go index e98c440c51b6..25565b631f28 100644 --- a/pkg/integrations/mysqld_exporter/mysqld-exporter.go +++ b/pkg/integrations/mysqld_exporter/mysqld-exporter.go @@ -12,7 +12,6 @@ import ( "github.com/go-kit/log/level" "github.com/go-sql-driver/mysql" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" "github.com/prometheus/mysqld_exporter/collector" ) @@ -36,8 +35,6 @@ var DefaultConfig = Config{ // Config controls the mysqld_exporter integration. type Config struct { - Common config.Common `yaml:",inline"` - // DataSourceName to use to connect to MySQL. DataSourceName config_util.Secret `yaml:"data_source_name,omitempty"` @@ -82,11 +79,6 @@ func (c *Config) Name() string { return "mysqld_exporter" } -// CommonConfig returns the common settings shared across all integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns network(hostname:port)/dbname of the MySQL server. func (c *Config) InstanceKey(_ string) (string, error) { m, err := mysql.ParseDSN(string(c.DataSourceName)) diff --git a/pkg/integrations/node_exporter/config.go b/pkg/integrations/node_exporter/config.go index 35b5fc252337..ed895b74d706 100644 --- a/pkg/integrations/node_exporter/config.go +++ b/pkg/integrations/node_exporter/config.go @@ -8,7 +8,6 @@ import ( "github.com/go-kit/log" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" "github.com/grafana/dskit/flagext" "github.com/prometheus/procfs" "gopkg.in/alecthomas/kingpin.v2" @@ -64,8 +63,6 @@ func init() { // Config controls the node_exporter integration. type Config struct { - Common config.Common `yaml:",inline"` - IncludeExporterMetrics bool `yaml:"include_exporter_metrics,omitempty"` ProcFSPath string `yaml:"procfs_path,omitempty"` @@ -124,11 +121,6 @@ func (c *Config) Name() string { return "node_exporter" } -// CommonConfig returns the common configs that are shared across all integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the hostname:port of the agent process. func (c *Config) InstanceKey(agentKey string) (string, error) { return agentKey, nil diff --git a/pkg/integrations/postgres_exporter/postgres_exporter.go b/pkg/integrations/postgres_exporter/postgres_exporter.go index ac51779d48a8..f5f2838892e6 100644 --- a/pkg/integrations/postgres_exporter/postgres_exporter.go +++ b/pkg/integrations/postgres_exporter/postgres_exporter.go @@ -10,15 +10,12 @@ import ( "github.com/go-kit/log" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" "github.com/lib/pq" "github.com/prometheus-community/postgres_exporter/exporter" ) // Config controls the postgres_exporter integration. type Config struct { - Common config.Common `yaml:",inline"` - // DataSourceNames to use to connect to Postgres. DataSourceNames []config_util.Secret `yaml:"data_source_names,omitempty"` @@ -35,12 +32,6 @@ func (c *Config) Name() string { return "postgres_exporter" } -// CommonConfig returns the common set of options shared across all configs for -// integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // NewIntegration converts this config into an instance of a configuration. func (c *Config) NewIntegration(l log.Logger) (integrations.Integration, error) { return New(l, c) diff --git a/pkg/integrations/process_exporter/config.go b/pkg/integrations/process_exporter/config.go index 97eec461f351..71148f4df474 100644 --- a/pkg/integrations/process_exporter/config.go +++ b/pkg/integrations/process_exporter/config.go @@ -4,7 +4,6 @@ package process_exporter //nolint:golint import ( "github.com/go-kit/log" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" exporter_config "github.com/ncabatoff/process-exporter/config" ) @@ -20,7 +19,6 @@ var DefaultConfig = Config{ // Config controls the process_exporter integration. type Config struct { - Common config.Common `yaml:",inline"` ProcessExporter exporter_config.MatcherRules `yaml:"process_names,omitempty"` ProcFSPath string `yaml:"procfs_path,omitempty"` @@ -43,11 +41,6 @@ func (c *Config) Name() string { return "process_exporter" } -// CommonConfig returns the set of common settings shared across all integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the hostname of the machine. func (c *Config) InstanceKey(agentKey string) (string, error) { return agentKey, nil diff --git a/pkg/integrations/redis_exporter/redis_exporter.go b/pkg/integrations/redis_exporter/redis_exporter.go index 2620d299b055..21c818916bbf 100644 --- a/pkg/integrations/redis_exporter/redis_exporter.go +++ b/pkg/integrations/redis_exporter/redis_exporter.go @@ -8,7 +8,6 @@ import ( "time" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" "github.com/go-kit/log" "github.com/go-kit/log/level" @@ -29,8 +28,6 @@ var DefaultConfig = Config{ // Config controls the redis_exporter integration. type Config struct { - Common config.Common `yaml:",inline"` - IncludeExporterMetrics bool `yaml:"include_exporter_metrics"` // exporter-specific config. @@ -107,12 +104,6 @@ func (c *Config) Name() string { return "redis_exporter" } -// CommonConfig returns the common set of settings shared across all configs -// for integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the addr of the redis server. func (c *Config) InstanceKey(agentKey string) (string, error) { return c.RedisAddr, nil diff --git a/pkg/integrations/register.go b/pkg/integrations/register.go index cdbc1a467eb0..8fd09b3b463b 100644 --- a/pkg/integrations/register.go +++ b/pkg/integrations/register.go @@ -5,6 +5,8 @@ import ( "reflect" "strings" + "github.com/grafana/agent/pkg/integrations/config" + "github.com/grafana/agent/pkg/util" "gopkg.in/yaml.v2" ) @@ -30,46 +32,18 @@ func RegisterIntegration(cfg Config) { configFieldNames[reflect.TypeOf(cfg)] = cfg.Name() } -// Configs is a list of integrations. -type Configs []Config - -// UnmarshalYAML implements yaml.Unmarshaler. -func (c *Configs) UnmarshalYAML(unmarshal func(interface{}) error) error { - return c.unmarshalWithIntegrations(registeredIntegrations, unmarshal) +func cloneIntegration(c Config) Config { + return reflect.New(reflect.TypeOf(c).Elem()).Interface().(Config) } -func (c *Configs) unmarshalWithIntegrations(integrations []Config, unmarshal func(interface{}) error) error { - // Create a dynamic struct type full of our registered integrations and - // unmarshal to it. - var fields []reflect.StructField - for _, cfg := range integrations { - fields = append(fields, reflect.StructField{ - Name: "Config_" + cfg.Name(), - Tag: reflect.StructTag(fmt.Sprintf(`yaml:"%s"`, cfg.Name())), - Type: reflect.TypeOf(cfg), - }) - } - - var ( - structType = reflect.StructOf(fields) - structVal = reflect.New(structType) - ) - if err := unmarshal(structVal.Interface()); err != nil { - return err - } - - // Go over all non-nil fields in structVal and append them to c. - structVal = structVal.Elem() - for i := 0; i < structVal.NumField(); i++ { - if structVal.Field(i).IsNil() { - continue - } - - val := structVal.Field(i).Interface().(Config) - *c = append(*c, val) - } +// Configs is a list of UnmarshaledConfig. Configs for integrations which are +// unmarshaled from YAML are combined with common settings. +type Configs []UnmarshaledConfig - return nil +// UnmarshaledConfig combines an integration's config with common settings. +type UnmarshaledConfig struct { + Config + Common config.Common } // MarshalYAML helps implement yaml.Marshaller for structs that have a Configs @@ -112,17 +86,32 @@ func MarshalYAML(v interface{}) (interface{}, error) { } for _, c := range configs { - fieldName, ok := configFieldNames[reflect.TypeOf(c)] + fieldName, ok := configFieldNames[reflect.TypeOf(c.Config)] if !ok { return nil, fmt.Errorf("integrations: cannot marshal unregistered Config type: %T", c) } field := cfgVal.FieldByName("XXX_Config_" + fieldName) - field.Set(reflect.ValueOf(c)) + rawConfig, err := getRawIntegrationConfig(c) + if err != nil { + return nil, fmt.Errorf("integrations: cannot marshal integration %q: %w", c.Name(), err) + } + field.Set(rawConfig) } return cfgPointer.Interface(), nil } +// getRawIntegrationConfig turns an UnmarshaledConfig into the *util.RawYAML +// used to represent it in configs. +func getRawIntegrationConfig(uc UnmarshaledConfig) (v reflect.Value, err error) { + bb, err := util.MarshalYAMLMerged(uc.Common, uc.Config) + if err != nil { + return v, err + } + raw := util.RawYAML(bb) + return reflect.ValueOf(&raw), nil +} + // UnmarshalYAML helps implement yaml.Unmarshaller for structs that have a // Configs field that should be inlined in the YAML string. func UnmarshalYAML(out interface{}, unmarshal func(interface{}) error) error { @@ -188,16 +177,29 @@ func unmarshalIntegrationsWithList(integrations []Config, out interface{}, unmar outVal.Field(i).Set(cfgVal.Field(i)) } - // Iterate through the remainder of our fields, which should all be of - // type Config. + // Iterate through the remainder of our fields, which should all be dynamic + // structs where the first field is a config.Common and the second field is + // a Config. + integrationLookup := buildIntegrationsMap(integrations) for i := outVal.NumField(); i < cfgVal.NumField(); i++ { + // Our integrations are unmarshaled as *util.RawYAML. If it's nil, we treat + // it as not defined. + fieldType := cfgVal.Type().Field(i) field := cfgVal.Field(i) - if field.IsNil() { continue } - val := cfgVal.Field(i).Interface().(Config) - *configs = append(*configs, val) + + configName := strings.TrimPrefix(fieldType.Name, "XXX_Config_") + configReference, ok := integrationLookup[configName] + if !ok { + return fmt.Errorf("integration %q not registered", configName) + } + uc, err := buildUnmarshaledConfig(field.Interface().(*util.RawYAML), configReference) + if err != nil { + return fmt.Errorf("failed to unmarshal integration %q: %w", configName, err) + } + *configs = append(*configs, uc) } return nil @@ -205,6 +207,8 @@ func unmarshalIntegrationsWithList(integrations []Config, out interface{}, unmar // getConfigTypeForIntegrations returns a dynamic struct type that has all of // the same fields as out including the fields for the provided integrations. +// +// integrations are unmarshaled to *util.RawYAML for deferred unmarshaling. func getConfigTypeForIntegrations(integrations []Config, out reflect.Type) reflect.Type { // Initial exported fields map one-to-one. var fields []reflect.StructField @@ -226,18 +230,37 @@ func getConfigTypeForIntegrations(integrations []Config, out reflect.Type) refle fields = append(fields, reflect.StructField{ Name: fieldName, Tag: reflect.StructTag(fmt.Sprintf(`yaml:"%s,omitempty"`, cfg.Name())), - Type: reflect.TypeOf(cfg), + Type: reflect.PtrTo(reflect.TypeOf(util.RawYAML{})), }) } return reflect.StructOf(fields) } +func buildIntegrationsMap(in []Config) map[string]Config { + m := make(map[string]Config, len(in)) + for _, i := range in { + m[i.Name()] = i + } + return m +} + +// buildUnmarshaledConfig converts raw YAML into an UnmarshaledConfig where the +// config type is the same as ref. +func buildUnmarshaledConfig(raw *util.RawYAML, ref Config) (uc UnmarshaledConfig, err error) { + // Initalize uc.Config so it can be unmarshaled properly as an interface. + uc = UnmarshaledConfig{ + Config: cloneIntegration(ref), + } + err = util.UnmarshalYAMLMerged(*raw, &uc.Common, uc.Config) + return +} + func replaceYAMLTypeError(err error, oldTyp, newTyp reflect.Type) error { if e, ok := err.(*yaml.TypeError); ok { oldStr := oldTyp.String() newStr := newTyp.String() for i, s := range e.Errors { - e.Errors[i] = strings.Replace(s, oldStr, newStr, -1) + e.Errors[i] = strings.ReplaceAll(s, oldStr, newStr) } } return err diff --git a/pkg/integrations/register_test.go b/pkg/integrations/register_test.go index 84a7bfcfd781..c9252b48c113 100644 --- a/pkg/integrations/register_test.go +++ b/pkg/integrations/register_test.go @@ -6,7 +6,6 @@ import ( "time" "github.com/go-kit/log" - "github.com/grafana/agent/pkg/integrations/config" "github.com/stretchr/testify/require" "gopkg.in/yaml.v2" ) @@ -35,9 +34,9 @@ test: Name: "John Doe", Duration: 500 * time.Millisecond, Default: 12345, - Configs: []Config{ - &testIntegrationA{Text: "Hello, world!", Truth: true}, - }, + Configs: []UnmarshaledConfig{{ + Config: &testIntegrationA{Text: "Hello, world!", Truth: true}, + }}, } require.Equal(t, expect, fullCfg) } @@ -48,7 +47,6 @@ type testIntegrationA struct { } func (i *testIntegrationA) Name() string { return "test" } -func (i *testIntegrationA) CommonConfig() config.Common { return config.Common{} } func (i *testIntegrationA) InstanceKey(_ string) (string, error) { return "integrationA", nil } func (i *testIntegrationA) NewIntegration(l log.Logger) (Integration, error) { @@ -66,7 +64,6 @@ type testIntegrationB struct { } func (*testIntegrationB) Name() string { return "shouldnotbefound" } -func (*testIntegrationB) CommonConfig() config.Common { return config.Common{} } func (*testIntegrationB) InstanceKey(_ string) (string, error) { return "integrationB", nil } func (*testIntegrationB) NewIntegration(l log.Logger) (Integration, error) { diff --git a/pkg/integrations/statsd_exporter/statsd_exporter.go b/pkg/integrations/statsd_exporter/statsd_exporter.go index a6c47c279d78..265b7c79a12a 100644 --- a/pkg/integrations/statsd_exporter/statsd_exporter.go +++ b/pkg/integrations/statsd_exporter/statsd_exporter.go @@ -50,8 +50,6 @@ var DefaultConfig = Config{ // Config controls the statsd_exporter integration. type Config struct { - Common config.Common `yaml:",inline"` - ListenUDP string `yaml:"listen_udp,omitempty"` ListenTCP string `yaml:"listen_tcp,omitempty"` ListenUnixgram string `yaml:"listen_unixgram,omitempty"` @@ -84,11 +82,6 @@ func (c *Config) Name() string { return "statsd_exporter" } -// CommonConfig returns the common settings shared across all integrations. -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the hostname:port of the agent. func (c *Config) InstanceKey(agentKey string) (string, error) { return agentKey, nil diff --git a/pkg/integrations/windows_exporter/config.go b/pkg/integrations/windows_exporter/config.go index 78de89533623..c65b677f2bbd 100644 --- a/pkg/integrations/windows_exporter/config.go +++ b/pkg/integrations/windows_exporter/config.go @@ -2,7 +2,6 @@ package windows_exporter //nolint:golint import ( "github.com/go-kit/log" "github.com/grafana/agent/pkg/integrations" - "github.com/grafana/agent/pkg/integrations/config" ) // DefaultConfig holds the default settings for the windows_exporter integration. @@ -20,8 +19,6 @@ func init() { // Config controls the windows_exporter integration. // All of these and their child fields are pointers so we can determine if the value was set or not. type Config struct { - Common config.Common `yaml:",inline"` - EnabledCollectors string `yaml:"enabled_collectors"` Exchange ExchangeConfig `yaml:"exchange,omitempty"` @@ -49,11 +46,6 @@ func (c *Config) Name() string { return "windows_exporter" } -// CommonConfig returns the common fields that all integrations have -func (c *Config) CommonConfig() config.Common { - return c.Common -} - // InstanceKey returns the hostname:port of the agent. func (c *Config) InstanceKey(agentKey string) (string, error) { return agentKey, nil diff --git a/pkg/util/yaml.go b/pkg/util/yaml.go new file mode 100644 index 000000000000..9a0e1dff3ef8 --- /dev/null +++ b/pkg/util/yaml.go @@ -0,0 +1,119 @@ +package util + +import ( + "errors" + "regexp" + "sort" + + "gopkg.in/yaml.v2" +) + +// RawYAML is similar to json.RawMessage and allows for deferred YAML decoding. +type RawYAML []byte + +// UnmarshalYAML implements yaml.Unmarshaler. +func (r *RawYAML) UnmarshalYAML(unmarshal func(interface{}) error) error { + var ms yaml.MapSlice + if err := unmarshal(&ms); err != nil { + return err + } + bb, err := yaml.Marshal(ms) + if err != nil { + return err + } + *r = bb + return nil +} + +// MarshalYAML implements yaml.Marshaler. +func (r RawYAML) MarshalYAML() (interface{}, error) { + return r.Map() +} + +func (r RawYAML) Map() (yaml.MapSlice, error) { + var ms yaml.MapSlice + if err := yaml.Unmarshal(r, &ms); err != nil { + return nil, err + } + return ms, nil +} + +// MarshalYAMLMerged marshals all values from vv into a single object. +func MarshalYAMLMerged(vv ...interface{}) ([]byte, error) { + var full yaml.MapSlice + for _, v := range vv { + bb, err := yaml.Marshal(v) + if err != nil { + return nil, err + } + ms, err := RawYAML(bb).Map() + if err != nil { + return nil, err + } + full = append(full, ms...) + } + return yaml.Marshal(full) +} + +// UnmarshalYAMLMerged performs a strict unmarshal of bb into all values from +// vv. +func UnmarshalYAMLMerged(bb []byte, vv ...interface{}) error { + var typeErrors []yaml.TypeError + + for _, v := range vv { + // Perform a non-strict unmarshal first to populate the object completely. + if err := yaml.Unmarshal(bb, v); err != nil { + return err + } + + // Then, perform a strict unmarshal. This is likely to fail with type + // errors for missing fields that may have been consumed by another object + // in vv. + var te *yaml.TypeError + if err := yaml.UnmarshalStrict(bb, v); errors.As(err, &te) { + typeErrors = append(typeErrors, *te) + } else if err != nil { + return err + } + } + + var ( + addedErrors = map[string]struct{}{} + notFoundErrors = map[string]int{} + ) + + // Do an initial pass over our errors, separating errors for not found fields. + // Other errors are "real" and should be returned. + for _, te := range typeErrors { + for _, msg := range te.Errors { + notFound := notFoundErrRegex.FindStringSubmatch(msg) + if notFound != nil { + // Track the invalid field error. Use the first capture group which + // excludes the type, which will be unique per v. + notFoundErrors[notFound[1]]++ + continue + } + addedErrors[msg] = struct{}{} + } + } + + // Iterate over our errors for not found fields. The field truly isn't defined + // if it was reported len(vv) times (i.e., no v consumed it). + for msg, count := range notFoundErrors { + if count == len(vv) { + addedErrors[msg] = struct{}{} + } + } + + if len(addedErrors) > 0 { + realErrors := make([]string, 0, len(addedErrors)) + for msg := range addedErrors { + realErrors = append(realErrors, msg) + } + sort.Strings(realErrors) + return &yaml.TypeError{Errors: realErrors} + } + return nil +} + +var notFoundErrRegex = regexp.MustCompile(`^(line \d+: field .* not found) in type .*$`) From 1467a1058b4d8b72d623dc4bd4193a65a9a71ed2 Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Fri, 17 Dec 2021 15:42:02 -0500 Subject: [PATCH 2/2] fix linting issues --- pkg/integrations/register.go | 2 +- pkg/util/yaml.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/integrations/register.go b/pkg/integrations/register.go index 8fd09b3b463b..7453cc82b97e 100644 --- a/pkg/integrations/register.go +++ b/pkg/integrations/register.go @@ -247,7 +247,7 @@ func buildIntegrationsMap(in []Config) map[string]Config { // buildUnmarshaledConfig converts raw YAML into an UnmarshaledConfig where the // config type is the same as ref. func buildUnmarshaledConfig(raw *util.RawYAML, ref Config) (uc UnmarshaledConfig, err error) { - // Initalize uc.Config so it can be unmarshaled properly as an interface. + // Initialize uc.Config so it can be unmarshaled properly as an interface. uc = UnmarshaledConfig{ Config: cloneIntegration(ref), } diff --git a/pkg/util/yaml.go b/pkg/util/yaml.go index 9a0e1dff3ef8..043bfdcd665f 100644 --- a/pkg/util/yaml.go +++ b/pkg/util/yaml.go @@ -30,6 +30,7 @@ func (r RawYAML) MarshalYAML() (interface{}, error) { return r.Map() } +// Map converts the raw YAML into a yaml.MapSlice. func (r RawYAML) Map() (yaml.MapSlice, error) { var ms yaml.MapSlice if err := yaml.Unmarshal(r, &ms); err != nil {