Skip to content

Commit 694c1cb

Browse files
committed
refactor(dlv): replace custom logger func with structured logger
- Before this commit, logging relied on a custom logger function that lacked structured logging, leading to inconsistent logging patterns and limited flexibility. - After this commit, a structured `logger.Logger` is used, improving log consistency, flexibility, and integration with the existing logging framework.
1 parent 5e9069f commit 694c1cb

File tree

4 files changed

+19
-42
lines changed

4 files changed

+19
-42
lines changed

cmd/jujud-controller/main_debug.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func main() {
2424

2525
// Start the delve runner against a socket.
2626
debugMain := dlv.NewDlvRunner(
27-
dlv.WithLoggerFunc(logger.Infof),
27+
dlv.WithLogger(logger.Child("dlv")),
2828
dlv.Headless(),
2929
dlv.NoWait(),
3030
dlv.WithApiVersion(2),

cmd/jujud/main_debug.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ func main() {
2424

2525
// Start the delve runner against a socket.
2626
debugMain := dlv.NewDlvRunner(
27-
dlv.WithLoggerFunc(logger.Infof),
27+
dlv.WithLogger(logger.Child("dlv")),
2828
dlv.Headless(),
2929
dlv.NoWait(),
3030
dlv.WithApiVersion(2),

internal/dlv/delve.go

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package dlv
55

66
import (
7-
"fmt"
87
"os"
98

109
"github.com/go-delve/delve/cmd/dlv/cmds"
@@ -36,8 +35,8 @@ func NewDlvRunner(opts ...Option) func(main MainWithArgs) MainWithArgs {
3635
return main
3736
}
3837
if err := os.Setenv(envNoDebug, "1"); err != nil {
39-
logger.Printf("Failed to set env %q: %v", envNoDebug, err)
40-
logger.Printf("Starting without debug mode...")
38+
logger.Warningf("Failed to set env %q: %v", envNoDebug, err)
39+
logger.Warningf("Starting without debug mode...")
4140
return main
4241
}
4342

@@ -55,21 +54,16 @@ func NewDlvRunner(opts ...Option) func(main MainWithArgs) MainWithArgs {
5554
// socket
5655
config.runSidecars()
5756

58-
logger.Printf("Starting dlv with %v", dlvArgs)
59-
logger.Printf("Running in debug mode")
60-
defer logger.Printf("dlv has stopped")
57+
logger.Infof("Starting dlv with %v", dlvArgs)
58+
logger.Infof("Running in debug mode")
59+
defer logger.Infof("dlv has stopped")
6160

6261
// Execute delve
6362
if err := dlvCmd.Execute(); err != nil {
64-
fmt.Printf("Failed to run dlv: %v\n", err)
63+
logger.Errorf("Failed to run dlv: %v\n", err)
6564
return 1
6665
}
6766
return 0
6867
}
6968
}
7069
}
71-
72-
// logger allows to inject the way logs will be handled by wrapped program.
73-
type logger interface {
74-
Printf(format string, v ...interface{})
75-
}

internal/dlv/options.go

Lines changed: 11 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,16 @@ import (
1212

1313
"github.com/juju/clock"
1414
"github.com/juju/retry"
15+
16+
"github.com/juju/juju/core/logger"
17+
internallogger "github.com/juju/juju/internal/logger"
1518
)
1619

1720
// Config is a type that represents a map of configuration options where the key is a string and the value can be any type.
1821
// They are passed to Delve as command line option.
1922
type Config struct {
20-
// definedLogger allows injecting a custom logger implementation to handle logging for the wrapped program.
21-
definedLogger logger
23+
// definedLogger allows injecting the logger to dlv instance
24+
definedLogger logger.Logger
2225

2326
// dlvArgs contains a map of command line arguments for the Delve debugger, which would typically be added to the
2427
// command in the form `--[key]=[value]`
@@ -50,14 +53,10 @@ func With(key string, value ...any) Option {
5053
}
5154
}
5255

53-
// LoggerFunc is a type that defines a function signature for logging messages formatted according to a format specifier.
54-
type LoggerFunc func(format string, v ...interface{})
55-
56-
// WithLoggerFunc allows setting a custom LoggerFunc for logging.
57-
// It returns an Option that configures a Config instance to use the provided LoggerFunc for logging.
58-
func WithLoggerFunc(logger LoggerFunc) Option {
56+
// WithLogger sets a custom logger to be used in the configuration.
57+
func WithLogger(logger logger.Logger) Option {
5958
return func(o *Config) {
60-
o.definedLogger = loggerWrapper{logf: logger}
59+
o.definedLogger = logger
6160
}
6261
}
6362

@@ -138,30 +137,14 @@ func tryFixPermission(path string) error {
138137
})
139138
}
140139

141-
// logger returns the logger implementation to use, defaulting to the predefined defaultLogger if none is specified.
142-
func (config *Config) logger() logger {
140+
// logger returns the logger implementation to use, defaulting to noop
141+
func (config *Config) logger() logger.Logger {
143142
if config.definedLogger == nil {
144-
return defaultLogger
143+
return internallogger.Noop()
145144
}
146145
return config.definedLogger
147146
}
148147

149-
// loggerWrapper wraps a LoggerFunc to conform with the logger interface.
150-
type loggerWrapper struct {
151-
logf LoggerFunc
152-
}
153-
154-
// Printf logs a formatted message using a specified format and variadic arguments.
155-
func (l loggerWrapper) Printf(format string, v ...interface{}) {
156-
l.logf(format, v...)
157-
}
158-
159-
// defaultLogger is the default logger implementing the logger interface, using fmt.Printf to log formatted messages.
160-
var defaultLogger = loggerWrapper{func(format string, v ...interface{}) {
161-
fmt.Printf(format, v...)
162-
fmt.Println()
163-
}}
164-
165148
// apply sets multiple configuration options on a Config instance.
166149
func (o *Config) apply(opts ...Option) {
167150
for _, opt := range opts {

0 commit comments

Comments
 (0)