From 6b3d00980b283f61fc22a79e519fbd2e24ee32a5 Mon Sep 17 00:00:00 2001 From: alex boten <223565+codeboten@users.noreply.github.com> Date: Wed, 4 Jun 2025 10:36:25 -0700 Subject: [PATCH 1/4] [cmd/opampsupervisor] log stderr using Error This seems like a bug, but it is also a breaking change. The logger used when logging errors from the collector was always logging messages with a severity of Info, even for messages from the stderr pipe. Signed-off-by: alex boten <223565+codeboten@users.noreply.github.com> --- cmd/opampsupervisor/supervisor/commander/commander.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/opampsupervisor/supervisor/commander/commander.go b/cmd/opampsupervisor/supervisor/commander/commander.go index 239f24dcd8a4..c9fce7b59139 100644 --- a/cmd/opampsupervisor/supervisor/commander/commander.go +++ b/cmd/opampsupervisor/supervisor/commander/commander.go @@ -157,7 +157,7 @@ func (c *Commander) startWithPassthroughLogging() error { scanner := bufio.NewScanner(stderrPipe) for scanner.Scan() { line := scanner.Text() - colLogger.Info(line) + colLogger.Error(line) } if err := scanner.Err(); err != nil { c.logger.Error("Error reading agent stderr: %w", zap.Error(err)) From 5f90c50f470c983f876001deeed98131fb1c38cc Mon Sep 17 00:00:00 2001 From: alex boten <223565+codeboten@users.noreply.github.com> Date: Wed, 4 Jun 2025 10:39:27 -0700 Subject: [PATCH 2/4] changelog Signed-off-by: alex boten <223565+codeboten@users.noreply.github.com> --- .chloggen/codeboten_log-stderr-to-error.yaml | 27 ++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .chloggen/codeboten_log-stderr-to-error.yaml diff --git a/.chloggen/codeboten_log-stderr-to-error.yaml b/.chloggen/codeboten_log-stderr-to-error.yaml new file mode 100644 index 000000000000..e0d766f1c309 --- /dev/null +++ b/.chloggen/codeboten_log-stderr-to-error.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: bug_fix + +# 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: Update passthrough logging to use Error for stderr pipe. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [40491] + +# (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: [] From 630e29452c403488eb8eb6f9819c1dbc2bd3a496 Mon Sep 17 00:00:00 2001 From: Alex Boten <223565+codeboten@users.noreply.github.com> Date: Wed, 4 Jun 2025 11:31:35 -0700 Subject: [PATCH 3/4] Update .chloggen/codeboten_log-stderr-to-error.yaml Co-authored-by: Antoine Toulme --- .chloggen/codeboten_log-stderr-to-error.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/codeboten_log-stderr-to-error.yaml b/.chloggen/codeboten_log-stderr-to-error.yaml index e0d766f1c309..82168e8b94b2 100644 --- a/.chloggen/codeboten_log-stderr-to-error.yaml +++ b/.chloggen/codeboten_log-stderr-to-error.yaml @@ -1,7 +1,7 @@ # Use this changelog template to create an entry for release notes. # One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' -change_type: bug_fix +change_type: breaking # The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) component: opampsupervisor From b7953322a0e5f67bb440dbbe3e5b69565f035528 Mon Sep 17 00:00:00 2001 From: alex boten <223565+codeboten@users.noreply.github.com> Date: Thu, 5 Jun 2025 12:22:59 -0700 Subject: [PATCH 4/4] set error_output_paths and output_paths defaults Signed-off-by: alex boten <223565+codeboten@users.noreply.github.com> --- cmd/opampsupervisor/supervisor/config/config.go | 10 ++++++---- .../supervisor/config/config_test.go | 6 ++++-- .../supervisor/supervisor_test.go | 16 ++++++++++++++++ .../supervisor/templates/extraconfig.yaml | 4 ++++ .../testdata/merged_effective_config.yaml | 2 ++ .../supervisor/testdata/merged_local_config.yaml | 2 ++ .../testdata/collector/effective_config.yaml | 2 ++ .../effective_config_without_extensions.yaml | 2 ++ 8 files changed, 38 insertions(+), 6 deletions(-) diff --git a/cmd/opampsupervisor/supervisor/config/config.go b/cmd/opampsupervisor/supervisor/config/config.go index 7305554111bf..d88f1af574cf 100644 --- a/cmd/opampsupervisor/supervisor/config/config.go +++ b/cmd/opampsupervisor/supervisor/config/config.go @@ -237,8 +237,9 @@ type Telemetry struct { } type Logs struct { - Level zapcore.Level `mapstructure:"level"` - OutputPaths []string `mapstructure:"output_paths"` + Level zapcore.Level `mapstructure:"level"` + ErrorOutputPaths []string `mapstructure:"error_output_paths"` + OutputPaths []string `mapstructure:"output_paths"` // Processors allow configuration of log record processors to emit logs to // any number of supported backends. Processors []config.LogRecordProcessor `mapstructure:"processors,omitempty"` @@ -288,8 +289,9 @@ func DefaultSupervisor() Supervisor { }, Telemetry: Telemetry{ Logs: Logs{ - Level: zapcore.InfoLevel, - OutputPaths: []string{"stderr"}, + Level: zapcore.InfoLevel, + OutputPaths: []string{"stdout"}, + ErrorOutputPaths: []string{"stderr"}, }, }, } diff --git a/cmd/opampsupervisor/supervisor/config/config_test.go b/cmd/opampsupervisor/supervisor/config/config_test.go index 5c1ee4e531ab..718f0d1f25f6 100644 --- a/cmd/opampsupervisor/supervisor/config/config_test.go +++ b/cmd/opampsupervisor/supervisor/config/config_test.go @@ -541,6 +541,7 @@ agent: telemetry: logs: level: warn + error_output_paths: ["stderr"] output_paths: ["stdout"] ` config = fmt.Sprintf(config, filepath.Join(tmpDir, "storage"), executablePath) @@ -584,8 +585,9 @@ telemetry: }, Telemetry: Telemetry{ Logs: Logs{ - Level: zapcore.WarnLevel, - OutputPaths: []string{"stdout"}, + Level: zapcore.WarnLevel, + OutputPaths: []string{"stdout"}, + ErrorOutputPaths: []string{"stderr"}, }, }, } diff --git a/cmd/opampsupervisor/supervisor/supervisor_test.go b/cmd/opampsupervisor/supervisor/supervisor_test.go index feb09065f278..e243ed759965 100644 --- a/cmd/opampsupervisor/supervisor/supervisor_test.go +++ b/cmd/opampsupervisor/supervisor/supervisor_test.go @@ -534,6 +534,10 @@ service: telemetry: logs: encoding: json + error_output_paths: + - stderr + output_paths: + - stdout resource: null ` @@ -630,6 +634,10 @@ service: telemetry: logs: encoding: json + error_output_paths: + - stderr + output_paths: + - stdout resource: null ` @@ -793,6 +801,10 @@ service: telemetry: logs: encoding: json + error_output_paths: + - stderr + output_paths: + - stdout resource: null ` @@ -1494,6 +1506,10 @@ service: telemetry: logs: encoding: json + error_output_paths: + - stderr + output_paths: + - stdout processors: - batch: exporter: diff --git a/cmd/opampsupervisor/supervisor/templates/extraconfig.yaml b/cmd/opampsupervisor/supervisor/templates/extraconfig.yaml index 78ed890755d1..6cbf2101d7cc 100644 --- a/cmd/opampsupervisor/supervisor/templates/extraconfig.yaml +++ b/cmd/opampsupervisor/supervisor/templates/extraconfig.yaml @@ -3,6 +3,10 @@ service: logs: # Enables JSON log output for the Agent. encoding: json + # Ensure logs and errors are using the correct output + # channel + error_output_paths: ["stderr"] + output_paths: ["stdout"] resource: # Set resource attributes suggested by the OpAMP spec. # See https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescription-message diff --git a/cmd/opampsupervisor/supervisor/testdata/merged_effective_config.yaml b/cmd/opampsupervisor/supervisor/testdata/merged_effective_config.yaml index c33780b6677e..6b2a5b02177c 100644 --- a/cmd/opampsupervisor/supervisor/testdata/merged_effective_config.yaml +++ b/cmd/opampsupervisor/supervisor/testdata/merged_effective_config.yaml @@ -44,5 +44,7 @@ service: telemetry: logs: encoding: json + error_output_paths: ["stderr"] + output_paths: ["stdout"] resource: service.name: otelcol diff --git a/cmd/opampsupervisor/supervisor/testdata/merged_local_config.yaml b/cmd/opampsupervisor/supervisor/testdata/merged_local_config.yaml index 8adf57ee1350..71fa3eddfe80 100644 --- a/cmd/opampsupervisor/supervisor/testdata/merged_local_config.yaml +++ b/cmd/opampsupervisor/supervisor/testdata/merged_local_config.yaml @@ -33,5 +33,7 @@ service: telemetry: logs: encoding: json + error_output_paths: ["stderr"] + output_paths: ["stdout"] resource: service.name: otelcol diff --git a/cmd/opampsupervisor/testdata/collector/effective_config.yaml b/cmd/opampsupervisor/testdata/collector/effective_config.yaml index 8c8bc21eb638..40d7b073d69c 100644 --- a/cmd/opampsupervisor/testdata/collector/effective_config.yaml +++ b/cmd/opampsupervisor/testdata/collector/effective_config.yaml @@ -37,5 +37,7 @@ service: telemetry: logs: encoding: json + error_output_paths: ["stderr"] + output_paths: ["stdout"] resource: service.name: otelcol diff --git a/cmd/opampsupervisor/testdata/collector/effective_config_without_extensions.yaml b/cmd/opampsupervisor/testdata/collector/effective_config_without_extensions.yaml index 345b60088afd..199f4619bfe7 100644 --- a/cmd/opampsupervisor/testdata/collector/effective_config_without_extensions.yaml +++ b/cmd/opampsupervisor/testdata/collector/effective_config_without_extensions.yaml @@ -35,5 +35,7 @@ service: telemetry: logs: encoding: json + error_output_paths: ["stderr"] + output_paths: ["stdout"] resource: service.name: otelcol