From 3873c2d531293cf1a9c7ad0a906f08079ba8d71e 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] create logger & load cfg in main, fix ci, add chglog --- .../configurable-supervisor-logging.yaml | 27 +++++++++++++++++++ cmd/opampsupervisor/e2e_test.go | 4 ++- cmd/opampsupervisor/main.go | 14 +++++++++- .../supervisor/config/config.go | 25 +++++++++++++++++ cmd/opampsupervisor/supervisor/supervisor.go | 15 +++-------- .../supervisor/telemetry/logger.go | 3 +++ 6 files changed, 74 insertions(+), 14 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/e2e_test.go b/cmd/opampsupervisor/e2e_test.go index 9316e8ffde8a..9d6c6412cf78 100644 --- a/cmd/opampsupervisor/e2e_test.go +++ b/cmd/opampsupervisor/e2e_test.go @@ -154,7 +154,9 @@ func newUnstartedOpAMPServer(t *testing.T, connectingCallback onConnectingFuncFa func newSupervisor(t *testing.T, configType string, extraConfigData map[string]string) *supervisor.Supervisor { cfgFile := getSupervisorConfig(t, configType, extraConfigData) - s, err := supervisor.NewSupervisor(zap.NewNop(), cfgFile.Name()) + cfg, err := config.LoadConfig(cfgFile.Name()) + require.NoError(t, err) + s, err := supervisor.NewSupervisor(zap.NewNop(), cfg) require.NoError(t, err) return s diff --git a/cmd/opampsupervisor/main.go b/cmd/opampsupervisor/main.go index 83d9dd3e2648..b6db4ad2b409 100644 --- a/cmd/opampsupervisor/main.go +++ b/cmd/opampsupervisor/main.go @@ -10,13 +10,25 @@ 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() - supervisor, err := supervisor.NewSupervisor(*configFlag) + cfg, err := config.LoadConfig(*configFlag) + if err != nil { + log.Fatal("failed to load config: %w", err) + } + + logger, err := telemetry.NewLogger(cfg.Telemetry.Logs) + if err != nil { + log.Fatal("failed to create logger: %w", err) + } + + supervisor, err := supervisor.NewSupervisor(logger, cfg) if err != nil { log.Fatal("failed to create new supervisor: %w", err) } diff --git a/cmd/opampsupervisor/supervisor/config/config.go b/cmd/opampsupervisor/supervisor/config/config.go index fa419ef1c6f5..cbe9790be482 100644 --- a/cmd/opampsupervisor/supervisor/config/config.go +++ b/cmd/opampsupervisor/supervisor/config/config.go @@ -13,6 +13,9 @@ import ( "runtime" "time" + "github.com/knadh/koanf/parsers/yaml" + "github.com/knadh/koanf/providers/file" + "github.com/knadh/koanf/v2" "github.com/open-telemetry/opamp-go/protobufs" "go.opentelemetry.io/collector/config/configtls" "go.uber.org/zap/zapcore" @@ -201,3 +204,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 372f29bab94f..e464eb0b31b8 100644 --- a/cmd/opampsupervisor/supervisor/supervisor.go +++ b/cmd/opampsupervisor/supervisor/supervisor.go @@ -40,7 +40,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 ( @@ -137,8 +136,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{}, @@ -153,10 +154,6 @@ func NewSupervisor(configFile string) (*Supervisor, error) { return nil, err } - 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) } @@ -165,12 +162,6 @@ func NewSupervisor(configFile string) (*Supervisor, error) { 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 (