(bugfix) log level does not change when standalone agent is reloaded or when otel runtime is used

changelog/fragments/1766556100-reload-log-level.yaml

@@ -0,0 +1,45 @@
+# REQUIRED
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: bug-fix
+
+# REQUIRED for all kinds
+# Change summary; a 80ish characters long description of the change.
+summary: Fix reloading agent.logging.level for standalone Elastic Agent 
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# description:
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# impact:
+
+# REQUIRED for breaking-change, deprecation, known-issue
+# action:
+
+# REQUIRED for all kinds
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: elastic-agent
+
+# AUTOMATED
+# OPTIONAL to manually add other PR URLs
+# PR URL: A link the PR that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+# pr: https://github.com/owner/repo/1234
+
+# AUTOMATED
+# OPTIONAL to manually add other issue URLs
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+# issue: https://github.com/owner/repo/1234

internal/pkg/agent/application/application.go

@@ -270,7 +270,8 @@ func New(
 
 	otelManager, err := otelmanager.NewOTelManager(
 		log.Named("otel_manager"),
-		logLevel, baseLogger,
+		logLevel,
+		baseLogger,
 		agentInfo,
 		cfg.Settings.Collector,
 		monitor.ComponentMonitoringConfig,

internal/pkg/agent/application/coordinator/coordinator.go

@@ -162,7 +162,7 @@ type OTelManager interface {
 	Runner
 
 	// Update updates the current plain configuration for the otel collector and components.
-	Update(*confmap.Conf, *monitoringCfg.MonitoringConfig, []component.Component)
+	Update(*confmap.Conf, *monitoringCfg.MonitoringConfig, logp.Level, []component.Component)
 
 	// WatchCollector returns a channel to watch for collector status updates.
 	WatchCollector() <-chan *status.AggregateStatus
@@ -1674,10 +1674,20 @@ func (c *Coordinator) processConfigAgent(ctx context.Context, cfg *config.Config
 	}
 	c.currentCfg = currentCfg
 
-	if c.vars != nil {
-		return c.refreshComponentModel(ctx)
+	// check if log level has changed for standalone elastic-agent
+	// we'd have to update both the periodic and once config watchers and refactor initialization in application.go to do otherwise.
+	if c.agentInfo.IsStandalone() {
+		ll := currentCfg.Settings.LoggingConfig.Level
+		if ll != c.state.LogLevel {
+			// set log level for the coordinator
+			c.setLogLevel(ll)
+			// set global log level
+			logger.SetLevel(ll)
+			c.logger.Infof("log level changed to %s", ll.String())
+		}
 	}
-	return nil
+
+	return c.refreshComponentModel(ctx)
 }
 
 // Generate the AST for a new incoming configuration and, if successful,
@@ -1802,10 +1812,13 @@ func (c *Coordinator) processVars(ctx context.Context, vars []*transpiler.Vars)
 
 // Called on the main Coordinator goroutine.
 func (c *Coordinator) processLogLevel(ctx context.Context, ll logp.Level) {
-	c.setLogLevel(ll)
-	err := c.refreshComponentModel(ctx)
-	if err != nil {
-		c.logger.Errorf("updating log level: %s", err.Error())
+	// do not refresh component model if log level did not change
+	if c.state.LogLevel != ll {
+		c.setLogLevel(ll)
+		err := c.refreshComponentModel(ctx)
+		if err != nil {
+			c.logger.Errorf("updating log level: %s", err.Error())
+		}
 	}
 }
 
@@ -1872,7 +1885,7 @@ func (c *Coordinator) updateManagersWithConfig(model *component.Model) {
 		}
 		c.logger.With("component_ids", componentIDs).Info("Using OpenTelemetry collector runtime.")
 	}
-	c.otelMgr.Update(c.otelCfg, c.currentCfg.Settings.MonitoringConfig, otelModel.Components)
+	c.otelMgr.Update(c.otelCfg, c.currentCfg.Settings.MonitoringConfig, c.state.LogLevel, otelModel.Components)
 }
 
 // splitModelBetweenManager splits the model components between the runtime manager and the otel manager.

internal/pkg/agent/application/coordinator/coordinator_test.go

@@ -1457,7 +1457,7 @@ func (f *fakeOTelManager) Errors() <-chan error {
 	return f.errChan
 }
 
-func (f *fakeOTelManager) Update(cfg *confmap.Conf, monitoring *monitoringCfg.MonitoringConfig, components []component.Component) {
+func (f *fakeOTelManager) Update(cfg *confmap.Conf, monitoring *monitoringCfg.MonitoringConfig, ll logp.Level, components []component.Component) {
 	var collectorResult, componentResult error
 	if f.updateCollectorCallback != nil {
 		collectorResult = f.updateCollectorCallback(cfg)

internal/pkg/agent/application/coordinator/coordinator_unit_test.go

@@ -469,7 +469,9 @@ func TestCoordinatorReportsInvalidPolicy(t *testing.T) {
 	}()
 
 	tmpDir := t.TempDir()
-	upgradeMgr, err := upgrade.NewUpgrader(log, &artifact.Config{}, nil, &info.AgentInfo{}, new(upgrade.AgentWatcherHelper), ttl.NewTTLMarkerRegistry(nil, tmpDir))
+	agentInfo, err := info.NewAgentInfo(ctx, false)
+	require.NoError(t, err)
+	upgradeMgr, err := upgrade.NewUpgrader(log, &artifact.Config{}, nil, agentInfo, new(upgrade.AgentWatcherHelper), ttl.NewTTLMarkerRegistry(nil, tmpDir))
 	require.NoError(t, err, "errored when creating a new upgrader")
 
 	// Channels have buffer length 1, so we don't have to run on multiple
@@ -502,6 +504,7 @@ func TestCoordinatorReportsInvalidPolicy(t *testing.T) {
 		ast:                emptyAST(t),
 		componentPIDTicker: time.NewTicker(time.Second * 30),
 		secretMarkerFunc:   testSecretMarkerFunc,
+		agentInfo:          agentInfo,
 	}
 
 	// Send an invalid config update and confirm that Coordinator reports
@@ -594,6 +597,9 @@ func TestCoordinatorReportsComponentModelError(t *testing.T) {
 	defer cancel()
 	logger := logp.NewLogger("testing")
 
+	agentInfo, err := info.NewAgentInfo(t.Context(), false)
+	require.NoError(t, err)
+
 	// Channels have buffer length 1 so we don't have to run on multiple
 	// goroutines.
 	stateChan := make(chan State, 1)
@@ -619,6 +625,7 @@ func TestCoordinatorReportsComponentModelError(t *testing.T) {
 		ast:                emptyAST(t),
 		componentPIDTicker: time.NewTicker(time.Second * 30),
 		secretMarkerFunc:   testSecretMarkerFunc,
+		agentInfo:          agentInfo,
 	}
 
 	// This configuration produces a valid AST but its EQL condition is

internal/pkg/agent/application/periodic.go

@@ -17,6 +17,7 @@ import (
 	"github.com/elastic/elastic-agent/pkg/core/logger"
 )
 
+// periodic checks for local configuration changes
 type periodic struct {
 	log      *logger.Logger
 	period   time.Duration
@@ -147,6 +148,7 @@ func newPeriodic(
 	}
 }
 
+// localConfigChange implements coordinator.ConfigChange for local file changes.
 type localConfigChange struct {
 	cfg *config.Config
 }

internal/pkg/agent/cmd/run.go

@@ -353,19 +353,17 @@ func runElasticAgent(
 			errors.M(errors.MetaKeyPath, paths.AgentConfigFile()))
 	}
 
+	// Set the initial log level (either default or from config file)
+	logger.SetLevel(logLvl)
+
 	// Ensure that the log level now matches what is configured in the agentInfo.
-	if agentInfo.LogLevel() != "" {
-		var lvl logp.Level
-		err = lvl.Unpack(agentInfo.LogLevel())
-		if err != nil {
-			l.Error(errors.New(err, "failed to parse agent information log level"))
-		} else {
-			logLvl = lvl
-			logger.SetLevel(lvl)
-		}
+	var lvl logp.Level
+	err = lvl.Unpack(agentInfo.LogLevel())
+	if err != nil {
+		l.Error(errors.New(err, "failed to parse agent information log level"))
 	} else {
-		// Set the initial log level (either default or from config file)
-		logger.SetLevel(logLvl)
+		logLvl = lvl
+		logger.SetLevel(lvl)
 	}
 
 	// initiate agent watcher

internal/pkg/otel/manager/execution.go

@@ -25,7 +25,7 @@ type collectorExecution interface {
 	//   - errCh: Process exit errors are sent to the errCh channel
 	//   - statusCh: Collector's status updates are sent to statusCh channel.
 	//   - forceFetchStatusCh: Channel that is used to trigger a forced status update.
-	startCollector(ctx context.Context, baseLogger *logger.Logger, logger *logger.Logger, cfg *confmap.Conf, errCh chan error, statusCh chan *status.AggregateStatus, forceFetchStatusCh chan struct{}) (collectorHandle, error)
+	startCollector(ctx context.Context, logLevel string, baseLogger *logger.Logger, logger *logger.Logger, cfg *confmap.Conf, errCh chan error, statusCh chan *status.AggregateStatus, forceFetchStatusCh chan struct{}) (collectorHandle, error)
 }
 
 type collectorHandle interface {

internal/pkg/otel/manager/execution_subprocess.go

@@ -40,7 +40,7 @@ const (
 
 // newSubprocessExecution creates a new execution which runs the otel collector in a subprocess. A metricsPort or
 // healthCheckPort of 0 will result in a random port being used.
-func newSubprocessExecution(logLevel logp.Level, collectorPath string, uuid string, metricsPort int, healthCheckPort int) (*subprocessExecution, error) {
+func newSubprocessExecution(collectorPath string, uuid string, metricsPort int, healthCheckPort int) (*subprocessExecution, error) {
 	componentType, err := component.NewType(healthCheckExtensionName)
 	if err != nil {
 		return nil, fmt.Errorf("cannot create component type: %w", err)
@@ -51,21 +51,19 @@ func newSubprocessExecution(logLevel logp.Level, collectorPath string, uuid stri
 		collectorPath: collectorPath,
 		collectorArgs: []string{
 			fmt.Sprintf("--%s", OtelSetSupervisedFlagName),
-			fmt.Sprintf("--%s=%s", OtelSupervisedLoggingLevelFlagName, logLevel.String()),
 			fmt.Sprintf("--%s=%s", OtelSupervisedMonitoringURLFlagName, monitoring.EDOTMonitoringEndpoint()),
 		},
-		logLevel:                 logLevel,
 		healthCheckExtensionID:   healthCheckExtensionID,
 		collectorMetricsPort:     metricsPort,
 		collectorHealthCheckPort: healthCheckPort,
 		reportErrFn:              reportErr,
 	}, nil
 }
 
+// subprocessExecution implements collectorExecution by running the collector in a subprocess.
 type subprocessExecution struct {
 	collectorPath            string
 	collectorArgs            []string
-	logLevel                 logp.Level
 	healthCheckExtensionID   string
 	collectorMetricsPort     int
 	collectorHealthCheckPort int
@@ -74,7 +72,22 @@ type subprocessExecution struct {
 
 // startCollector starts a supervised collector and monitors its health. Process exit errors are sent to the
 // processErrCh channel. Other run errors, such as not able to connect to the health endpoint, are sent to the runErrCh channel.
-func (r *subprocessExecution) startCollector(ctx context.Context, baseLogger *logger.Logger, logger *logger.Logger, cfg *confmap.Conf, processErrCh chan error, statusCh chan *status.AggregateStatus, forceFetchStatusCh chan struct{}) (collectorHandle, error) {
+func (r *subprocessExecution) startCollector(
+	ctx context.Context,
+	logLevel string,
+	baseLogger *logger.Logger,
+	logger *logger.Logger,
+	cfg *confmap.Conf,
+	processErrCh chan error,
+	statusCh chan *status.AggregateStatus,
+	forceFetchStatusCh chan struct{},
+) (collectorHandle, error) {
+	var lvl logp.Level
+	err := lvl.Unpack(logLevel)
+	if err != nil {
+		return nil, fmt.Errorf("failed to unpack the log level '%s': %w", logLevel, err)
+	}
+
 	if cfg == nil {
 		// configuration is required
 		return nil, errors.New("no configuration provided")
@@ -106,17 +119,21 @@ func (r *subprocessExecution) startCollector(ctx context.Context, baseLogger *lo
 	}
 
 	stdOutLast := newZapLast(baseLogger.Core())
-	stdOut := runtimeLogger.NewLogWriterWithDefaults(stdOutLast, zapcore.Level(r.logLevel))
+	stdOut := runtimeLogger.NewLogWriterWithDefaults(stdOutLast, zapcore.Level(lvl))
 	// info level for stdErr because by default collector writes to stderr
 	stdErrLast := newZapLast(baseLogger.Core())
-	stdErr := runtimeLogger.NewLogWriterWithDefaults(stdErrLast, zapcore.Level(r.logLevel))
+	stdErr := runtimeLogger.NewLogWriterWithDefaults(stdErrLast, zapcore.Level(lvl))
 
 	procCtx, procCtxCancel := context.WithCancel(ctx)
 	env := os.Environ()
 	// Set the environment variable for the collector metrics port. See comment at the constant definition for more information.
 	env = append(env, fmt.Sprintf("%s=%d", componentmonitoring.OtelCollectorMetricsPortEnvVarName, collectorMetricsPort))
+
+	// set collector args
+	collectorArgs := append(r.collectorArgs, fmt.Sprintf("--%s=%s", OtelSupervisedLoggingLevelFlagName, lvl))
+
 	processInfo, err := process.Start(r.collectorPath,
-		process.WithArgs(r.collectorArgs),
+		process.WithArgs(collectorArgs),
 		process.WithEnv(env),
 		process.WithCmdOptions(func(c *exec.Cmd) error {
 			c.Stdin = bytes.NewReader(confBytes)