Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cmd/opampsupervisor] Add configurable supervisor logging #35468

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .chloggen/configurable-supervisor-logging.yaml
Original file line number Diff line number Diff line change
@@ -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: []
82 changes: 82 additions & 0 deletions cmd/opampsupervisor/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
package main

import (
"bufio"
"bytes"
"context"
"crypto/sha256"
"encoding/json"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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")

Expand Down
19 changes: 9 additions & 10 deletions cmd/opampsupervisor/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
19 changes: 19 additions & 0 deletions cmd/opampsupervisor/supervisor/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"},
},
},
}
}
3 changes: 1 addition & 2 deletions cmd/opampsupervisor/supervisor/supervisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down
23 changes: 23 additions & 0 deletions cmd/opampsupervisor/supervisor/telemetry/logger.go
Original file line number Diff line number Diff line change
@@ -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
}
21 changes: 21 additions & 0 deletions cmd/opampsupervisor/testdata/supervisor/supervisor_logging.yaml
Original file line number Diff line number Diff line change
@@ -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}}']
Loading