From f72edff987c4790edd5871f805bad45108af8bad Mon Sep 17 00:00:00 2001 From: Dakota Paasman <122491662+dpaasman00@users.noreply.github.com> Date: Fri, 27 Sep 2024 10:10:24 -0400 Subject: [PATCH 1/9] supervisor logging --- cmd/opampsupervisor/main.go | 44 ++++++++++++++++--- .../supervisor/config/config.go | 12 +++++ cmd/opampsupervisor/supervisor/supervisor.go | 1 + .../supervisor/telemetry/logger.go | 18 ++++++++ 4 files changed, 68 insertions(+), 7 deletions(-) create mode 100644 cmd/opampsupervisor/supervisor/telemetry/logger.go diff --git a/cmd/opampsupervisor/main.go b/cmd/opampsupervisor/main.go index 137d2d37c3a2..b607e4e96944 100644 --- a/cmd/opampsupervisor/main.go +++ b/cmd/opampsupervisor/main.go @@ -4,27 +4,35 @@ package main import ( + "errors" "flag" + "fmt" + "log" "os" "os/signal" - "go.uber.org/zap" - + "github.com/knadh/koanf/parsers/yaml" + "github.com/knadh/koanf/providers/file" + "github.com/knadh/koanf/v2" "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor" "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/config" + "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/telemetry" ) func main() { configFlag := flag.String("config", "", "Path to a supervisor configuration file") flag.Parse() - logger, _ := zap.NewDevelopment() - + // load & validate config cfg, err := config.Load(*configFlag) if err != nil { - logger.Error(err.Error()) - os.Exit(-1) - return + log.Fatal("failed to load config: %w", err) + } + + // create logger + logger, err := telemetry.NewLogger(cfg.Telemetry.Logs) + if err != nil { + log.Fatal("failed to create logger: %w", err) } supervisor, err := supervisor.NewSupervisor(logger, cfg) @@ -46,3 +54,25 @@ func main() { <-interrupt supervisor.Shutdown() } + +func loadConfig(configFile string) (config.Supervisor, error) { + if configFile == "" { + return config.Supervisor{}, errors.New("path to config file cannot be empty") + } + + k := koanf.New("::") + if err := k.Load(file.Provider(configFile), yaml.Parser()); err != nil { + return config.Supervisor{}, err + } + + decodeConf := koanf.UnmarshalConf{ + Tag: "mapstructure", + } + + cfg := config.DefaultSupervisor() + if err := k.UnmarshalWithConf("", &cfg, decodeConf); err != nil { + return config.Supervisor{}, fmt.Errorf("cannot parse %v: %w", configFile, err) + } + + return cfg, nil +} diff --git a/cmd/opampsupervisor/supervisor/config/config.go b/cmd/opampsupervisor/supervisor/config/config.go index a260410f9c95..d9de7a9d2c1f 100644 --- a/cmd/opampsupervisor/supervisor/config/config.go +++ b/cmd/opampsupervisor/supervisor/config/config.go @@ -18,6 +18,7 @@ import ( "github.com/knadh/koanf/v2" "github.com/open-telemetry/opamp-go/protobufs" "go.opentelemetry.io/collector/config/configtls" + "go.uber.org/zap/zapcore" ) // Supervisor is the Supervisor config file format. @@ -26,6 +27,7 @@ type Supervisor struct { Agent Agent Capabilities Capabilities `mapstructure:"capabilities"` Storage Storage `mapstructure:"storage"` + Telemetry Telemetry `mapstructure:"telemetry"` } // Load loads the Supervisor config from a file. @@ -185,6 +187,16 @@ type AgentDescription struct { NonIdentifyingAttributes map[string]string `mapstructure:"non_identifying_attributes"` } +type Telemetry struct { + // TODO: flesh out other configurable telemetry options + Logs Logs `mapstructure:"logs"` +} + +type Logs struct { + Level zapcore.Level `mapstructure:"level"` + OutputPaths []string `mapstructure:"output_paths"` +} + // DefaultSupervisor returns the default supervisor config func DefaultSupervisor() Supervisor { defaultStorageDir := "/var/lib/otelcol/supervisor" diff --git a/cmd/opampsupervisor/supervisor/supervisor.go b/cmd/opampsupervisor/supervisor/supervisor.go index d96a1c8fbc93..1fb949a85461 100644 --- a/cmd/opampsupervisor/supervisor/supervisor.go +++ b/cmd/opampsupervisor/supervisor/supervisor.go @@ -150,6 +150,7 @@ type Supervisor struct { func NewSupervisor(logger *zap.Logger, cfg config.Supervisor) (*Supervisor, error) { s := &Supervisor{ + config: cfg, logger: logger, pidProvider: defaultPIDProvider{}, hasNewConfig: make(chan struct{}, 1), diff --git a/cmd/opampsupervisor/supervisor/telemetry/logger.go b/cmd/opampsupervisor/supervisor/telemetry/logger.go new file mode 100644 index 000000000000..41d5dabc4a3f --- /dev/null +++ b/cmd/opampsupervisor/supervisor/telemetry/logger.go @@ -0,0 +1,18 @@ +package telemetry + +import ( + "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/config" + "go.uber.org/zap" +) + +func NewLogger(cfg config.Logs) (*zap.Logger, error) { + zapCfg := zap.NewProductionConfig() + zapCfg.Level = zap.NewAtomicLevelAt(cfg.Level) + zapCfg.OutputPaths = cfg.OutputPaths + + logger, err := zapCfg.Build() + if err != nil { + return nil, err + } + return logger, nil +} From e78b33aa701d7ec58d3fd87f79f30c859b58366b Mon Sep 17 00:00:00 2001 From: Dakota Paasman <122491662+dpaasman00@users.noreply.github.com> Date: Fri, 27 Sep 2024 10:44:45 -0400 Subject: [PATCH 2/9] set defaults --- cmd/opampsupervisor/supervisor/config/config.go | 6 ++++++ cmd/opampsupervisor/supervisor/telemetry/logger.go | 1 + 2 files changed, 7 insertions(+) diff --git a/cmd/opampsupervisor/supervisor/config/config.go b/cmd/opampsupervisor/supervisor/config/config.go index d9de7a9d2c1f..16d584abeb37 100644 --- a/cmd/opampsupervisor/supervisor/config/config.go +++ b/cmd/opampsupervisor/supervisor/config/config.go @@ -229,5 +229,11 @@ func DefaultSupervisor() Supervisor { OrphanDetectionInterval: 5 * time.Second, BootstrapTimeout: 3 * time.Second, }, + Telemetry: Telemetry{ + Logs: Logs{ + Level: zapcore.InfoLevel, + OutputPaths: []string{"stdout", "stderr"}, + }, + }, } } diff --git a/cmd/opampsupervisor/supervisor/telemetry/logger.go b/cmd/opampsupervisor/supervisor/telemetry/logger.go index 41d5dabc4a3f..05b89b395f7f 100644 --- a/cmd/opampsupervisor/supervisor/telemetry/logger.go +++ b/cmd/opampsupervisor/supervisor/telemetry/logger.go @@ -7,6 +7,7 @@ import ( func NewLogger(cfg config.Logs) (*zap.Logger, error) { zapCfg := zap.NewProductionConfig() + zapCfg.Level = zap.NewAtomicLevelAt(cfg.Level) zapCfg.OutputPaths = cfg.OutputPaths From c80801739d6f68dbcb3587435b7b5c405505278b Mon Sep 17 00:00:00 2001 From: Dakota Paasman <122491662+dpaasman00@users.noreply.github.com> Date: Fri, 27 Sep 2024 10:52:07 -0400 Subject: [PATCH 3/9] create logger in NewSupervisor --- cmd/opampsupervisor/main.go | 39 +++-------------- cmd/opampsupervisor/supervisor/supervisor.go | 45 ++++++++++++++++++-- 2 files changed, 49 insertions(+), 35 deletions(-) diff --git a/cmd/opampsupervisor/main.go b/cmd/opampsupervisor/main.go index b607e4e96944..f765e4c6a251 100644 --- a/cmd/opampsupervisor/main.go +++ b/cmd/opampsupervisor/main.go @@ -4,25 +4,19 @@ package main import ( - "errors" "flag" - "fmt" "log" "os" "os/signal" - "github.com/knadh/koanf/parsers/yaml" - "github.com/knadh/koanf/providers/file" - "github.com/knadh/koanf/v2" "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor" - "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/config" - "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/telemetry" ) func main() { configFlag := flag.String("config", "", "Path to a supervisor configuration file") flag.Parse() +<<<<<<< HEAD // load & validate config cfg, err := config.Load(*configFlag) if err != nil { @@ -40,13 +34,16 @@ func main() { logger.Error(err.Error()) os.Exit(-1) return +======= + supervisor, err := supervisor.NewSupervisor(*configFlag) + if err != nil { + log.Fatal("failed to create new supervisor: %w", err) +>>>>>>> 4aa3ba6ab9 (create logger in NewSupervisor) } err = supervisor.Start() if err != nil { - logger.Error(err.Error()) - os.Exit(-1) - return + log.Fatal("failed to start supervisor: %w", err) } interrupt := make(chan os.Signal, 1) @@ -54,25 +51,3 @@ func main() { <-interrupt supervisor.Shutdown() } - -func loadConfig(configFile string) (config.Supervisor, error) { - if configFile == "" { - return config.Supervisor{}, errors.New("path to config file cannot be empty") - } - - k := koanf.New("::") - if err := k.Load(file.Provider(configFile), yaml.Parser()); err != nil { - return config.Supervisor{}, err - } - - decodeConf := koanf.UnmarshalConf{ - Tag: "mapstructure", - } - - cfg := config.DefaultSupervisor() - if err := k.UnmarshalWithConf("", &cfg, decodeConf); err != nil { - return config.Supervisor{}, fmt.Errorf("cannot parse %v: %w", configFile, err) - } - - return cfg, nil -} diff --git a/cmd/opampsupervisor/supervisor/supervisor.go b/cmd/opampsupervisor/supervisor/supervisor.go index 1fb949a85461..8b372bc009f0 100644 --- a/cmd/opampsupervisor/supervisor/supervisor.go +++ b/cmd/opampsupervisor/supervisor/supervisor.go @@ -25,6 +25,7 @@ import ( "github.com/google/uuid" "github.com/knadh/koanf/maps" "github.com/knadh/koanf/parsers/yaml" + "github.com/knadh/koanf/providers/file" "github.com/knadh/koanf/providers/rawbytes" "github.com/knadh/koanf/v2" "github.com/open-telemetry/opamp-go/client" @@ -41,6 +42,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/commander" "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/config" "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/healthchecker" + "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/telemetry" ) var ( @@ -148,10 +150,8 @@ type Supervisor struct { opampServerPort int } -func NewSupervisor(logger *zap.Logger, cfg config.Supervisor) (*Supervisor, error) { +func NewSupervisor(configFile string) (*Supervisor, error) { s := &Supervisor{ - config: cfg, - logger: logger, pidProvider: defaultPIDProvider{}, hasNewConfig: make(chan struct{}, 1), agentConfigOwnMetricsSection: &atomic.Value{}, @@ -166,16 +166,33 @@ func NewSupervisor(logger *zap.Logger, cfg config.Supervisor) (*Supervisor, erro return nil, err } +<<<<<<< HEAD if err := cfg.Validate(); err != nil { return nil, fmt.Errorf("error validating config: %w", err) } s.config = cfg +======= + if err := s.loadConfig(configFile); err != nil { + return nil, fmt.Errorf("error loading config: %w", err) + } + + if err := s.config.Validate(); err != nil { + return nil, fmt.Errorf("error validating config: %w", err) + } + +>>>>>>> 4aa3ba6ab9 (create logger in NewSupervisor) if err := os.MkdirAll(s.config.Storage.Directory, 0700); err != nil { return nil, fmt.Errorf("error creating storage dir: %w", err) } + logger, err := telemetry.NewLogger(s.config.Telemetry.Logs) + if err != nil { + return nil, fmt.Errorf("error creating logger: %w", err) + } + s.logger = logger + return s, nil } @@ -259,6 +276,28 @@ func (s *Supervisor) createTemplates() error { return nil } +func (s *Supervisor) loadConfig(configFile string) error { + if configFile == "" { + return errors.New("path to config file cannot be empty") + } + + k := koanf.New("::") + if err := k.Load(file.Provider(configFile), yaml.Parser()); err != nil { + return err + } + + decodeConf := koanf.UnmarshalConf{ + Tag: "mapstructure", + } + + s.config = config.DefaultSupervisor() + if err := k.UnmarshalWithConf("", &s.config, decodeConf); err != nil { + return fmt.Errorf("cannot parse %v: %w", configFile, err) + } + + return nil +} + // getBootstrapInfo obtains the Collector's agent description by // starting a Collector with a specific config that only starts // an OpAMP extension, obtains the agent description, then From 84defbec4db9e17aee7140c458d4350a6914ac3d Mon Sep 17 00:00:00 2001 From: Dakota Paasman <122491662+dpaasman00@users.noreply.github.com> Date: Fri, 27 Sep 2024 12:04:43 -0400 Subject: [PATCH 4/9] create logger & load cfg in main, fix ci, add chglog --- .../configurable-supervisor-logging.yaml | 27 +++++++++++++++++++ cmd/opampsupervisor/main.go | 8 ++---- .../supervisor/config/config.go | 22 +++++++++++++++ cmd/opampsupervisor/supervisor/supervisor.go | 23 +++------------- .../supervisor/telemetry/logger.go | 3 +++ 5 files changed, 57 insertions(+), 26 deletions(-) create mode 100644 .chloggen/configurable-supervisor-logging.yaml diff --git a/.chloggen/configurable-supervisor-logging.yaml b/.chloggen/configurable-supervisor-logging.yaml new file mode 100644 index 000000000000..eb5cacf7a9ac --- /dev/null +++ b/.chloggen/configurable-supervisor-logging.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: opampsupervisor + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add configurable logging for the supervisor. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [35466] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/cmd/opampsupervisor/main.go b/cmd/opampsupervisor/main.go index f765e4c6a251..a998ca0e8f0a 100644 --- a/cmd/opampsupervisor/main.go +++ b/cmd/opampsupervisor/main.go @@ -10,13 +10,14 @@ import ( "os/signal" "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor" + "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/config" + "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/telemetry" ) func main() { configFlag := flag.String("config", "", "Path to a supervisor configuration file") flag.Parse() -<<<<<<< HEAD // load & validate config cfg, err := config.Load(*configFlag) if err != nil { @@ -34,11 +35,6 @@ func main() { logger.Error(err.Error()) os.Exit(-1) return -======= - supervisor, err := supervisor.NewSupervisor(*configFlag) - if err != nil { - log.Fatal("failed to create new supervisor: %w", err) ->>>>>>> 4aa3ba6ab9 (create logger in NewSupervisor) } err = supervisor.Start() diff --git a/cmd/opampsupervisor/supervisor/config/config.go b/cmd/opampsupervisor/supervisor/config/config.go index 16d584abeb37..3a7f384db3f5 100644 --- a/cmd/opampsupervisor/supervisor/config/config.go +++ b/cmd/opampsupervisor/supervisor/config/config.go @@ -237,3 +237,25 @@ func DefaultSupervisor() Supervisor { }, } } + +func LoadConfig(configFile string) (Supervisor, error) { + if configFile == "" { + return Supervisor{}, errors.New("path to config file cannot be empty") + } + + k := koanf.New("::") + if err := k.Load(file.Provider(configFile), yaml.Parser()); err != nil { + return Supervisor{}, err + } + + decodeConf := koanf.UnmarshalConf{ + Tag: "mapstructure", + } + + cfg := DefaultSupervisor() + if err := k.UnmarshalWithConf("", &cfg, decodeConf); err != nil { + return Supervisor{}, fmt.Errorf("cannot parse %v: %w", configFile, err) + } + + return cfg, nil +} diff --git a/cmd/opampsupervisor/supervisor/supervisor.go b/cmd/opampsupervisor/supervisor/supervisor.go index 8b372bc009f0..f72cf5f8f7c5 100644 --- a/cmd/opampsupervisor/supervisor/supervisor.go +++ b/cmd/opampsupervisor/supervisor/supervisor.go @@ -42,7 +42,6 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/commander" "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/config" "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/healthchecker" - "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/telemetry" ) var ( @@ -150,8 +149,10 @@ type Supervisor struct { opampServerPort int } -func NewSupervisor(configFile string) (*Supervisor, error) { +func NewSupervisor(logger *zap.Logger, cfg config.Supervisor) (*Supervisor, error) { s := &Supervisor{ + config: cfg, + logger: logger, pidProvider: defaultPIDProvider{}, hasNewConfig: make(chan struct{}, 1), agentConfigOwnMetricsSection: &atomic.Value{}, @@ -166,33 +167,15 @@ func NewSupervisor(configFile string) (*Supervisor, error) { return nil, err } -<<<<<<< HEAD if err := cfg.Validate(); err != nil { return nil, fmt.Errorf("error validating config: %w", err) } - s.config = cfg -======= - if err := s.loadConfig(configFile); err != nil { - return nil, fmt.Errorf("error loading config: %w", err) - } - - if err := s.config.Validate(); err != nil { - return nil, fmt.Errorf("error validating config: %w", err) - } - ->>>>>>> 4aa3ba6ab9 (create logger in NewSupervisor) if err := os.MkdirAll(s.config.Storage.Directory, 0700); err != nil { return nil, fmt.Errorf("error creating storage dir: %w", err) } - logger, err := telemetry.NewLogger(s.config.Telemetry.Logs) - if err != nil { - return nil, fmt.Errorf("error creating logger: %w", err) - } - s.logger = logger - return s, nil } diff --git a/cmd/opampsupervisor/supervisor/telemetry/logger.go b/cmd/opampsupervisor/supervisor/telemetry/logger.go index 05b89b395f7f..92870ce77d75 100644 --- a/cmd/opampsupervisor/supervisor/telemetry/logger.go +++ b/cmd/opampsupervisor/supervisor/telemetry/logger.go @@ -1,3 +1,6 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + package telemetry import ( From 512c72ee2e2b7a90dcc5626ba17f24b0974b9a2e Mon Sep 17 00:00:00 2001 From: Dakota Paasman <122491662+dpaasman00@users.noreply.github.com> Date: Fri, 27 Sep 2024 13:10:32 -0400 Subject: [PATCH 5/9] remove old loadConfig --- cmd/opampsupervisor/supervisor/supervisor.go | 23 -------------------- 1 file changed, 23 deletions(-) diff --git a/cmd/opampsupervisor/supervisor/supervisor.go b/cmd/opampsupervisor/supervisor/supervisor.go index f72cf5f8f7c5..0c5f85abc6fb 100644 --- a/cmd/opampsupervisor/supervisor/supervisor.go +++ b/cmd/opampsupervisor/supervisor/supervisor.go @@ -25,7 +25,6 @@ import ( "github.com/google/uuid" "github.com/knadh/koanf/maps" "github.com/knadh/koanf/parsers/yaml" - "github.com/knadh/koanf/providers/file" "github.com/knadh/koanf/providers/rawbytes" "github.com/knadh/koanf/v2" "github.com/open-telemetry/opamp-go/client" @@ -259,28 +258,6 @@ func (s *Supervisor) createTemplates() error { return nil } -func (s *Supervisor) loadConfig(configFile string) error { - if configFile == "" { - return errors.New("path to config file cannot be empty") - } - - k := koanf.New("::") - if err := k.Load(file.Provider(configFile), yaml.Parser()); err != nil { - return err - } - - decodeConf := koanf.UnmarshalConf{ - Tag: "mapstructure", - } - - s.config = config.DefaultSupervisor() - if err := k.UnmarshalWithConf("", &s.config, decodeConf); err != nil { - return fmt.Errorf("cannot parse %v: %w", configFile, err) - } - - return nil -} - // getBootstrapInfo obtains the Collector's agent description by // starting a Collector with a specific config that only starts // an OpAMP extension, obtains the agent description, then From 7d977cfdd24d59c8ede1e323a250305e7ec2890f Mon Sep 17 00:00:00 2001 From: Dakota Paasman <122491662+dpaasman00@users.noreply.github.com> Date: Fri, 27 Sep 2024 14:59:25 -0400 Subject: [PATCH 6/9] ci --- cmd/opampsupervisor/supervisor/telemetry/logger.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/opampsupervisor/supervisor/telemetry/logger.go b/cmd/opampsupervisor/supervisor/telemetry/logger.go index 92870ce77d75..0bec527b1188 100644 --- a/cmd/opampsupervisor/supervisor/telemetry/logger.go +++ b/cmd/opampsupervisor/supervisor/telemetry/logger.go @@ -4,8 +4,9 @@ package telemetry import ( - "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/config" "go.uber.org/zap" + + "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/config" ) func NewLogger(cfg config.Logs) (*zap.Logger, error) { From 040949008d49cf7c19c20414ef9ee9f3e4e9f3f0 Mon Sep 17 00:00:00 2001 From: Dakota Paasman <122491662+dpaasman00@users.noreply.github.com> Date: Thu, 3 Oct 2024 10:07:59 -0400 Subject: [PATCH 7/9] e2e logging test --- cmd/opampsupervisor/e2e_test.go | 78 ++++++++++++++++++- .../supervisor/config/config.go | 22 ------ cmd/opampsupervisor/supervisor/supervisor.go | 3 +- .../supervisor/supervisor_logging.yaml | 21 +++++ 4 files changed, 98 insertions(+), 26 deletions(-) create mode 100644 cmd/opampsupervisor/testdata/supervisor/supervisor_logging.yaml diff --git a/cmd/opampsupervisor/e2e_test.go b/cmd/opampsupervisor/e2e_test.go index 4ec5034f785e..d8477815c07a 100644 --- a/cmd/opampsupervisor/e2e_test.go +++ b/cmd/opampsupervisor/e2e_test.go @@ -1,14 +1,14 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 -//go:build e2e - package main import ( + "bufio" "bytes" "context" "crypto/sha256" + "encoding/json" "errors" "fmt" "io" @@ -40,10 +40,12 @@ import ( "github.com/stretchr/testify/require" semconv "go.opentelemetry.io/collector/semconv/v1.21.0" "go.uber.org/zap" + "go.uber.org/zap/zapcore" "google.golang.org/protobuf/proto" "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor" "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/config" + "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/telemetry" ) var _ clientTypes.Logger = testLogger{} @@ -1354,6 +1356,78 @@ func TestSupervisorStopsAgentProcessWithEmptyConfigMap(t *testing.T) { } +type LogEntry struct { + Level string `json:"level"` +} + +func TestSupervisorInfoLoggingLevel(t *testing.T) { + storageDir := t.TempDir() + remoteCfgFilePath := filepath.Join(storageDir, "last_recv_remote_config.dat") + + collectorCfg, hash, _, _ := createSimplePipelineCollectorConf(t) + remoteCfgProto := &protobufs.AgentRemoteConfig{ + Config: &protobufs.AgentConfigMap{ + ConfigMap: map[string]*protobufs.AgentConfigFile{ + "": {Body: collectorCfg.Bytes()}, + }, + }, + ConfigHash: hash, + } + marshalledRemoteCfg, err := proto.Marshal(remoteCfgProto) + require.NoError(t, err) + require.NoError(t, os.WriteFile(remoteCfgFilePath, marshalledRemoteCfg, 0600)) + + connected := atomic.Bool{} + server := newUnstartedOpAMPServer(t, defaultConnectingHandler, server.ConnectionCallbacksStruct{ + OnConnectedFunc: func(ctx context.Context, conn types.Connection) { + connected.Store(true) + }, + }) + defer server.shutdown() + + supervisorLogFilePath := filepath.Join(storageDir, "supervisor_log.log") + cfgFile := getSupervisorConfig(t, "logging", map[string]string{ + "url": server.addr, + "storage_dir": storageDir, + "log_level": "0", + "log_file": supervisorLogFilePath, + }) + + cfg, err := config.Load(cfgFile.Name()) + require.NoError(t, err) + logger, err := telemetry.NewLogger(cfg.Telemetry.Logs) + require.NoError(t, err) + + s, err := supervisor.NewSupervisor(logger, cfg) + require.NoError(t, err) + require.Nil(t, s.Start()) + + // Start the server and wait for the supervisor to connect + server.start() + waitForSupervisorConnection(server.supervisorConnected, true) + require.True(t, connected.Load(), "Supervisor failed to connect") + + s.Shutdown() + + // Read from log file checking for Info level logs + logFile, err := os.Open(supervisorLogFilePath) + require.NoError(t, err) + defer logFile.Close() + + scanner := bufio.NewScanner(logFile) + + for scanner.Scan() { + line := scanner.Bytes() + var log LogEntry + err := json.Unmarshal(line, &log) + require.NoError(t, err) + + level, err := zapcore.ParseLevel(log.Level) + require.NoError(t, err) + require.GreaterOrEqual(t, level, zapcore.InfoLevel) + } +} + func findRandomPort() (int, error) { l, err := net.Listen("tcp", "localhost:0") diff --git a/cmd/opampsupervisor/supervisor/config/config.go b/cmd/opampsupervisor/supervisor/config/config.go index 3a7f384db3f5..16d584abeb37 100644 --- a/cmd/opampsupervisor/supervisor/config/config.go +++ b/cmd/opampsupervisor/supervisor/config/config.go @@ -237,25 +237,3 @@ func DefaultSupervisor() Supervisor { }, } } - -func LoadConfig(configFile string) (Supervisor, error) { - if configFile == "" { - return Supervisor{}, errors.New("path to config file cannot be empty") - } - - k := koanf.New("::") - if err := k.Load(file.Provider(configFile), yaml.Parser()); err != nil { - return Supervisor{}, err - } - - decodeConf := koanf.UnmarshalConf{ - Tag: "mapstructure", - } - - cfg := DefaultSupervisor() - if err := k.UnmarshalWithConf("", &cfg, decodeConf); err != nil { - return Supervisor{}, fmt.Errorf("cannot parse %v: %w", configFile, err) - } - - return cfg, nil -} diff --git a/cmd/opampsupervisor/supervisor/supervisor.go b/cmd/opampsupervisor/supervisor/supervisor.go index 0c5f85abc6fb..8d683e5b09b4 100644 --- a/cmd/opampsupervisor/supervisor/supervisor.go +++ b/cmd/opampsupervisor/supervisor/supervisor.go @@ -150,7 +150,6 @@ type Supervisor struct { func NewSupervisor(logger *zap.Logger, cfg config.Supervisor) (*Supervisor, error) { s := &Supervisor{ - config: cfg, logger: logger, pidProvider: defaultPIDProvider{}, hasNewConfig: make(chan struct{}, 1), @@ -200,7 +199,7 @@ func (s *Supervisor) Start() error { s.agentHealthCheckEndpoint = fmt.Sprintf("localhost:%d", healthCheckPort) - s.logger.Debug("Supervisor starting", + s.logger.Info("Supervisor starting", zap.String("id", s.persistentState.InstanceID.String())) err = s.loadAndWriteInitialMergedConfig() diff --git a/cmd/opampsupervisor/testdata/supervisor/supervisor_logging.yaml b/cmd/opampsupervisor/testdata/supervisor/supervisor_logging.yaml new file mode 100644 index 000000000000..dc6153dfb86c --- /dev/null +++ b/cmd/opampsupervisor/testdata/supervisor/supervisor_logging.yaml @@ -0,0 +1,21 @@ +server: + endpoint: ws://{{.url}}/v1/opamp + +capabilities: + reports_effective_config: true + reports_own_metrics: true + reports_health: true + accepts_remote_config: true + reports_remote_config: true + accepts_restart_command: true + +storage: + directory: '{{.storage_dir}}' + +agent: + executable: ../../bin/otelcontribcol_{{.goos}}_{{.goarch}}{{.extension}} + +telemetry: + logs: + level: {{.log_level}} # info level logs + output_paths: ['{{.log_file}}'] From b913e9abcd3b1aeb9848dc3a53198a6f60be12d0 Mon Sep 17 00:00:00 2001 From: Dakota Paasman <122491662+dpaasman00@users.noreply.github.com> Date: Thu, 3 Oct 2024 10:38:13 -0400 Subject: [PATCH 8/9] add comment for new issue --- cmd/opampsupervisor/main.go | 2 -- cmd/opampsupervisor/supervisor/config/config.go | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cmd/opampsupervisor/main.go b/cmd/opampsupervisor/main.go index a998ca0e8f0a..e2f5db5e130f 100644 --- a/cmd/opampsupervisor/main.go +++ b/cmd/opampsupervisor/main.go @@ -18,13 +18,11 @@ func main() { configFlag := flag.String("config", "", "Path to a supervisor configuration file") flag.Parse() - // load & validate config cfg, err := config.Load(*configFlag) if err != nil { log.Fatal("failed to load config: %w", err) } - // create logger logger, err := telemetry.NewLogger(cfg.Telemetry.Logs) if err != nil { log.Fatal("failed to create logger: %w", err) diff --git a/cmd/opampsupervisor/supervisor/config/config.go b/cmd/opampsupervisor/supervisor/config/config.go index 16d584abeb37..7d1275a37a30 100644 --- a/cmd/opampsupervisor/supervisor/config/config.go +++ b/cmd/opampsupervisor/supervisor/config/config.go @@ -188,7 +188,8 @@ type AgentDescription struct { } type Telemetry struct { - // TODO: flesh out other configurable telemetry options + // TODO: Add more telemetry options + // Issue here: https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/35582 Logs Logs `mapstructure:"logs"` } From d95a30ec1623e44edd836857318b6f064f6f125e Mon Sep 17 00:00:00 2001 From: Dakota Paasman <122491662+dpaasman00@users.noreply.github.com> Date: Thu, 3 Oct 2024 10:54:40 -0400 Subject: [PATCH 9/9] add e2e build tag back --- cmd/opampsupervisor/e2e_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cmd/opampsupervisor/e2e_test.go b/cmd/opampsupervisor/e2e_test.go index d8477815c07a..6563888c22fe 100644 --- a/cmd/opampsupervisor/e2e_test.go +++ b/cmd/opampsupervisor/e2e_test.go @@ -1,6 +1,8 @@ // Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 +//go:build e2e + package main import ( @@ -1415,8 +1417,12 @@ func TestSupervisorInfoLoggingLevel(t *testing.T) { defer logFile.Close() scanner := bufio.NewScanner(logFile) - + check := false for scanner.Scan() { + if !check { + check = true + } + line := scanner.Bytes() var log LogEntry err := json.Unmarshal(line, &log) @@ -1426,6 +1432,8 @@ func TestSupervisorInfoLoggingLevel(t *testing.T) { require.NoError(t, err) require.GreaterOrEqual(t, level, zapcore.InfoLevel) } + // verify at least 1 log was read + require.True(t, check) } func findRandomPort() (int, error) {