Skip to content

Commit

Permalink
[cmd/opampsupervisor] Add configurable supervisor logging (open-telem…
Browse files Browse the repository at this point in the history
…etry#35468)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

Adds configurable logging for the supervisor. Lays ground work to expand
with more config options and to configure other telemetry signals. Meant
to mimic how collector telemetry/logging is configured.

**Link to tracking Issue:** <Issue number if applicable> Closes open-telemetry#35466 

**Testing:** <Describe what testing was performed and which tests were
added.>
Tested running the supervisor with default and different log levels and
outputs specified.

**Documentation:** <Describe the documentation added.>
  • Loading branch information
dpaasman00 authored and AkhigbeEromo committed Oct 9, 2024
1 parent ad8771e commit 51f4cfd
Show file tree
Hide file tree
Showing 7 changed files with 182 additions and 12 deletions.
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}}']

0 comments on commit 51f4cfd

Please sign in to comment.