From 11d49361e6e2addc421086e89ab9285a0f1ffe7a Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Thu, 16 Dec 2021 18:07:40 -0500 Subject: [PATCH 01/11] feature flag wip --- cmd/agent/entrypoint.go | 3 +- pkg/integrations/manager.go | 15 +++- pkg/integrations/versionselector/selector.go | 87 ++++++++++++++++++++ 3 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 pkg/integrations/versionselector/selector.go diff --git a/cmd/agent/entrypoint.go b/cmd/agent/entrypoint.go index 3f208e0fa92f..395493f64cc1 100644 --- a/cmd/agent/entrypoint.go +++ b/cmd/agent/entrypoint.go @@ -13,7 +13,6 @@ import ( "github.com/gorilla/mux" integrations "github.com/grafana/agent/pkg/integrations/v2" "github.com/grafana/agent/pkg/logs" - loki "github.com/grafana/agent/pkg/logs" "github.com/grafana/agent/pkg/metrics" "github.com/grafana/agent/pkg/metrics/instance" "github.com/grafana/agent/pkg/traces" @@ -41,7 +40,7 @@ type Entrypoint struct { srv *server.Server promMetrics *metrics.Agent - lokiLogs *loki.Logs + lokiLogs *logs.Logs tempoTraces *traces.Traces integrations *integrations.Subsystem diff --git a/pkg/integrations/manager.go b/pkg/integrations/manager.go index 0306d58ea7a5..9ceed71eaf19 100644 --- a/pkg/integrations/manager.go +++ b/pkg/integrations/manager.go @@ -24,6 +24,7 @@ import ( promConfig "github.com/prometheus/prometheus/config" "github.com/prometheus/prometheus/discovery" "github.com/prometheus/prometheus/pkg/relabel" + "github.com/weaveworks/common/server" ) var ( @@ -115,7 +116,17 @@ func (c *ManagerConfig) DefaultRelabelConfigs(instanceKey string) []*relabel.Con // // If any integrations are enabled and are configured to be scraped, the // Prometheus configuration must have a WAL directory configured. -func (c *ManagerConfig) ApplyDefaults(cfg *metrics.Config) error { +func (c *ManagerConfig) ApplyDefaults(scfg *server.Config, mcfg *metrics.Config) error { + c.ListenPort = scfg.HTTPListenPort + c.ListenHost = scfg.HTTPListenAddress + + c.ServerUsingTLS = scfg.HTTPTLSConfig.TLSKeyPath != "" && scfg.HTTPTLSConfig.TLSCertPath != "" + + if len(c.PrometheusRemoteWrite) == 0 { + c.PrometheusRemoteWrite = mcfg.Global.RemoteWrite + } + c.PrometheusGlobalConfig = mcfg.Global.Prometheus + for _, ic := range c.Integrations { if !ic.CommonConfig().Enabled { continue @@ -127,7 +138,7 @@ func (c *ManagerConfig) ApplyDefaults(cfg *metrics.Config) error { } // WAL must be configured if an integration is going to be scraped. - if scrapeIntegration && cfg.WALDir == "" { + if scrapeIntegration && mcfg.WALDir == "" { return fmt.Errorf("no wal_directory configured") } } diff --git a/pkg/integrations/versionselector/selector.go b/pkg/integrations/versionselector/selector.go new file mode 100644 index 000000000000..3c3521625426 --- /dev/null +++ b/pkg/integrations/versionselector/selector.go @@ -0,0 +1,87 @@ +// Package integrations exposes the integrations subsystem. It will select +// between v1 and v2 based on a field. +package integrations + +import ( + "github.com/go-kit/log" + "github.com/gorilla/mux" + v1 "github.com/grafana/agent/pkg/integrations" + v2 "github.com/grafana/agent/pkg/integrations/v2" + "github.com/grafana/agent/pkg/metrics" + "github.com/weaveworks/common/server" +) + +// Config abstracts the subsystem configs for integrations v1 and v2. +type Config struct { + // UseV2 should be true if the newer v2 package should be used. This MUST be + // set prior to unmarshaling. + UseV2 bool + + configV1 *v1.ManagerConfig + configV2 *v2.SubsystemOptions +} + +// UnmarshalYAML implements yaml.Unmarshaler. +func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { + if !c.UseV2 { + return unmarshal(c.configV1) + } + return unmarshal(c.configV2) +} + +// MarshalYAML implements yaml.Marshaler. +func (c *Config) MarshalYAML() (interface{}, error) { + if !c.UseV2 { + return c.configV1, nil + } + return c.configV2, nil +} + +// ApplyDefaults applies defaults to the subsystem based on globals. Only +// needed when UseV2 is false. +func (c *Config) ApplyDefaults(scfg *server.Config, mcfg *metrics.Config) error { + if !c.UseV2 { + return c.configV1.ApplyDefaults(scfg, mcfg) + } + return nil +} + +// Subsystem is an abstraction over both the v1 and v2 systems. +type Subsystem interface { + ApplyConfig(*Config, v2.Globals) error + WireAPI(*mux.Router) + Stop() +} + +// NewSubsystem creates a new subsystem. globals should be provided regardless +// of useV2. globals.SubsystemOptions will be automatically set if useV2 is +// true. +func NewSubsystem(logger log.Logger, cfg *Config, globals v2.Globals, useV2 bool) (Subsystem, error) { + if !useV2 { + instance, err := v1.NewManager(*cfg.configV1, logger, globals.Metrics.InstanceManager(), globals.Metrics.Validate) + if err != nil { + return nil, err + } + return &v1Subsystem{Manager: instance}, nil + } + + globals.SubsystemOpts = *cfg.configV2 + instance, err := v2.NewSubsystem(logger, globals) + if err != nil { + return nil, err + } + return &v2Subsystem{Subsystem: instance}, nil +} + +type v1Subsystem struct{ *v1.Manager } + +func (s *v1Subsystem) ApplyConfig(cfg *Config, globals v2.Globals) error { + return s.Manager.ApplyConfig(*cfg.configV1) +} + +type v2Subsystem struct{ *v2.Subsystem } + +func (s *v2Subsystem) ApplyConfig(cfg *Config, globals v2.Globals) error { + globals.SubsystemOpts = *cfg.configV2 + return s.Subsystem.ApplyConfig(globals) +} From aee3b565a42193bd13d0d0d94afe73412e6d6fab Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Thu, 16 Dec 2021 19:02:14 -0500 Subject: [PATCH 02/11] dynamically switch between integrations v1 and v2 default to v1. --- cmd/agent/entrypoint.go | 10 +-- pkg/config/config.go | 44 +++++++---- pkg/config/config_test.go | 2 - pkg/integrations/versionselector/selector.go | 79 +++++++++++++++----- 4 files changed, 97 insertions(+), 38 deletions(-) diff --git a/cmd/agent/entrypoint.go b/cmd/agent/entrypoint.go index 395493f64cc1..14267184abb6 100644 --- a/cmd/agent/entrypoint.go +++ b/cmd/agent/entrypoint.go @@ -11,7 +11,7 @@ import ( "syscall" "github.com/gorilla/mux" - integrations "github.com/grafana/agent/pkg/integrations/v2" + integrations "github.com/grafana/agent/pkg/integrations/versionselector" "github.com/grafana/agent/pkg/logs" "github.com/grafana/agent/pkg/metrics" "github.com/grafana/agent/pkg/metrics/instance" @@ -42,7 +42,7 @@ type Entrypoint struct { promMetrics *metrics.Agent lokiLogs *logs.Logs tempoTraces *traces.Traces - integrations *integrations.Subsystem + integrations integrations.Subsystem reloadListener net.Listener reloadServer *http.Server @@ -95,7 +95,7 @@ func NewEntrypoint(logger *util.Logger, cfg *config.Config, reloader Reloader) ( if err != nil { return nil, err } - ep.integrations, err = integrations.NewSubsystem(logger, integrationGlobals) + ep.integrations, err = integrations.NewSubsystem(logger, &cfg.Integrations, integrationGlobals) if err != nil { return nil, err } @@ -125,7 +125,7 @@ func (ep *Entrypoint) createIntegrationsGlobals(cfg *config.Config) (integration Metrics: ep.promMetrics, Logs: ep.lokiLogs, Tracing: ep.tempoTraces, - SubsystemOpts: cfg.Integrations, + // TODO(rfratto): set SubsystemOptions here when v1 is removed. AgentBaseURL: &url.URL{ Scheme: scheme, Host: fmt.Sprintf("127.0.0.1:%d", cfg.Server.HTTPListenPort), @@ -170,7 +170,7 @@ func (ep *Entrypoint) ApplyConfig(cfg config.Config) error { if err != nil { level.Error(ep.log).Log("msg", "failed to update integrations", "err", err) failed = true - } else if err := ep.integrations.ApplyConfig(integrationGlobals); err != nil { + } else if err := ep.integrations.ApplyConfig(&cfg.Integrations, integrationGlobals); err != nil { level.Error(ep.log).Log("msg", "failed to update integrations", "err", err) failed = true } diff --git a/pkg/config/config.go b/pkg/config/config.go index 4940d7e28139..a990d9f6f155 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -12,7 +12,7 @@ import ( "github.com/weaveworks/common/server" "github.com/drone/envsubst/v2" - "github.com/grafana/agent/pkg/integrations/v2" + integrations "github.com/grafana/agent/pkg/integrations/versionselector" "github.com/grafana/agent/pkg/logs" "github.com/grafana/agent/pkg/metrics" "github.com/grafana/agent/pkg/traces" @@ -25,17 +25,16 @@ import ( // DefaultConfig holds default settings for all the subsystems. var DefaultConfig = Config{ // All subsystems with a DefaultConfig should be listed here. - Metrics: metrics.DefaultConfig, - Integrations: integrations.DefaultSubsystemOptions, + Metrics: metrics.DefaultConfig, } // Config contains underlying configurations for the agent type Config struct { - Server server.Config `yaml:"server,omitempty"` - Metrics metrics.Config `yaml:"metrics,omitempty"` - Integrations integrations.SubsystemOptions `yaml:"integrations,omitempty"` - Traces traces.Config `yaml:"traces,omitempty"` - Logs *logs.Config `yaml:"logs,omitempty"` + Server server.Config `yaml:"server,omitempty"` + Metrics metrics.Config `yaml:"metrics,omitempty"` + Integrations integrations.Config `yaml:"integrations,omitempty"` + Traces traces.Config `yaml:"traces,omitempty"` + Logs *logs.Config `yaml:"logs,omitempty"` // We support a secondary server just for the /-/reload endpoint, since // invoking /-/reload against the primary server can cause the server @@ -49,11 +48,21 @@ type Config struct { // UnmarshalYAML implements yaml.Unmarshaler. func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { + // NOTE(rfratto): We must temporarily save the version used for integrations + // before setting everything to default values. The c.Integrations.Version + // field is set based on a flag, and we do not want to override it. + // + // This is gross, but necessary for now. + integrationsVersion := c.Integrations.Version + // Apply defaults to the config from our struct and any defaults inherited // from flags before unmarshaling. *c = DefaultConfig util.DefaultConfigFromFlags(c) + // Restore fields we don't want to override from defaults. + c.Integrations.Version = integrationsVersion + type baseConfig Config type config struct { @@ -117,8 +126,8 @@ func (c *Config) ApplyDefaults() error { c.Metrics.ServiceConfig.Lifecycler.ListenPort = c.Server.GRPCListenPort - if len(c.Integrations.PrometheusRemoteWrite) == 0 { - c.Integrations.PrometheusRemoteWrite = c.Metrics.Global.RemoteWrite + if err := c.Integrations.ApplyDefaults(&c.Server, &c.Metrics); err != nil { + return err } // since the Traces config might rely on an existing Loki config @@ -198,14 +207,16 @@ func load(fs *flag.FlagSet, args []string, loader func(string, bool, *Config) er var ( cfg = DefaultConfig - printVersion bool - file string - configExpandEnv bool + printVersion bool + file string + configExpandEnv bool + useIntegrationsV2 bool ) fs.StringVar(&file, "config.file", "", "configuration file to load") fs.BoolVar(&printVersion, "version", false, "Print this build's version information") fs.BoolVar(&configExpandEnv, "config.expand-env", false, "Expands ${var} in config according to the values of the environment variables.") + fs.BoolVar(&useIntegrationsV2, "experiment.integrations-next.enable", false, "Enable next-gen integrations.") cfg.RegisterFlags(fs) if err := fs.Parse(args); err != nil { @@ -217,6 +228,13 @@ func load(fs *flag.FlagSet, args []string, loader func(string, bool, *Config) er os.Exit(0) } + // Save the loaded integrations version before unmarshaling from YAML. + if useIntegrationsV2 { + cfg.Integrations.Version = integrations.Version2 + } else { + cfg.Integrations.Version = integrations.Version1 + } + if file == "" { return nil, fmt.Errorf("-config.file flag required") } else if err := loader(file, configExpandEnv, &cfg); err != nil { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 5be4d3248cb3..4fd576e73d19 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -6,7 +6,6 @@ import ( "testing" "time" - "github.com/grafana/agent/pkg/integrations/v2" "github.com/grafana/agent/pkg/metrics" "github.com/grafana/agent/pkg/metrics/instance" "github.com/grafana/agent/pkg/util" @@ -149,7 +148,6 @@ func TestConfig_Defaults(t *testing.T) { require.NoError(t, err) require.Equal(t, metrics.DefaultConfig, c.Metrics) - require.Equal(t, integrations.DefaultSubsystemOptions, c.Integrations) } func TestConfig_TracesLokiValidates(t *testing.T) { diff --git a/pkg/integrations/versionselector/selector.go b/pkg/integrations/versionselector/selector.go index 3c3521625426..5b2d282bfd74 100644 --- a/pkg/integrations/versionselector/selector.go +++ b/pkg/integrations/versionselector/selector.go @@ -9,55 +9,98 @@ import ( v2 "github.com/grafana/agent/pkg/integrations/v2" "github.com/grafana/agent/pkg/metrics" "github.com/weaveworks/common/server" + "gopkg.in/yaml.v2" +) + +type Version int + +const ( + VersionDefault Version = 0 + + Version1 Version = iota + Version2 ) // Config abstracts the subsystem configs for integrations v1 and v2. type Config struct { - // UseV2 should be true if the newer v2 package should be used. This MUST be - // set prior to unmarshaling. - UseV2 bool + Version Version configV1 *v1.ManagerConfig configV2 *v2.SubsystemOptions } +// init will initialize the inner config based on the set version. +func (c *Config) init() { + switch c.Version { + case VersionDefault, Version1: + if c.configV1 == nil { + val := v1.DefaultManagerConfig + c.configV1 = &val + } + case Version2: + if c.configV2 == nil { + val := v2.DefaultSubsystemOptions + c.configV2 = &val + } + } +} + +var ( + _ yaml.Unmarshaler = (*Config)(nil) + _ yaml.Marshaler = (*Config)(nil) +) + // UnmarshalYAML implements yaml.Unmarshaler. func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { - if !c.UseV2 { - return unmarshal(c.configV1) + c.init() + + if c.Version != Version2 { + return unmarshal(&c.configV1) } - return unmarshal(c.configV2) + return unmarshal(&c.configV2) } // MarshalYAML implements yaml.Marshaler. -func (c *Config) MarshalYAML() (interface{}, error) { - if !c.UseV2 { +func (c Config) MarshalYAML() (interface{}, error) { + c.init() + + if c.Version != Version2 { return c.configV1, nil } return c.configV2, nil } -// ApplyDefaults applies defaults to the subsystem based on globals. Only -// needed when UseV2 is false. +// ApplyDefaults applies defaults to the subsystem based on globals. func (c *Config) ApplyDefaults(scfg *server.Config, mcfg *metrics.Config) error { - if !c.UseV2 { + c.init() + + if c.Version != Version2 { return c.configV1.ApplyDefaults(scfg, mcfg) } + + if len(c.configV2.PrometheusRemoteWrite) == 0 { + c.configV2.PrometheusRemoteWrite = mcfg.Global.RemoteWrite + } + return nil } +type Globals = v2.Globals + // Subsystem is an abstraction over both the v1 and v2 systems. type Subsystem interface { - ApplyConfig(*Config, v2.Globals) error + ApplyConfig(*Config, Globals) error WireAPI(*mux.Router) Stop() } // NewSubsystem creates a new subsystem. globals should be provided regardless -// of useV2. globals.SubsystemOptions will be automatically set if useV2 is -// true. -func NewSubsystem(logger log.Logger, cfg *Config, globals v2.Globals, useV2 bool) (Subsystem, error) { - if !useV2 { +// of useV2. globals.SubsystemOptions will be automatically set if cfg.Version +// is set to Version2. +func NewSubsystem(logger log.Logger, cfg *Config, globals Globals) (Subsystem, error) { + cfg.init() + + if cfg.Version != Version2 { instance, err := v1.NewManager(*cfg.configV1, logger, globals.Metrics.InstanceManager(), globals.Metrics.Validate) if err != nil { return nil, err @@ -75,13 +118,13 @@ func NewSubsystem(logger log.Logger, cfg *Config, globals v2.Globals, useV2 bool type v1Subsystem struct{ *v1.Manager } -func (s *v1Subsystem) ApplyConfig(cfg *Config, globals v2.Globals) error { +func (s *v1Subsystem) ApplyConfig(cfg *Config, globals Globals) error { return s.Manager.ApplyConfig(*cfg.configV1) } type v2Subsystem struct{ *v2.Subsystem } -func (s *v2Subsystem) ApplyConfig(cfg *Config, globals v2.Globals) error { +func (s *v2Subsystem) ApplyConfig(cfg *Config, globals Globals) error { globals.SubsystemOpts = *cfg.configV2 return s.Subsystem.ApplyConfig(globals) } From 8037357c2aa2953ef2d3568a5b372e9dffe727c7 Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Thu, 16 Dec 2021 19:22:49 -0500 Subject: [PATCH 03/11] pkg/integrations/versionselector to file in pkg/config --- cmd/agent/entrypoint.go | 11 +- pkg/config/config.go | 19 ++- pkg/config/integrations.go | 143 +++++++++++++++++++ pkg/integrations/versionselector/selector.go | 130 ----------------- 4 files changed, 157 insertions(+), 146 deletions(-) create mode 100644 pkg/config/integrations.go delete mode 100644 pkg/integrations/versionselector/selector.go diff --git a/cmd/agent/entrypoint.go b/cmd/agent/entrypoint.go index 14267184abb6..370f457a5d9d 100644 --- a/cmd/agent/entrypoint.go +++ b/cmd/agent/entrypoint.go @@ -11,7 +11,6 @@ import ( "syscall" "github.com/gorilla/mux" - integrations "github.com/grafana/agent/pkg/integrations/versionselector" "github.com/grafana/agent/pkg/logs" "github.com/grafana/agent/pkg/metrics" "github.com/grafana/agent/pkg/metrics/instance" @@ -42,7 +41,7 @@ type Entrypoint struct { promMetrics *metrics.Agent lokiLogs *logs.Logs tempoTraces *traces.Traces - integrations integrations.Subsystem + integrations config.Integrations reloadListener net.Listener reloadServer *http.Server @@ -95,7 +94,7 @@ func NewEntrypoint(logger *util.Logger, cfg *config.Config, reloader Reloader) ( if err != nil { return nil, err } - ep.integrations, err = integrations.NewSubsystem(logger, &cfg.Integrations, integrationGlobals) + ep.integrations, err = config.NewIntegrations(logger, &cfg.Integrations, integrationGlobals) if err != nil { return nil, err } @@ -108,10 +107,10 @@ func NewEntrypoint(logger *util.Logger, cfg *config.Config, reloader Reloader) ( return ep, nil } -func (ep *Entrypoint) createIntegrationsGlobals(cfg *config.Config) (integrations.Globals, error) { +func (ep *Entrypoint) createIntegrationsGlobals(cfg *config.Config) (config.IntegrationsGlobals, error) { hostname, err := instance.Hostname() if err != nil { - return integrations.Globals{}, fmt.Errorf("getting hostname: %w", err) + return config.IntegrationsGlobals{}, fmt.Errorf("getting hostname: %w", err) } usingTLS := len(cfg.Server.HTTPTLSConfig.TLSCertPath) > 0 && len(cfg.Server.HTTPTLSConfig.TLSKeyPath) > 0 @@ -120,7 +119,7 @@ func (ep *Entrypoint) createIntegrationsGlobals(cfg *config.Config) (integration scheme = "https" } - return integrations.Globals{ + return config.IntegrationsGlobals{ AgentIdentifier: fmt.Sprintf("%s:%d", hostname, cfg.Server.HTTPListenPort), Metrics: ep.promMetrics, Logs: ep.lokiLogs, diff --git a/pkg/config/config.go b/pkg/config/config.go index a990d9f6f155..7731257c7d97 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -12,7 +12,6 @@ import ( "github.com/weaveworks/common/server" "github.com/drone/envsubst/v2" - integrations "github.com/grafana/agent/pkg/integrations/versionselector" "github.com/grafana/agent/pkg/logs" "github.com/grafana/agent/pkg/metrics" "github.com/grafana/agent/pkg/traces" @@ -30,11 +29,11 @@ var DefaultConfig = Config{ // Config contains underlying configurations for the agent type Config struct { - Server server.Config `yaml:"server,omitempty"` - Metrics metrics.Config `yaml:"metrics,omitempty"` - Integrations integrations.Config `yaml:"integrations,omitempty"` - Traces traces.Config `yaml:"traces,omitempty"` - Logs *logs.Config `yaml:"logs,omitempty"` + Server server.Config `yaml:"server,omitempty"` + Metrics metrics.Config `yaml:"metrics,omitempty"` + Integrations VersionedIntegrations `yaml:"integrations,omitempty"` + Traces traces.Config `yaml:"traces,omitempty"` + Logs *logs.Config `yaml:"logs,omitempty"` // We support a secondary server just for the /-/reload endpoint, since // invoking /-/reload against the primary server can cause the server @@ -53,7 +52,7 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { // field is set based on a flag, and we do not want to override it. // // This is gross, but necessary for now. - integrationsVersion := c.Integrations.Version + integrationsVersion := c.Integrations.version // Apply defaults to the config from our struct and any defaults inherited // from flags before unmarshaling. @@ -61,7 +60,7 @@ func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { util.DefaultConfigFromFlags(c) // Restore fields we don't want to override from defaults. - c.Integrations.Version = integrationsVersion + c.Integrations.version = integrationsVersion type baseConfig Config @@ -230,9 +229,9 @@ func load(fs *flag.FlagSet, args []string, loader func(string, bool, *Config) er // Save the loaded integrations version before unmarshaling from YAML. if useIntegrationsV2 { - cfg.Integrations.Version = integrations.Version2 + cfg.Integrations.version = integrationsVersion2 } else { - cfg.Integrations.Version = integrations.Version1 + cfg.Integrations.version = integrationsVersion1 } if file == "" { diff --git a/pkg/config/integrations.go b/pkg/config/integrations.go new file mode 100644 index 000000000000..524aef8fcdc4 --- /dev/null +++ b/pkg/config/integrations.go @@ -0,0 +1,143 @@ +package config + +import ( + "reflect" + + "github.com/go-kit/log" + "github.com/gorilla/mux" + v1 "github.com/grafana/agent/pkg/integrations" + v2 "github.com/grafana/agent/pkg/integrations/v2" + "github.com/grafana/agent/pkg/metrics" + "github.com/weaveworks/common/server" + "gopkg.in/yaml.v2" +) + +type integrationsVersion int + +const ( + integrationsVersionDefault integrationsVersion = 0 + + integrationsVersion1 integrationsVersion = iota + integrationsVersion2 +) + +// VersionedIntegrations abstracts the subsystem configs for integrations v1 and v2. +type VersionedIntegrations struct { + version integrationsVersion + + configV1 *v1.ManagerConfig + configV2 *v2.SubsystemOptions +} + +// init will initialize the inner config based on the set version. +func (c *VersionedIntegrations) init() { + switch c.version { + case integrationsVersionDefault, integrationsVersion1: + if c.configV1 == nil { + val := v1.DefaultManagerConfig + c.configV1 = &val + } + case integrationsVersion2: + if c.configV2 == nil { + val := v2.DefaultSubsystemOptions + c.configV2 = &val + } + } +} + +var ( + _ yaml.Unmarshaler = (*VersionedIntegrations)(nil) + _ yaml.Marshaler = (*VersionedIntegrations)(nil) +) + +// UnmarshalYAML implements yaml.Unmarshaler. +func (c *VersionedIntegrations) UnmarshalYAML(unmarshal func(interface{}) error) error { + c.init() + + if c.version != integrationsVersion2 { + return unmarshal(&c.configV1) + } + return unmarshal(&c.configV2) +} + +// MarshalYAML implements yaml.Marshaler. +func (c VersionedIntegrations) MarshalYAML() (interface{}, error) { + c.init() + + if c.version != integrationsVersion2 { + return c.configV1, nil + } + return c.configV2, nil +} + +// IsZero implements yaml.IsZeroer. +func (c VersionedIntegrations) IsZero() bool { + switch { + case c.configV1 != nil: + return reflect.ValueOf(*c.configV1).IsZero() + case c.configV2 != nil: + return reflect.ValueOf(*c.configV2).IsZero() + default: + return true + } +} + +// ApplyDefaults applies defaults to the subsystem based on globals. +func (c *VersionedIntegrations) ApplyDefaults(scfg *server.Config, mcfg *metrics.Config) error { + c.init() + + if c.version != integrationsVersion2 { + return c.configV1.ApplyDefaults(scfg, mcfg) + } + + if len(c.configV2.PrometheusRemoteWrite) == 0 { + c.configV2.PrometheusRemoteWrite = mcfg.Global.RemoteWrite + } + + return nil +} + +// IntegrationsGlobals is a global struct shared across integrations. +type IntegrationsGlobals = v2.Globals + +// Integrations is an abstraction over both the v1 and v2 systems. +type Integrations interface { + ApplyConfig(*VersionedIntegrations, IntegrationsGlobals) error + WireAPI(*mux.Router) + Stop() +} + +// NewIntegrations creates a new subsystem. globals should be provided regardless +// of useV2. globals.SubsystemOptions will be automatically set if cfg.Version +// is set to IntegrationsVersion2. +func NewIntegrations(logger log.Logger, cfg *VersionedIntegrations, globals IntegrationsGlobals) (Integrations, error) { + cfg.init() + + if cfg.version != integrationsVersion2 { + instance, err := v1.NewManager(*cfg.configV1, logger, globals.Metrics.InstanceManager(), globals.Metrics.Validate) + if err != nil { + return nil, err + } + return &v1Integrations{Manager: instance}, nil + } + + globals.SubsystemOpts = *cfg.configV2 + instance, err := v2.NewSubsystem(logger, globals) + if err != nil { + return nil, err + } + return &v2Integrations{Subsystem: instance}, nil +} + +type v1Integrations struct{ *v1.Manager } + +func (s *v1Integrations) ApplyConfig(cfg *VersionedIntegrations, globals IntegrationsGlobals) error { + return s.Manager.ApplyConfig(*cfg.configV1) +} + +type v2Integrations struct{ *v2.Subsystem } + +func (s *v2Integrations) ApplyConfig(cfg *VersionedIntegrations, globals IntegrationsGlobals) error { + globals.SubsystemOpts = *cfg.configV2 + return s.Subsystem.ApplyConfig(globals) +} diff --git a/pkg/integrations/versionselector/selector.go b/pkg/integrations/versionselector/selector.go deleted file mode 100644 index 5b2d282bfd74..000000000000 --- a/pkg/integrations/versionselector/selector.go +++ /dev/null @@ -1,130 +0,0 @@ -// Package integrations exposes the integrations subsystem. It will select -// between v1 and v2 based on a field. -package integrations - -import ( - "github.com/go-kit/log" - "github.com/gorilla/mux" - v1 "github.com/grafana/agent/pkg/integrations" - v2 "github.com/grafana/agent/pkg/integrations/v2" - "github.com/grafana/agent/pkg/metrics" - "github.com/weaveworks/common/server" - "gopkg.in/yaml.v2" -) - -type Version int - -const ( - VersionDefault Version = 0 - - Version1 Version = iota - Version2 -) - -// Config abstracts the subsystem configs for integrations v1 and v2. -type Config struct { - Version Version - - configV1 *v1.ManagerConfig - configV2 *v2.SubsystemOptions -} - -// init will initialize the inner config based on the set version. -func (c *Config) init() { - switch c.Version { - case VersionDefault, Version1: - if c.configV1 == nil { - val := v1.DefaultManagerConfig - c.configV1 = &val - } - case Version2: - if c.configV2 == nil { - val := v2.DefaultSubsystemOptions - c.configV2 = &val - } - } -} - -var ( - _ yaml.Unmarshaler = (*Config)(nil) - _ yaml.Marshaler = (*Config)(nil) -) - -// UnmarshalYAML implements yaml.Unmarshaler. -func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { - c.init() - - if c.Version != Version2 { - return unmarshal(&c.configV1) - } - return unmarshal(&c.configV2) -} - -// MarshalYAML implements yaml.Marshaler. -func (c Config) MarshalYAML() (interface{}, error) { - c.init() - - if c.Version != Version2 { - return c.configV1, nil - } - return c.configV2, nil -} - -// ApplyDefaults applies defaults to the subsystem based on globals. -func (c *Config) ApplyDefaults(scfg *server.Config, mcfg *metrics.Config) error { - c.init() - - if c.Version != Version2 { - return c.configV1.ApplyDefaults(scfg, mcfg) - } - - if len(c.configV2.PrometheusRemoteWrite) == 0 { - c.configV2.PrometheusRemoteWrite = mcfg.Global.RemoteWrite - } - - return nil -} - -type Globals = v2.Globals - -// Subsystem is an abstraction over both the v1 and v2 systems. -type Subsystem interface { - ApplyConfig(*Config, Globals) error - WireAPI(*mux.Router) - Stop() -} - -// NewSubsystem creates a new subsystem. globals should be provided regardless -// of useV2. globals.SubsystemOptions will be automatically set if cfg.Version -// is set to Version2. -func NewSubsystem(logger log.Logger, cfg *Config, globals Globals) (Subsystem, error) { - cfg.init() - - if cfg.Version != Version2 { - instance, err := v1.NewManager(*cfg.configV1, logger, globals.Metrics.InstanceManager(), globals.Metrics.Validate) - if err != nil { - return nil, err - } - return &v1Subsystem{Manager: instance}, nil - } - - globals.SubsystemOpts = *cfg.configV2 - instance, err := v2.NewSubsystem(logger, globals) - if err != nil { - return nil, err - } - return &v2Subsystem{Subsystem: instance}, nil -} - -type v1Subsystem struct{ *v1.Manager } - -func (s *v1Subsystem) ApplyConfig(cfg *Config, globals Globals) error { - return s.Manager.ApplyConfig(*cfg.configV1) -} - -type v2Subsystem struct{ *v2.Subsystem } - -func (s *v2Subsystem) ApplyConfig(cfg *Config, globals Globals) error { - globals.SubsystemOpts = *cfg.configV2 - return s.Subsystem.ApplyConfig(globals) -} From 96819950e7005073c29d511459fed1d390f59a5b Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Thu, 16 Dec 2021 19:26:34 -0500 Subject: [PATCH 04/11] pkg/config: fix defaults for Integrations --- pkg/config/config.go | 3 ++- pkg/config/integrations.go | 34 +++++++++++----------------------- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 7731257c7d97..2ea0b674a49b 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -24,7 +24,8 @@ import ( // DefaultConfig holds default settings for all the subsystems. var DefaultConfig = Config{ // All subsystems with a DefaultConfig should be listed here. - Metrics: metrics.DefaultConfig, + Metrics: metrics.DefaultConfig, + Integrations: DefaultVersionedIntegrations, } // Config contains underlying configurations for the agent diff --git a/pkg/config/integrations.go b/pkg/config/integrations.go index 524aef8fcdc4..da746b66af38 100644 --- a/pkg/config/integrations.go +++ b/pkg/config/integrations.go @@ -21,6 +21,14 @@ const ( integrationsVersion2 ) +var DefaultVersionedIntegrations = VersionedIntegrations{ + version: integrationsVersion1, + configV1: func() *v1.ManagerConfig { + cfg := v1.DefaultManagerConfig + return &cfg + }(), +} + // VersionedIntegrations abstracts the subsystem configs for integrations v1 and v2. type VersionedIntegrations struct { version integrationsVersion @@ -29,22 +37,6 @@ type VersionedIntegrations struct { configV2 *v2.SubsystemOptions } -// init will initialize the inner config based on the set version. -func (c *VersionedIntegrations) init() { - switch c.version { - case integrationsVersionDefault, integrationsVersion1: - if c.configV1 == nil { - val := v1.DefaultManagerConfig - c.configV1 = &val - } - case integrationsVersion2: - if c.configV2 == nil { - val := v2.DefaultSubsystemOptions - c.configV2 = &val - } - } -} - var ( _ yaml.Unmarshaler = (*VersionedIntegrations)(nil) _ yaml.Marshaler = (*VersionedIntegrations)(nil) @@ -52,18 +44,18 @@ var ( // UnmarshalYAML implements yaml.Unmarshaler. func (c *VersionedIntegrations) UnmarshalYAML(unmarshal func(interface{}) error) error { - c.init() + c.configV1 = nil + c.configV2 = nil if c.version != integrationsVersion2 { return unmarshal(&c.configV1) } + return unmarshal(&c.configV2) } // MarshalYAML implements yaml.Marshaler. func (c VersionedIntegrations) MarshalYAML() (interface{}, error) { - c.init() - if c.version != integrationsVersion2 { return c.configV1, nil } @@ -84,8 +76,6 @@ func (c VersionedIntegrations) IsZero() bool { // ApplyDefaults applies defaults to the subsystem based on globals. func (c *VersionedIntegrations) ApplyDefaults(scfg *server.Config, mcfg *metrics.Config) error { - c.init() - if c.version != integrationsVersion2 { return c.configV1.ApplyDefaults(scfg, mcfg) } @@ -111,8 +101,6 @@ type Integrations interface { // of useV2. globals.SubsystemOptions will be automatically set if cfg.Version // is set to IntegrationsVersion2. func NewIntegrations(logger log.Logger, cfg *VersionedIntegrations, globals IntegrationsGlobals) (Integrations, error) { - cfg.init() - if cfg.version != integrationsVersion2 { instance, err := v1.NewManager(*cfg.configV1, logger, globals.Metrics.InstanceManager(), globals.Metrics.Validate) if err != nil { From 0326eef5835a0471fada9eb86c7e32624de7ff9b Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Thu, 16 Dec 2021 19:50:58 -0500 Subject: [PATCH 05/11] pkg/config: use more generic way to unmarshal differently based on flag --- pkg/config/config.go | 37 ++++++++++++++++++++++--------------- pkg/config/config_test.go | 1 + pkg/config/integrations.go | 3 +++ 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 2ea0b674a49b..4e5370af05bb 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -30,6 +30,10 @@ var DefaultConfig = Config{ // Config contains underlying configurations for the agent type Config struct { + // Custom defaults to use. When non-nil, defaultConfig must be recursive and + // contain a pointer to itself at defaultConfig.defaultConfig. + defaultConfig *Config + Server server.Config `yaml:"server,omitempty"` Metrics metrics.Config `yaml:"metrics,omitempty"` Integrations VersionedIntegrations `yaml:"integrations,omitempty"` @@ -48,21 +52,17 @@ type Config struct { // UnmarshalYAML implements yaml.Unmarshaler. func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { - // NOTE(rfratto): We must temporarily save the version used for integrations - // before setting everything to default values. The c.Integrations.Version - // field is set based on a flag, and we do not want to override it. - // - // This is gross, but necessary for now. - integrationsVersion := c.Integrations.version - - // Apply defaults to the config from our struct and any defaults inherited - // from flags before unmarshaling. - *c = DefaultConfig + // The root config has an unfortunate quirk: flags may change defaults, such + // as when enabling the feature flag to use the new implementation of + // integrations. We don't want to have flags affect globals, so we cache + // custom defaults in a field instead. + if c.defaultConfig != nil { + *c = *c.defaultConfig + } else { + *c = DefaultConfig + } util.DefaultConfigFromFlags(c) - // Restore fields we don't want to override from defaults. - c.Integrations.version = integrationsVersion - type baseConfig Config type config struct { @@ -228,11 +228,18 @@ func load(fs *flag.FlagSet, args []string, loader func(string, bool, *Config) er os.Exit(0) } + // Give our cfg a custom set of defaults that can safely be modified locally. + // The defaults are self-recursive in case Unmarshal is called multiple + // times. + configDefaults := DefaultConfig + configDefaults.defaultConfig = &configDefaults + cfg.defaultConfig = &configDefaults + // Save the loaded integrations version before unmarshaling from YAML. if useIntegrationsV2 { - cfg.Integrations.version = integrationsVersion2 + configDefaults.Integrations.version = integrationsVersion2 } else { - cfg.Integrations.version = integrationsVersion1 + configDefaults.Integrations.version = integrationsVersion1 } if file == "" { diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 4fd576e73d19..e088e61a3c02 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -148,6 +148,7 @@ func TestConfig_Defaults(t *testing.T) { require.NoError(t, err) require.Equal(t, metrics.DefaultConfig, c.Metrics) + require.Equal(t, DefaultVersionedIntegrations, c.Integrations) } func TestConfig_TracesLokiValidates(t *testing.T) { diff --git a/pkg/config/integrations.go b/pkg/config/integrations.go index da746b66af38..60e16f2fec07 100644 --- a/pkg/config/integrations.go +++ b/pkg/config/integrations.go @@ -8,6 +8,7 @@ import ( v1 "github.com/grafana/agent/pkg/integrations" v2 "github.com/grafana/agent/pkg/integrations/v2" "github.com/grafana/agent/pkg/metrics" + "github.com/prometheus/statsd_exporter/pkg/level" "github.com/weaveworks/common/server" "gopkg.in/yaml.v2" ) @@ -109,6 +110,8 @@ func NewIntegrations(logger log.Logger, cfg *VersionedIntegrations, globals Inte return &v1Integrations{Manager: instance}, nil } + level.Warn(logger).Log("msg", "integrations-next is enabled. integrations-next is subject to change") + globals.SubsystemOpts = *cfg.configV2 instance, err := v2.NewSubsystem(logger, globals) if err != nil { From 74ec74f9c6b04367e15ddb9e8d2a0d9d912db31e Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Thu, 16 Dec 2021 19:52:13 -0500 Subject: [PATCH 06/11] add missing godoc comment --- pkg/config/integrations.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/config/integrations.go b/pkg/config/integrations.go index 60e16f2fec07..5e1693b86c16 100644 --- a/pkg/config/integrations.go +++ b/pkg/config/integrations.go @@ -16,12 +16,11 @@ import ( type integrationsVersion int const ( - integrationsVersionDefault integrationsVersion = 0 - integrationsVersion1 integrationsVersion = iota integrationsVersion2 ) +// DefaultVersionedIntegrations is the default config for integrations. var DefaultVersionedIntegrations = VersionedIntegrations{ version: integrationsVersion1, configV1: func() *v1.ManagerConfig { From d686ce7777019788943f751d31203dd47812cd49 Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Thu, 16 Dec 2021 19:56:36 -0500 Subject: [PATCH 07/11] more comments --- pkg/config/integrations.go | 17 ++++++++--------- pkg/integrations/v2/subsystem.go | 10 ++++++++++ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/pkg/config/integrations.go b/pkg/config/integrations.go index 5e1693b86c16..3b0b5133c833 100644 --- a/pkg/config/integrations.go +++ b/pkg/config/integrations.go @@ -29,7 +29,9 @@ var DefaultVersionedIntegrations = VersionedIntegrations{ }(), } -// VersionedIntegrations abstracts the subsystem configs for integrations v1 and v2. +// VersionedIntegrations abstracts the subsystem configs for integrations v1 +// and v2. The version can only be set when calling Load through command line +// flags. type VersionedIntegrations struct { version integrationsVersion @@ -42,7 +44,8 @@ var ( _ yaml.Marshaler = (*VersionedIntegrations)(nil) ) -// UnmarshalYAML implements yaml.Unmarshaler. +// UnmarshalYAML implements yaml.Unmarshaler. Unmarshals to the enabled +// integrations subsystem version. func (c *VersionedIntegrations) UnmarshalYAML(unmarshal func(interface{}) error) error { c.configV1 = nil c.configV2 = nil @@ -54,7 +57,8 @@ func (c *VersionedIntegrations) UnmarshalYAML(unmarshal func(interface{}) error) return unmarshal(&c.configV2) } -// MarshalYAML implements yaml.Marshaler. +// MarshalYAML implements yaml.Marshaler. Marshals the enabled integrations +// subsystem version. func (c VersionedIntegrations) MarshalYAML() (interface{}, error) { if c.version != integrationsVersion2 { return c.configV1, nil @@ -79,12 +83,7 @@ func (c *VersionedIntegrations) ApplyDefaults(scfg *server.Config, mcfg *metrics if c.version != integrationsVersion2 { return c.configV1.ApplyDefaults(scfg, mcfg) } - - if len(c.configV2.PrometheusRemoteWrite) == 0 { - c.configV2.PrometheusRemoteWrite = mcfg.Global.RemoteWrite - } - - return nil + return c.configV2.ApplyDefaults(mcfg) } // IntegrationsGlobals is a global struct shared across integrations. diff --git a/pkg/integrations/v2/subsystem.go b/pkg/integrations/v2/subsystem.go index 28bcab0ac839..319676917c7f 100644 --- a/pkg/integrations/v2/subsystem.go +++ b/pkg/integrations/v2/subsystem.go @@ -10,6 +10,7 @@ import ( "github.com/go-kit/log" "github.com/gorilla/mux" + "github.com/grafana/agent/pkg/metrics" "github.com/grafana/agent/pkg/metrics/instance" common_config "github.com/prometheus/common/config" "github.com/prometheus/common/model" @@ -46,6 +47,15 @@ type SubsystemOptions struct { ClientConfig common_config.HTTPClientConfig `yaml:"client_config,omitempty"` } +// ApplyDefaults will apply defaults to o. +func (o *SubsystemOptions) ApplyDefaults(mcfg *metrics.Config) error { + if len(o.PrometheusRemoteWrite) == 0 { + o.PrometheusRemoteWrite = mcfg.Global.RemoteWrite + } + + return nil +} + // MarshalYAML implements yaml.Marshaler for SubsystemOptions. Integrations // will be marshaled inline. func (o SubsystemOptions) MarshalYAML() (interface{}, error) { From 86ed5aa63d1c267a374a185445dfc5d282180eb7 Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Thu, 16 Dec 2021 20:57:53 -0500 Subject: [PATCH 08/11] switch to deferred unmarshaling --- pkg/config/config.go | 34 ++++++------------- pkg/config/integrations.go | 69 +++++++++++++++++++++++++++++--------- pkg/util/rawyaml.go | 25 ++++++++++++++ 3 files changed, 89 insertions(+), 39 deletions(-) create mode 100644 pkg/util/rawyaml.go diff --git a/pkg/config/config.go b/pkg/config/config.go index 4e5370af05bb..88e693e3f728 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -52,15 +52,9 @@ type Config struct { // UnmarshalYAML implements yaml.Unmarshaler. func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error { - // The root config has an unfortunate quirk: flags may change defaults, such - // as when enabling the feature flag to use the new implementation of - // integrations. We don't want to have flags affect globals, so we cache - // custom defaults in a field instead. - if c.defaultConfig != nil { - *c = *c.defaultConfig - } else { - *c = DefaultConfig - } + // Apply defaults to the config from our struct and any defaults inherited + // from flags before unmarshaling. + *c = DefaultConfig util.DefaultConfigFromFlags(c) type baseConfig Config @@ -228,20 +222,6 @@ func load(fs *flag.FlagSet, args []string, loader func(string, bool, *Config) er os.Exit(0) } - // Give our cfg a custom set of defaults that can safely be modified locally. - // The defaults are self-recursive in case Unmarshal is called multiple - // times. - configDefaults := DefaultConfig - configDefaults.defaultConfig = &configDefaults - cfg.defaultConfig = &configDefaults - - // Save the loaded integrations version before unmarshaling from YAML. - if useIntegrationsV2 { - configDefaults.Integrations.version = integrationsVersion2 - } else { - configDefaults.Integrations.version = integrationsVersion1 - } - if file == "" { return nil, fmt.Errorf("-config.file flag required") } else if err := loader(file, configExpandEnv, &cfg); err != nil { @@ -254,6 +234,14 @@ func load(fs *flag.FlagSet, args []string, loader func(string, bool, *Config) er return nil, fmt.Errorf("error parsing flags: %w", err) } + // Pass the used integrations version to our config. This MUST be done before + // ApplyDefaults, which will perform deferred unmarshaling. + if useIntegrationsV2 { + cfg.Integrations.version = integrationsVersion2 + } else { + cfg.Integrations.version = integrationsVersion1 + } + // Finally, apply defaults to config that wasn't specified by file or flag if err := cfg.ApplyDefaults(); err != nil { return nil, fmt.Errorf("error in config file: %w", err) diff --git a/pkg/config/integrations.go b/pkg/config/integrations.go index 3b0b5133c833..9711da51a5dd 100644 --- a/pkg/config/integrations.go +++ b/pkg/config/integrations.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "reflect" "github.com/go-kit/log" @@ -8,6 +9,7 @@ import ( v1 "github.com/grafana/agent/pkg/integrations" v2 "github.com/grafana/agent/pkg/integrations/v2" "github.com/grafana/agent/pkg/metrics" + "github.com/grafana/agent/pkg/util" "github.com/prometheus/statsd_exporter/pkg/level" "github.com/weaveworks/common/server" "gopkg.in/yaml.v2" @@ -23,9 +25,12 @@ const ( // DefaultVersionedIntegrations is the default config for integrations. var DefaultVersionedIntegrations = VersionedIntegrations{ version: integrationsVersion1, - configV1: func() *v1.ManagerConfig { - cfg := v1.DefaultManagerConfig - return &cfg + raw: func() []byte { + bb, err := yaml.Marshal(&v1.DefaultManagerConfig) + if err != nil { + panic(err) + } + return bb }(), } @@ -34,6 +39,7 @@ var DefaultVersionedIntegrations = VersionedIntegrations{ // flags. type VersionedIntegrations struct { version integrationsVersion + raw util.RawYAML configV1 *v1.ManagerConfig configV2 *v2.SubsystemOptions @@ -44,26 +50,23 @@ var ( _ yaml.Marshaler = (*VersionedIntegrations)(nil) ) -// UnmarshalYAML implements yaml.Unmarshaler. Unmarshals to the enabled -// integrations subsystem version. +// UnmarshalYAML implements yaml.Unmarshaler. func (c *VersionedIntegrations) UnmarshalYAML(unmarshal func(interface{}) error) error { c.configV1 = nil c.configV2 = nil - - if c.version != integrationsVersion2 { - return unmarshal(&c.configV1) - } - - return unmarshal(&c.configV2) + return unmarshal(&c.raw) } -// MarshalYAML implements yaml.Marshaler. Marshals the enabled integrations -// subsystem version. +// MarshalYAML implements yaml.Marshaler. func (c VersionedIntegrations) MarshalYAML() (interface{}, error) { - if c.version != integrationsVersion2 { + switch { + case c.configV1 != nil: return c.configV1, nil + case c.configV2 != nil: + return c.configV2, nil + default: + return c.raw, nil } - return c.configV2, nil } // IsZero implements yaml.IsZeroer. @@ -74,18 +77,48 @@ func (c VersionedIntegrations) IsZero() bool { case c.configV2 != nil: return reflect.ValueOf(*c.configV2).IsZero() default: - return true + return len(c.raw) == 0 } } // ApplyDefaults applies defaults to the subsystem based on globals. func (c *VersionedIntegrations) ApplyDefaults(scfg *server.Config, mcfg *metrics.Config) error { + if err := c.completeUnmarshal(); err != nil { + return err + } if c.version != integrationsVersion2 { return c.configV1.ApplyDefaults(scfg, mcfg) } return c.configV2.ApplyDefaults(mcfg) } +// completeUnmarshal will unmarshal the raw config based on c.version. No-op if +// previously called successfully. +func (c *VersionedIntegrations) completeUnmarshal() error { + if c.configV1 != nil || c.configV2 != nil { + return nil + } + + var out interface{} + + switch c.version { + case integrationsVersion1: + cfg := v1.DefaultManagerConfig + c.configV1 = &cfg + + out = c.configV1 + case integrationsVersion2: + cfg := v2.DefaultSubsystemOptions + c.configV2 = &cfg + + out = c.configV2 + default: + panic(fmt.Sprintf("unknown integrations version %d", c.version)) + } + + return yaml.UnmarshalStrict(c.raw, out) +} + // IntegrationsGlobals is a global struct shared across integrations. type IntegrationsGlobals = v2.Globals @@ -100,6 +133,10 @@ type Integrations interface { // of useV2. globals.SubsystemOptions will be automatically set if cfg.Version // is set to IntegrationsVersion2. func NewIntegrations(logger log.Logger, cfg *VersionedIntegrations, globals IntegrationsGlobals) (Integrations, error) { + if err := cfg.completeUnmarshal(); err != nil { + return nil, err + } + if cfg.version != integrationsVersion2 { instance, err := v1.NewManager(*cfg.configV1, logger, globals.Metrics.InstanceManager(), globals.Metrics.Validate) if err != nil { diff --git a/pkg/util/rawyaml.go b/pkg/util/rawyaml.go new file mode 100644 index 000000000000..42ef2accaa73 --- /dev/null +++ b/pkg/util/rawyaml.go @@ -0,0 +1,25 @@ +package util + +import "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 string(r), nil +} From 17daf66ad810900af092666382451cf64dc717b8 Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Thu, 16 Dec 2021 20:58:33 -0500 Subject: [PATCH 09/11] remove unused Config field --- pkg/config/config.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 88e693e3f728..b8cf91bfea7d 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -30,10 +30,6 @@ var DefaultConfig = Config{ // Config contains underlying configurations for the agent type Config struct { - // Custom defaults to use. When non-nil, defaultConfig must be recursive and - // contain a pointer to itself at defaultConfig.defaultConfig. - defaultConfig *Config - Server server.Config `yaml:"server,omitempty"` Metrics metrics.Config `yaml:"metrics,omitempty"` Integrations VersionedIntegrations `yaml:"integrations,omitempty"` From a7b1873c7e852c7fda1d5564ac6f6d29c9ffd4b6 Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Thu, 16 Dec 2021 21:01:14 -0500 Subject: [PATCH 10/11] simplify completeUnmarshal --- pkg/config/integrations.go | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/pkg/config/integrations.go b/pkg/config/integrations.go index 9711da51a5dd..f5e4d4a110ad 100644 --- a/pkg/config/integrations.go +++ b/pkg/config/integrations.go @@ -99,24 +99,18 @@ func (c *VersionedIntegrations) completeUnmarshal() error { return nil } - var out interface{} - switch c.version { case integrationsVersion1: cfg := v1.DefaultManagerConfig c.configV1 = &cfg - - out = c.configV1 + return yaml.UnmarshalStrict(c.raw, c.configV1) case integrationsVersion2: cfg := v2.DefaultSubsystemOptions c.configV2 = &cfg - - out = c.configV2 + return yaml.UnmarshalStrict(c.raw, c.configV2) default: panic(fmt.Sprintf("unknown integrations version %d", c.version)) } - - return yaml.UnmarshalStrict(c.raw, out) } // IntegrationsGlobals is a global struct shared across integrations. From 96de6e131200599a667a1371155c708834580df2 Mon Sep 17 00:00:00 2001 From: Robert Fratto Date: Thu, 16 Dec 2021 21:12:45 -0500 Subject: [PATCH 11/11] do not perform lazy deferred unmarshaling --- pkg/config/config.go | 13 +++++++------ pkg/config/integrations.go | 33 +++++++++------------------------ 2 files changed, 16 insertions(+), 30 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index b8cf91bfea7d..b98a2d6cfcb6 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -230,18 +230,19 @@ func load(fs *flag.FlagSet, args []string, loader func(string, bool, *Config) er return nil, fmt.Errorf("error parsing flags: %w", err) } - // Pass the used integrations version to our config. This MUST be done before - // ApplyDefaults, which will perform deferred unmarshaling. + // Complete unmarshaling integrations using the version from the flag. This + // MUST be called before ApplyDefaults. + version := integrationsVersion1 if useIntegrationsV2 { - cfg.Integrations.version = integrationsVersion2 - } else { - cfg.Integrations.version = integrationsVersion1 + version = integrationsVersion2 + } + if err := cfg.Integrations.setVersion(version); err != nil { + return nil, fmt.Errorf("error loading config file %s: %w", file, err) } // Finally, apply defaults to config that wasn't specified by file or flag if err := cfg.ApplyDefaults(); err != nil { return nil, fmt.Errorf("error in config file: %w", err) } - return &cfg, nil } diff --git a/pkg/config/integrations.go b/pkg/config/integrations.go index f5e4d4a110ad..9cbec36cea44 100644 --- a/pkg/config/integrations.go +++ b/pkg/config/integrations.go @@ -24,19 +24,12 @@ const ( // DefaultVersionedIntegrations is the default config for integrations. var DefaultVersionedIntegrations = VersionedIntegrations{ - version: integrationsVersion1, - raw: func() []byte { - bb, err := yaml.Marshal(&v1.DefaultManagerConfig) - if err != nil { - panic(err) - } - return bb - }(), + version: integrationsVersion1, + configV1: &v1.DefaultManagerConfig, } // VersionedIntegrations abstracts the subsystem configs for integrations v1 -// and v2. The version can only be set when calling Load through command line -// flags. +// and v2. VersionedIntegrations can only be unmarshaled as part of Load. type VersionedIntegrations struct { version integrationsVersion raw util.RawYAML @@ -50,7 +43,8 @@ var ( _ yaml.Marshaler = (*VersionedIntegrations)(nil) ) -// UnmarshalYAML implements yaml.Unmarshaler. +// UnmarshalYAML implements yaml.Unmarshaler. Full unmarshaling is deferred until +// setVersion is invoked. func (c *VersionedIntegrations) UnmarshalYAML(unmarshal func(interface{}) error) error { c.configV1 = nil c.configV2 = nil @@ -83,21 +77,16 @@ func (c VersionedIntegrations) IsZero() bool { // ApplyDefaults applies defaults to the subsystem based on globals. func (c *VersionedIntegrations) ApplyDefaults(scfg *server.Config, mcfg *metrics.Config) error { - if err := c.completeUnmarshal(); err != nil { - return err - } if c.version != integrationsVersion2 { return c.configV1.ApplyDefaults(scfg, mcfg) } return c.configV2.ApplyDefaults(mcfg) } -// completeUnmarshal will unmarshal the raw config based on c.version. No-op if -// previously called successfully. -func (c *VersionedIntegrations) completeUnmarshal() error { - if c.configV1 != nil || c.configV2 != nil { - return nil - } +// setVersion completes the deferred unmarshal and unmarshals the raw YAML into +// the subsystem config for version v. +func (c *VersionedIntegrations) setVersion(v integrationsVersion) error { + c.version = v switch c.version { case integrationsVersion1: @@ -127,10 +116,6 @@ type Integrations interface { // of useV2. globals.SubsystemOptions will be automatically set if cfg.Version // is set to IntegrationsVersion2. func NewIntegrations(logger log.Logger, cfg *VersionedIntegrations, globals IntegrationsGlobals) (Integrations, error) { - if err := cfg.completeUnmarshal(); err != nil { - return nil, err - } - if cfg.version != integrationsVersion2 { instance, err := v1.NewManager(*cfg.configV1, logger, globals.Metrics.InstanceManager(), globals.Metrics.Validate) if err != nil {