From 8d2d93f01ed0e9d1a84bcdca6d4dd67bb9116dfc Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Sun, 27 Oct 2024 20:23:00 +0530 Subject: [PATCH 1/3] [cmd/opampsupervisor]: update defult output paths for supervisor logger --- cmd/opampsupervisor/main.go | 2 +- cmd/opampsupervisor/supervisor/config/config.go | 2 +- cmd/opampsupervisor/supervisor/logger.go | 4 ++-- cmd/opampsupervisor/supervisor/supervisor.go | 6 +++--- cmd/opampsupervisor/supervisor/supervisor_test.go | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd/opampsupervisor/main.go b/cmd/opampsupervisor/main.go index 533a1f54c591..17a62e2033cd 100644 --- a/cmd/opampsupervisor/main.go +++ b/cmd/opampsupervisor/main.go @@ -35,7 +35,7 @@ func runInteractive() error { return fmt.Errorf("failed to create logger: %w", err) } - supervisor, err := supervisor.NewSupervisor(logger, cfg) + supervisor, err := supervisor.NewSupervisor(logger.Named("supervisor"), cfg) if err != nil { return fmt.Errorf("failed to create supervisor: %w", err) } diff --git a/cmd/opampsupervisor/supervisor/config/config.go b/cmd/opampsupervisor/supervisor/config/config.go index 5e6049dddbd8..c314a40b900e 100644 --- a/cmd/opampsupervisor/supervisor/config/config.go +++ b/cmd/opampsupervisor/supervisor/config/config.go @@ -235,7 +235,7 @@ func DefaultSupervisor() Supervisor { Telemetry: Telemetry{ Logs: Logs{ Level: zapcore.InfoLevel, - OutputPaths: []string{"stdout", "stderr"}, + OutputPaths: []string{"stderr"}, }, }, } diff --git a/cmd/opampsupervisor/supervisor/logger.go b/cmd/opampsupervisor/supervisor/logger.go index 11811d539372..863b4aba7f5f 100644 --- a/cmd/opampsupervisor/supervisor/logger.go +++ b/cmd/opampsupervisor/supervisor/logger.go @@ -26,8 +26,8 @@ func (o *opAMPLogger) Errorf(_ context.Context, format string, v ...any) { o.l.Errorf(format, v...) } -func newLoggerFromZap(l *zap.Logger) types.Logger { +func newLoggerFromZap(l *zap.Logger, name string) types.Logger { return &opAMPLogger{ - l: l.Sugar(), + l: l.Sugar().Named(name), } } diff --git a/cmd/opampsupervisor/supervisor/supervisor.go b/cmd/opampsupervisor/supervisor/supervisor.go index 8d683e5b09b4..8bf94b669515 100644 --- a/cmd/opampsupervisor/supervisor/supervisor.go +++ b/cmd/opampsupervisor/supervisor/supervisor.go @@ -278,7 +278,7 @@ func (s *Supervisor) getBootstrapInfo() (err error) { return fmt.Errorf("failed to write agent config: %w", err) } - srv := server.New(newLoggerFromZap(s.logger)) + srv := server.New(newLoggerFromZap(s.logger, "opamp-server")) done := make(chan error, 1) var connected atomic.Bool @@ -376,7 +376,7 @@ func (s *Supervisor) startOpAMP() error { } func (s *Supervisor) startOpAMPClient() error { - s.opampClient = client.NewWebSocket(newLoggerFromZap(s.logger)) + s.opampClient = client.NewWebSocket(newLoggerFromZap(s.logger, "opamp-client")) // determine if we need to load a TLS config or not var tlsConfig *tls.Config @@ -454,7 +454,7 @@ func (s *Supervisor) startOpAMPClient() error { // depending on information received by the Supervisor from the remote // OpAMP server. func (s *Supervisor) startOpAMPServer() error { - s.opampServer = server.New(newLoggerFromZap(s.logger)) + s.opampServer = server.New(newLoggerFromZap(s.logger, "opamp-server")) var err error s.opampServerPort, err = s.findRandomPort() diff --git a/cmd/opampsupervisor/supervisor/supervisor_test.go b/cmd/opampsupervisor/supervisor/supervisor_test.go index 6185ff43c1c1..a4a461fdaedb 100644 --- a/cmd/opampsupervisor/supervisor/supervisor_test.go +++ b/cmd/opampsupervisor/supervisor/supervisor_test.go @@ -183,7 +183,7 @@ func Test_onMessage(t *testing.T) { cfgState: &atomic.Value{}, effectiveConfig: &atomic.Value{}, agentHealthCheckEndpoint: "localhost:8000", - opampClient: client.NewHTTP(newLoggerFromZap(zap.NewNop())), + opampClient: client.NewHTTP(newLoggerFromZap(zap.NewNop(), "opamp-client")), } require.NoError(t, s.createTemplates()) @@ -339,7 +339,7 @@ func Test_onMessage(t *testing.T) { cfgState: &atomic.Value{}, effectiveConfig: &atomic.Value{}, agentHealthCheckEndpoint: "localhost:8000", - opampClient: client.NewHTTP(newLoggerFromZap(zap.NewNop())), + opampClient: client.NewHTTP(newLoggerFromZap(zap.NewNop(), "opamp-client")), } require.NoError(t, s.createTemplates()) From 0f4bf56bcc34d8835420cb864820cb4a76cf57d6 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 31 Oct 2024 02:17:28 +0530 Subject: [PATCH 2/3] Add CHANGELOG entry --- ...opamp_supervisor_default_output_paths.yaml | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .chloggen/opamp_supervisor_default_output_paths.yaml diff --git a/.chloggen/opamp_supervisor_default_output_paths.yaml b/.chloggen/opamp_supervisor_default_output_paths.yaml new file mode 100644 index 000000000000..8fce1992cb4c --- /dev/null +++ b/.chloggen/opamp_supervisor_default_output_paths.yaml @@ -0,0 +1,28 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: breaking + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: cmd/opampsupervisor + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Update default output paths to stderr + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [36072] + +# (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: | + The default output paths for the opamp supervisor have been updated to stderr from [stdout, stderr]. + +# 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 b40e7bbf3a26d19b5324ac29b1650a5b96cd7ab8 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Thu, 31 Oct 2024 19:48:11 +0530 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> --- .chloggen/opamp_supervisor_default_output_paths.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.chloggen/opamp_supervisor_default_output_paths.yaml b/.chloggen/opamp_supervisor_default_output_paths.yaml index 8fce1992cb4c..0ca722205166 100644 --- a/.chloggen/opamp_supervisor_default_output_paths.yaml +++ b/.chloggen/opamp_supervisor_default_output_paths.yaml @@ -7,7 +7,7 @@ change_type: breaking component: cmd/opampsupervisor # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: Update default output paths to stderr +note: Update default logger output paths to stderr # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. issues: [36072] @@ -16,7 +16,7 @@ issues: [36072] # These lines will be padded with 2 spaces and then inserted directly into the document. # Use pipe (|) for multiline entries. subtext: | - The default output paths for the opamp supervisor have been updated to stderr from [stdout, stderr]. + The default output paths for the opamp supervisor logger have been updated to stderr from [stdout, stderr]. # 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.