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 4ec5034f785e..6563888c22fe 100644 --- a/cmd/opampsupervisor/e2e_test.go +++ b/cmd/opampsupervisor/e2e_test.go @@ -6,9 +6,11 @@ package main import ( + "bufio" "bytes" "context" "crypto/sha256" + "encoding/json" "errors" "fmt" "io" @@ -40,10 +42,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 +1358,84 @@ 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) + check := false + for scanner.Scan() { + if !check { + check = true + } + + 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) + } + // verify at least 1 log was read + require.True(t, check) +} + func findRandomPort() (int, error) { l, err := net.Listen("tcp", "localhost:0") diff --git a/cmd/opampsupervisor/main.go b/cmd/opampsupervisor/main.go index 137d2d37c3a2..e2f5db5e130f 100644 --- a/cmd/opampsupervisor/main.go +++ b/cmd/opampsupervisor/main.go @@ -5,26 +5,27 @@ package main import ( "flag" + "log" "os" "os/signal" - "go.uber.org/zap" - "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() - cfg, err := config.Load(*configFlag) if err != nil { - logger.Error(err.Error()) - os.Exit(-1) - return + 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) @@ -36,9 +37,7 @@ func main() { 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) diff --git a/cmd/opampsupervisor/supervisor/config/config.go b/cmd/opampsupervisor/supervisor/config/config.go index a260410f9c95..7d1275a37a30 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,17 @@ type AgentDescription struct { NonIdentifyingAttributes map[string]string `mapstructure:"non_identifying_attributes"` } +type Telemetry struct { + // TODO: Add more telemetry options + // Issue here: https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/35582 + 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" @@ -217,5 +230,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/supervisor.go b/cmd/opampsupervisor/supervisor/supervisor.go index d96a1c8fbc93..8d683e5b09b4 100644 --- a/cmd/opampsupervisor/supervisor/supervisor.go +++ b/cmd/opampsupervisor/supervisor/supervisor.go @@ -168,7 +168,6 @@ func NewSupervisor(logger *zap.Logger, cfg config.Supervisor) (*Supervisor, erro if err := cfg.Validate(); err != nil { return nil, fmt.Errorf("error validating config: %w", err) } - s.config = cfg if err := os.MkdirAll(s.config.Storage.Directory, 0700); err != nil { @@ -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/supervisor/telemetry/logger.go b/cmd/opampsupervisor/supervisor/telemetry/logger.go new file mode 100644 index 000000000000..0bec527b1188 --- /dev/null +++ b/cmd/opampsupervisor/supervisor/telemetry/logger.go @@ -0,0 +1,23 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package telemetry + +import ( + "go.uber.org/zap" + + "github.com/open-telemetry/opentelemetry-collector-contrib/cmd/opampsupervisor/supervisor/config" +) + +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 +} 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}}']