Skip to content

Commit 61c2d14

Browse files
committed
Refactor logging for server configuration and file operations
- Simplified logging messages by removing debug prefixes and enhancing clarity. - Updated log levels for configuration changes and reloads to improve traceability. - Streamlined logging in the upstream manager for better consistency.
1 parent 7341a2f commit 61c2d14

File tree

3 files changed

+20
-66
lines changed

3 files changed

+20
-66
lines changed

internal/server/server.go

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -346,16 +346,11 @@ func (s *Server) loadConfiguredServers() error {
346346
storedServer.Protocol != serverCfg.Protocol
347347

348348
if hasChanged {
349-
s.logger.Info("CONFIG_DEBUG: Server configuration changed, updating storage",
349+
s.logger.Info("Server configuration changed, updating storage",
350350
zap.String("server", serverCfg.Name),
351351
zap.Bool("new", !existsInStorage),
352352
zap.Bool("enabled_changed", existsInStorage && storedServer.Enabled != serverCfg.Enabled),
353353
zap.Bool("quarantined_changed", existsInStorage && storedServer.Quarantined != serverCfg.Quarantined))
354-
} else {
355-
s.logger.Info("CONFIG_DEBUG: Server configuration unchanged, but still processing",
356-
zap.String("server", serverCfg.Name),
357-
zap.Bool("enabled", serverCfg.Enabled),
358-
zap.Bool("quarantined", serverCfg.Quarantined))
359354
}
360355

361356
// Always sync config to storage (ensures consistency)
@@ -368,9 +363,6 @@ func (s *Server) loadConfiguredServers() error {
368363
if serverCfg.Enabled {
369364
// Add server to upstream manager regardless of quarantine status
370365
// Quarantined servers are kept connected for inspection but blocked for execution
371-
s.logger.Info("CONFIG_DEBUG: Calling AddServer on upstream manager",
372-
zap.String("server", serverCfg.Name),
373-
zap.Bool("config_changed", hasChanged))
374366
if err := s.upstreamManager.AddServer(serverCfg.Name, serverCfg); err != nil {
375367
s.logger.Error("Failed to add/update upstream server", zap.Error(err), zap.String("server", serverCfg.Name))
376368
}
@@ -380,8 +372,6 @@ func (s *Server) loadConfiguredServers() error {
380372
}
381373
} else {
382374
// Remove from upstream manager only if disabled (not quarantined)
383-
s.logger.Info("CONFIG_DEBUG: Removing disabled server from upstream manager",
384-
zap.String("server", serverCfg.Name))
385375
s.upstreamManager.RemoveServer(serverCfg.Name)
386376
s.logger.Info("Server is disabled, removing from active connections", zap.String("server", serverCfg.Name))
387377
}
@@ -750,11 +740,7 @@ func (s *Server) EnableServer(serverName string, enabled bool) error {
750740

751741
// QuarantineServer quarantines/unquarantines a server
752742
func (s *Server) QuarantineServer(serverName string, quarantined bool) error {
753-
s.logger.Info("CONFIG_DEBUG: QuarantineServer called - request to change server quarantine state",
754-
zap.String("server", serverName),
755-
zap.Bool("quarantined", quarantined))
756-
757-
s.logger.Debug("CONFIG_DEBUG: Calling storage manager QuarantineUpstreamServer",
743+
s.logger.Info("Request to change server quarantine state",
758744
zap.String("server", serverName),
759745
zap.Bool("quarantined", quarantined))
760746

@@ -763,15 +749,11 @@ func (s *Server) QuarantineServer(serverName string, quarantined bool) error {
763749
return fmt.Errorf("failed to update quarantine state for server '%s' in storage: %w", serverName, err)
764750
}
765751

766-
s.logger.Debug("CONFIG_DEBUG: Successfully updated quarantine state in storage, about to save configuration",
767-
zap.String("server", serverName),
768-
zap.Bool("quarantined", quarantined))
769-
770752
if err := s.SaveConfiguration(); err != nil {
771753
s.logger.Error("Failed to save configuration after quarantine state change", zap.Error(err))
772754
}
773755

774-
s.logger.Info("CONFIG_DEBUG: Successfully persisted server quarantine state change. This will trigger file watcher to reload config.",
756+
s.logger.Info("Successfully persisted server quarantine state change",
775757
zap.String("server", serverName),
776758
zap.Bool("quarantined", quarantined))
777759

@@ -951,7 +933,7 @@ func (s *Server) SaveConfiguration() error {
951933
return fmt.Errorf("configuration file path is not available")
952934
}
953935

954-
s.logger.Info("CONFIG_DEBUG: SaveConfiguration called - saving configuration to file", zap.String("path", configPath))
936+
s.logger.Debug("Saving configuration to file", zap.String("path", configPath))
955937

956938
// Ensure we have the latest server list from the storage manager
957939
latestServers, err := s.storageManager.ListUpstreamServers()
@@ -961,16 +943,12 @@ func (s *Server) SaveConfiguration() error {
961943
}
962944
s.config.Servers = latestServers
963945

964-
s.logger.Info("CONFIG_DEBUG: About to save config file - this will trigger file watcher",
965-
zap.String("path", configPath),
966-
zap.Int("server_count", len(latestServers)))
967-
968946
return config.SaveConfig(s.config, configPath)
969947
}
970948

971949
// ReloadConfiguration reloads the configuration from disk
972950
func (s *Server) ReloadConfiguration() error {
973-
s.logger.Info("CONFIG_DEBUG: ReloadConfiguration called - config as source of truth")
951+
s.logger.Info("Reloading configuration from disk")
974952

975953
// Store old config for comparison
976954
oldServerCount := len(s.config.Servers)
@@ -986,7 +964,6 @@ func (s *Server) ReloadConfiguration() error {
986964
s.config = newConfig
987965

988966
// Reload configured servers (this is where the comprehensive sync happens)
989-
s.logger.Info("CONFIG_DEBUG: About to call loadConfiguredServers from ReloadConfiguration")
990967
if err := s.loadConfiguredServers(); err != nil {
991968
return fmt.Errorf("failed to reload servers: %w", err)
992969
}
@@ -1001,7 +978,7 @@ func (s *Server) ReloadConfiguration() error {
1001978
}
1002979
}()
1003980

1004-
s.logger.Info("CONFIG_DEBUG: Configuration reload completed",
981+
s.logger.Info("Configuration reload completed",
1005982
zap.String("path", configPath),
1006983
zap.Int("old_server_count", oldServerCount),
1007984
zap.Int("new_server_count", len(newConfig.Servers)),

internal/tray/tray.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -261,15 +261,15 @@ func (a *App) watchConfigFile() {
261261
}
262262

263263
if event.Has(fsnotify.Write) || event.Has(fsnotify.Create) {
264-
a.logger.Info("CONFIG_DEBUG: Config file changed, reloading configuration", zap.String("event", event.String()))
264+
a.logger.Debug("Config file changed, reloading configuration", zap.String("event", event.String()))
265265

266266
// Add a small delay to ensure file write is complete
267267
time.Sleep(500 * time.Millisecond)
268268

269269
if err := a.server.ReloadConfiguration(); err != nil {
270270
a.logger.Error("Failed to reload configuration", zap.Error(err))
271271
} else {
272-
a.logger.Info("CONFIG_DEBUG: Configuration reloaded successfully")
272+
a.logger.Debug("Configuration reloaded successfully")
273273
// Force a menu refresh after config reload
274274
a.forceRefresh = true
275275
a.refreshMenusImmediate()

internal/upstream/manager.go

Lines changed: 12 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -64,34 +64,23 @@ func (m *Manager) AddServerConfig(id string, serverConfig *config.ServerConfig)
6464
existingConfig.Quarantined != serverConfig.Quarantined
6565

6666
if configChanged {
67-
m.logger.Info("UPSTREAM_DEBUG: Server configuration changed, disconnecting existing client",
67+
m.logger.Info("Server configuration changed, disconnecting existing client",
6868
zap.String("id", id),
6969
zap.String("name", serverConfig.Name),
7070
zap.String("current_state", existingClient.GetState().String()),
71-
zap.Bool("is_connected", existingClient.IsConnected()),
72-
zap.Bool("url_changed", existingConfig.URL != serverConfig.URL),
73-
zap.Bool("protocol_changed", existingConfig.Protocol != serverConfig.Protocol),
74-
zap.Bool("command_changed", existingConfig.Command != serverConfig.Command),
75-
zap.Bool("enabled_changed", existingConfig.Enabled != serverConfig.Enabled),
76-
zap.Bool("quarantined_changed", existingConfig.Quarantined != serverConfig.Quarantined))
71+
zap.Bool("is_connected", existingClient.IsConnected()))
7772
_ = existingClient.Disconnect()
7873
delete(m.clients, id)
7974
} else {
80-
m.logger.Info("UPSTREAM_DEBUG: Server configuration unchanged, keeping existing client",
75+
m.logger.Debug("Server configuration unchanged, keeping existing client",
8176
zap.String("id", id),
8277
zap.String("name", serverConfig.Name),
8378
zap.String("current_state", existingClient.GetState().String()),
84-
zap.Bool("is_connected", existingClient.IsConnected()),
85-
zap.Bool("enabled", serverConfig.Enabled),
86-
zap.Bool("quarantined", serverConfig.Quarantined))
79+
zap.Bool("is_connected", existingClient.IsConnected()))
8780
// Update the client's config reference to the new config but don't recreate the client
8881
existingClient.config = serverConfig
8982
return nil
9083
}
91-
} else {
92-
m.logger.Info("UPSTREAM_DEBUG: No existing client found, creating new client",
93-
zap.String("id", id),
94-
zap.String("name", serverConfig.Name))
9584
}
9685

9786
// Create new client but don't connect yet
@@ -116,11 +105,9 @@ func (m *Manager) AddServerConfig(id string, serverConfig *config.ServerConfig)
116105
}
117106

118107
m.clients[id] = client
119-
m.logger.Info("UPSTREAM_DEBUG: Added new client configuration",
108+
m.logger.Info("Added upstream server configuration",
120109
zap.String("id", id),
121-
zap.String("name", serverConfig.Name),
122-
zap.Bool("enabled", serverConfig.Enabled),
123-
zap.Bool("quarantined", serverConfig.Quarantined))
110+
zap.String("name", serverConfig.Name))
124111

125112
return nil
126113
}
@@ -157,7 +144,7 @@ func (m *Manager) AddServer(id string, serverConfig *config.ServerConfig) error
157144
}
158145

159146
if !serverConfig.Enabled {
160-
m.logger.Info("UPSTREAM_DEBUG: Server is disabled, skipping connection",
147+
m.logger.Debug("Skipping connection for disabled server",
161148
zap.String("id", id),
162149
zap.String("name", serverConfig.Name))
163150
return nil
@@ -166,25 +153,19 @@ func (m *Manager) AddServer(id string, serverConfig *config.ServerConfig) error
166153
// Check if client exists and is already connected
167154
if client, exists := m.GetClient(id); exists {
168155
if client.IsConnected() {
169-
m.logger.Info("UPSTREAM_DEBUG: Server is already connected, skipping connection attempt",
156+
m.logger.Debug("Server is already connected, skipping connection attempt",
170157
zap.String("id", id),
171-
zap.String("name", serverConfig.Name),
172-
zap.String("state", client.GetState().String()))
158+
zap.String("name", serverConfig.Name))
173159
return nil
174160
}
175161

176-
m.logger.Info("UPSTREAM_DEBUG: Server exists but not connected, attempting connection",
177-
zap.String("id", id),
178-
zap.String("name", serverConfig.Name),
179-
zap.String("state", client.GetState().String()))
180-
181162
// Connect to server
182163
ctx := context.Background()
183164
if err := client.Connect(ctx); err != nil {
184165
return fmt.Errorf("failed to connect to server %s: %w", serverConfig.Name, err)
185166
}
186167
} else {
187-
m.logger.Error("UPSTREAM_DEBUG: Client not found after AddServerConfig - this should not happen",
168+
m.logger.Error("Client not found after AddServerConfig - this should not happen",
188169
zap.String("id", id),
189170
zap.String("name", serverConfig.Name))
190171
}
@@ -198,15 +179,11 @@ func (m *Manager) RemoveServer(id string) {
198179
defer m.mu.Unlock()
199180

200181
if client, exists := m.clients[id]; exists {
201-
m.logger.Info("UPSTREAM_DEBUG: Removing server, disconnecting client",
182+
m.logger.Info("Removing upstream server",
202183
zap.String("id", id),
203-
zap.String("state", client.GetState().String()),
204-
zap.Bool("is_connected", client.IsConnected()))
184+
zap.String("state", client.GetState().String()))
205185
_ = client.Disconnect()
206186
delete(m.clients, id)
207-
m.logger.Info("UPSTREAM_DEBUG: Removed upstream server", zap.String("id", id))
208-
} else {
209-
m.logger.Info("UPSTREAM_DEBUG: Server not found for removal", zap.String("id", id))
210187
}
211188
}
212189

0 commit comments

Comments
 (0)