Skip to content

Commit 691e5da

Browse files
codebotenTylerHelmuthatoulme
authored
[cmd/opampsupervisor] log stderr using Error (#40491)
#### Description 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 <[email protected]> Co-authored-by: Tyler Helmuth <[email protected]> Co-authored-by: Antoine Toulme <[email protected]>
1 parent 119bb59 commit 691e5da

File tree

10 files changed

+66
-7
lines changed

10 files changed

+66
-7
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: breaking
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: opampsupervisor
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Update passthrough logging to use Error for stderr pipe.
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [40491]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

cmd/opampsupervisor/supervisor/commander/commander.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func (c *Commander) startWithPassthroughLogging() error {
157157
scanner := bufio.NewScanner(stderrPipe)
158158
for scanner.Scan() {
159159
line := scanner.Text()
160-
colLogger.Info(line)
160+
colLogger.Error(line)
161161
}
162162
if err := scanner.Err(); err != nil {
163163
c.logger.Error("Error reading agent stderr: %w", zap.Error(err))

cmd/opampsupervisor/supervisor/config/config.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,9 @@ type Telemetry struct {
237237
}
238238

239239
type Logs struct {
240-
Level zapcore.Level `mapstructure:"level"`
241-
OutputPaths []string `mapstructure:"output_paths"`
240+
Level zapcore.Level `mapstructure:"level"`
241+
ErrorOutputPaths []string `mapstructure:"error_output_paths"`
242+
OutputPaths []string `mapstructure:"output_paths"`
242243
// Processors allow configuration of log record processors to emit logs to
243244
// any number of supported backends.
244245
Processors []config.LogRecordProcessor `mapstructure:"processors,omitempty"`
@@ -288,8 +289,9 @@ func DefaultSupervisor() Supervisor {
288289
},
289290
Telemetry: Telemetry{
290291
Logs: Logs{
291-
Level: zapcore.InfoLevel,
292-
OutputPaths: []string{"stderr"},
292+
Level: zapcore.InfoLevel,
293+
OutputPaths: []string{"stdout"},
294+
ErrorOutputPaths: []string{"stderr"},
293295
},
294296
},
295297
}

cmd/opampsupervisor/supervisor/config/config_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,6 +541,7 @@ agent:
541541
telemetry:
542542
logs:
543543
level: warn
544+
error_output_paths: ["stderr"]
544545
output_paths: ["stdout"]
545546
`
546547
config = fmt.Sprintf(config, filepath.Join(tmpDir, "storage"), executablePath)
@@ -584,8 +585,9 @@ telemetry:
584585
},
585586
Telemetry: Telemetry{
586587
Logs: Logs{
587-
Level: zapcore.WarnLevel,
588-
OutputPaths: []string{"stdout"},
588+
Level: zapcore.WarnLevel,
589+
OutputPaths: []string{"stdout"},
590+
ErrorOutputPaths: []string{"stderr"},
589591
},
590592
},
591593
}

cmd/opampsupervisor/supervisor/supervisor_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,10 @@ service:
534534
telemetry:
535535
logs:
536536
encoding: json
537+
error_output_paths:
538+
- stderr
539+
output_paths:
540+
- stdout
537541
resource: null
538542
`
539543

@@ -630,6 +634,10 @@ service:
630634
telemetry:
631635
logs:
632636
encoding: json
637+
error_output_paths:
638+
- stderr
639+
output_paths:
640+
- stdout
633641
resource: null
634642
`
635643

@@ -793,6 +801,10 @@ service:
793801
telemetry:
794802
logs:
795803
encoding: json
804+
error_output_paths:
805+
- stderr
806+
output_paths:
807+
- stdout
796808
resource: null
797809
`
798810

@@ -1494,6 +1506,10 @@ service:
14941506
telemetry:
14951507
logs:
14961508
encoding: json
1509+
error_output_paths:
1510+
- stderr
1511+
output_paths:
1512+
- stdout
14971513
processors:
14981514
- batch:
14991515
exporter:

cmd/opampsupervisor/supervisor/templates/extraconfig.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ service:
33
logs:
44
# Enables JSON log output for the Agent.
55
encoding: json
6+
# Ensure logs and errors are using the correct output
7+
# channel
8+
error_output_paths: ["stderr"]
9+
output_paths: ["stdout"]
610
resource:
711
# Set resource attributes suggested by the OpAMP spec.
812
# See https://github.com/open-telemetry/opamp-spec/blob/main/specification.md#agentdescription-message

cmd/opampsupervisor/supervisor/testdata/merged_effective_config.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,7 @@ service:
4444
telemetry:
4545
logs:
4646
encoding: json
47+
error_output_paths: ["stderr"]
48+
output_paths: ["stdout"]
4749
resource:
4850
service.name: otelcol

cmd/opampsupervisor/supervisor/testdata/merged_local_config.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,5 +33,7 @@ service:
3333
telemetry:
3434
logs:
3535
encoding: json
36+
error_output_paths: ["stderr"]
37+
output_paths: ["stdout"]
3638
resource:
3739
service.name: otelcol

cmd/opampsupervisor/testdata/collector/effective_config.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,7 @@ service:
3737
telemetry:
3838
logs:
3939
encoding: json
40+
error_output_paths: ["stderr"]
41+
output_paths: ["stdout"]
4042
resource:
4143
service.name: otelcol

cmd/opampsupervisor/testdata/collector/effective_config_without_extensions.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,7 @@ service:
3535
telemetry:
3636
logs:
3737
encoding: json
38+
error_output_paths: ["stderr"]
39+
output_paths: ["stdout"]
3840
resource:
3941
service.name: otelcol

0 commit comments

Comments
 (0)