Skip to content

Commit 0c2bde1

Browse files
joonas-fijoonas-fi
committed
better CLI wrapping & begin using log/slog 
1 parent d54f69d commit 0c2bde1

12 files changed

+247
-58
lines changed

‎README.md

+3-2
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,6 @@ related things like AWS wrappers or Prometheus helpers.
2828
Deprecations
2929
------------
3030

31-
- `csrf/` - just use SameSite cookies
32-
- `crypto/pkencryptedstream/` - provides confidentiality, but is malleable (ciphertext is not authenticated). Use Age instead.
31+
- [net/http/csrf/](net/http/csrf/) - just use SameSite cookies
32+
- [crypto/pkencryptedstream/](crypto/pkencryptedstream/) - provides confidentiality, but is malleable (ciphertext is not authenticated). Use Age instead.
33+
- [log/logex/](log/logex/) - use [log/slog](https://go.dev/blog/slog) i.e. the official solution to pain which our package addressed.

‎app/cli/DEPRECATED.go

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package cli
2+
3+
import (
4+
"context"
5+
"log"
6+
7+
"github.com/function61/gokit/log/logex"
8+
"github.com/function61/gokit/os/osutil"
9+
"github.com/spf13/cobra"
10+
)
11+
12+
// below intended to be deprecated soon
13+
14+
func RunnerNoArgs(run func(ctx context.Context, logger *log.Logger) error) func(*cobra.Command, []string) {
15+
return func(_ *cobra.Command, _ []string) {
16+
logger := logex.StandardLogger()
17+
18+
osutil.ExitIfError(run(
19+
osutil.CancelOnInterruptOrTerminate(logger),
20+
logger))
21+
}
22+
}
23+
24+
func Runner(run func(ctx context.Context, args []string, logger *log.Logger) error) func(*cobra.Command, []string) {
25+
return func(_ *cobra.Command, args []string) {
26+
logger := logex.StandardLogger()
27+
28+
osutil.ExitIfError(run(
29+
osutil.CancelOnInterruptOrTerminate(logger),
30+
args,
31+
logger))
32+
}
33+
}

‎app/cli/cobrawrappers.go

+42-16
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,58 @@
1-
// Cobra wrappers to wrap awkward API (no exit codes and no built-in "ctrl-c => cancel" support)
1+
// Making CLI commands have some quality without too much boilerplate.
22
package cli
33

44
import (
55
"context"
6-
"log"
6+
"os"
77

8-
"github.com/function61/gokit/log/logex"
8+
"github.com/function61/gokit/app/dynversion"
99
"github.com/function61/gokit/os/osutil"
1010
"github.com/spf13/cobra"
11+
"github.com/spf13/pflag"
1112
)
1213

13-
func RunnerNoArgs(run func(ctx context.Context, logger *log.Logger) error) func(*cobra.Command, []string) {
14-
return func(_ *cobra.Command, _ []string) {
15-
logger := logex.StandardLogger()
14+
// wraps the `Execute()` call of the command to inject boilerplate details like `Use`, `Version` and
15+
// handling of error to `Command.Execute()` (such as flag validation, missing command etc.)
16+
func Execute(app *cobra.Command) {
17+
// dirty to mutate after-the-fact
1618

17-
osutil.ExitIfError(run(
18-
osutil.CancelOnInterruptOrTerminate(logger),
19-
logger))
20-
}
19+
app.Use = os.Args[0]
20+
app.Version = dynversion.Version
21+
// hide the default "completion" subcommand from polluting UX (it can still be used). https://github.com/spf13/cobra/issues/1507
22+
app.CompletionOptions = cobra.CompletionOptions{HiddenDefaultCmd: true}
23+
24+
// cannot `AddLogLevelControls(app.Flags())` here because it gets confusing if:
25+
// a) the root command is not runnable
26+
// b) the root command is runnable BUT it doesn't do logging (or there is no debug-level logs to suppress)
27+
28+
osutil.ExitIfError(app.Execute())
2129
}
2230

23-
func Runner(run func(ctx context.Context, args []string, logger *log.Logger) error) func(*cobra.Command, []string) {
31+
// fixes problems of cobra commands' bare run callbacks with regards to application quality:
32+
// 1. logging not configured
33+
// 2. no interrupt handling
34+
// 3. no error handling
35+
func WrapRun(run func(ctx context.Context, args []string) error) func(*cobra.Command, []string) {
2436
return func(_ *cobra.Command, args []string) {
25-
logger := logex.StandardLogger()
37+
// handle logging
38+
configureLogging()
39+
40+
// handle interrupts
41+
ctx := notifyContextInterruptOrTerminate()
2642

27-
osutil.ExitIfError(run(
28-
osutil.CancelOnInterruptOrTerminate(logger),
29-
args,
30-
logger))
43+
// run the actual code (jump from CLI context to higher-level application context)
44+
// this can be kinda read as:
45+
// output = logic(input)
46+
err := run(ctx, args)
47+
48+
// handle errors
49+
osutil.ExitIfError(err)
3150
}
3251
}
52+
53+
// adds CLI flags that control the logging level
54+
func AddLogLevelControls(flags *pflag.FlagSet) {
55+
flags.BoolVarP(&logLevelVerbose, "verbose", "v", logLevelVerbose, "Include debug-level logs")
56+
57+
// TODO: maybe add a "quiet" level as well
58+
}

‎app/cli/logging.go

+82
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package cli
2+
3+
import (
4+
"log/slog"
5+
"os"
6+
"time"
7+
8+
"github.com/lmittmann/tint"
9+
"github.com/mattn/go-isatty"
10+
)
11+
12+
var (
13+
logLevelVerbose = false
14+
discardAttr = slog.Attr{} // zero `Attr` means discard
15+
)
16+
17+
func configureLogging() {
18+
logLevel := func() slog.Level {
19+
if logLevelVerbose {
20+
return slog.LevelDebug
21+
} else {
22+
return slog.LevelInfo
23+
}
24+
}()
25+
26+
addSource := func() bool {
27+
if logLevelVerbose {
28+
return true
29+
} else {
30+
return false
31+
}
32+
}()
33+
34+
errorStream := os.Stderr
35+
errorStreamIsUserTerminal := isatty.IsTerminal(errorStream.Fd())
36+
37+
logHandler := func() slog.Handler {
38+
if errorStreamIsUserTerminal { // output format optimized to looking at from terminal
39+
return tint.NewHandler(errorStream, &tint.Options{
40+
Level: logLevel,
41+
AddSource: addSource,
42+
TimeFormat: time.TimeOnly, // not using freedom time (`time.Kitchen`)
43+
// intentionally not giving `ReplaceAttr` because for terminal we can always include times
44+
})
45+
} else { // "production" log output
46+
logAttrReplacer := timeRemoverAttrReplacer
47+
if !logsShouldOmitTime() {
48+
logAttrReplacer = nil
49+
}
50+
51+
return slog.NewTextHandler(errorStream, &slog.HandlerOptions{
52+
Level: logLevel,
53+
AddSource: addSource,
54+
ReplaceAttr: logAttrReplacer,
55+
})
56+
}
57+
}()
58+
59+
// expecting the apps to just use the global logger
60+
slog.SetDefault(slog.New(logHandler))
61+
}
62+
63+
// if our logs are redirected to journald or similar which already add timestamps don't add double timestamps
64+
func logsShouldOmitTime() bool {
65+
// "This permits invoked processes to safely detect whether their standard output or standard
66+
// error output are connected to the journal."
67+
// https://www.freedesktop.org/software/systemd/man/systemd.exec.html#%24JOURNAL_STREAM
68+
systemdJournal := os.Getenv("JOURNAL_STREAM") != ""
69+
70+
// explicitly asked, e.g. set by orchestrator when running in Docker with log redirection taken care of
71+
explicitSuppress := os.Getenv("LOGGER_SUPPRESS_TIMESTAMPS") == "1"
72+
73+
return systemdJournal || explicitSuppress
74+
}
75+
76+
func timeRemoverAttrReplacer(groups []string, a slog.Attr) slog.Attr {
77+
if a.Key == slog.TimeKey {
78+
return discardAttr
79+
} else {
80+
return a
81+
}
82+
}

‎app/cli/signalnotifycontext.go

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package cli
2+
3+
import (
4+
"context"
5+
"log/slog"
6+
"os"
7+
"os/signal"
8+
"syscall"
9+
)
10+
11+
// same as `signal.NotifyContext()` but logs output when got signal to give useful feedback to user
12+
// that we have begun teardown. this feedback becomes extremely important if the teardown process
13+
// takes time or gets stuck.
14+
func notifyContextInterruptOrTerminate() context.Context {
15+
ctx, cancel := context.WithCancel(context.Background())
16+
17+
// need a buffered channel because the sending side is non-blocking
18+
ch := make(chan os.Signal, 1)
19+
20+
// "The SIGINT signal is sent to a process by its controlling terminal when a user wishes to interrupt the process"
21+
// "The SIGTERM signal is sent to a process to request its termination"
22+
// "SIGINT is nearly identical to SIGTERM"
23+
// https://en.wikipedia.org/wiki/Signal_(IPC)
24+
signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM)
25+
26+
go func() {
27+
slog.Info("STOPPING. (If stuck, send sig again to force exit.)", "signal", <-ch)
28+
29+
// stop accepting signals on the channel. this undoes the effect of this func,
30+
// and thus makes the process terminate on the next received signal (so you can stop
31+
// your program if the cleanup code gets stuck)
32+
signal.Stop(ch)
33+
34+
cancel()
35+
}()
36+
37+
return ctx
38+
}

‎app/retry/retry.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ package retry
44
import (
55
"context"
66
"fmt"
7-
"log"
7+
"log/slog"
88
"time"
99

1010
"github.com/function61/gokit/app/backoff"
@@ -67,8 +67,8 @@ func IgnoreErrorsWithin(expectErrorsWithin time.Duration, handleError func(error
6767
}
6868

6969
// creates `Retry()` compatible error handler function that just pushes errors to a logger
70-
func ErrorsToLogger(logger *log.Logger) func(error) {
70+
func ErrorsToLogger(logger *slog.Logger) func(error) {
7171
return func(err error) {
72-
logger.Println(err.Error())
72+
logger.Error(err.Error())
7373
}
7474
}

‎go.mod

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ require (
2424
github.com/golang/protobuf v1.3.2 // indirect
2525
github.com/inconshreveable/mousetrap v1.0.1 // indirect
2626
github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af // indirect
27+
github.com/lmittmann/tint v1.0.5 // indirect
2728
github.com/matttproud/golang_protobuf_extensions v1.0.1 // indirect
2829
github.com/pkg/errors v0.8.1 // indirect
2930
github.com/prometheus/client_model v0.2.0 // indirect

‎go.sum

+2
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ github.com/json-iterator/go v1.1.7/go.mod h1:KdQUCv79m/52Kvf8AW2vK1V8akMuk1QjK/u
4545
github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7VTCxuUUipMqKk8s4w=
4646
github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ=
4747
github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc=
48+
github.com/lmittmann/tint v1.0.5 h1:NQclAutOfYsqs2F1Lenue6OoWCajs5wJcP3DfWVpePw=
49+
github.com/lmittmann/tint v1.0.5/go.mod h1:HIS3gSy7qNwGCj+5oRjAutErFBl4BzdQP6cJZ0NfMwE=
4850
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
4951
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
5052
github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU=

‎net/netutil/cancelableserver.go

+3-4
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@ package netutil
33
import (
44
"context"
55
"errors"
6-
"log"
6+
"log/slog"
77
"net"
88
"sync"
99

1010
. "github.com/function61/gokit/builtin"
11-
"github.com/function61/gokit/log/logex"
1211
"github.com/function61/gokit/sync/syncutil"
1312
)
1413

@@ -53,13 +52,13 @@ func CancelableListen(
5352
ctx context.Context,
5453
listener net.Listener,
5554
listen func() error,
56-
logger *log.Logger,
55+
logger *slog.Logger,
5756
) error {
5857
go func() {
5958
<-ctx.Done()
6059

6160
if err := listener.Close(); err != nil {
62-
logex.Levels(logger).Error.Println(err.Error())
61+
logger.Error("listener close", "err", err)
6362
}
6463
}()
6564

‎os/systemdcli/cli.go

+6-7
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ package systemdcli
44
import (
55
"context"
66
"fmt"
7-
"log"
87
"os"
98
"os/exec"
109

@@ -40,7 +39,7 @@ func Entrypoint(serviceName string, makeAdditionalCommands func(string) []*cobra
4039
Use: "start",
4140
Short: "Start the service",
4241
Args: cobra.NoArgs,
43-
Run: cli.RunnerNoArgs(func(ctx context.Context, _ *log.Logger) error {
42+
Run: cli.WrapRun(func(ctx context.Context, _ []string) error {
4443
return runSystemctlVerb(ctx, "start")
4544
}),
4645
})
@@ -49,7 +48,7 @@ func Entrypoint(serviceName string, makeAdditionalCommands func(string) []*cobra
4948
Use: "stop",
5049
Short: "Stop the service",
5150
Args: cobra.NoArgs,
52-
Run: cli.RunnerNoArgs(func(ctx context.Context, _ *log.Logger) error {
51+
Run: cli.WrapRun(func(ctx context.Context, _ []string) error {
5352
return runSystemctlVerb(ctx, "stop")
5453
}),
5554
})
@@ -58,7 +57,7 @@ func Entrypoint(serviceName string, makeAdditionalCommands func(string) []*cobra
5857
Use: "restart",
5958
Short: "Restart the service",
6059
Args: cobra.NoArgs,
61-
Run: cli.RunnerNoArgs(func(ctx context.Context, _ *log.Logger) error {
60+
Run: cli.WrapRun(func(ctx context.Context, _ []string) error {
6261
return runSystemctlVerb(ctx, "restart")
6362
}),
6463
})
@@ -67,7 +66,7 @@ func Entrypoint(serviceName string, makeAdditionalCommands func(string) []*cobra
6766
Use: "status",
6867
Short: "Show status of the service",
6968
Args: cobra.NoArgs,
70-
Run: cli.RunnerNoArgs(func(ctx context.Context, _ *log.Logger) error {
69+
Run: cli.WrapRun(func(ctx context.Context, _ []string) error {
7170
translateNonError := func(err error) error {
7271
if err != nil {
7372
// LSB dictates that successful status show of non-running program must return 3:
@@ -90,7 +89,7 @@ func Entrypoint(serviceName string, makeAdditionalCommands func(string) []*cobra
9089
Use: "logs",
9190
Short: "Get logs for the service",
9291
Args: cobra.NoArgs,
93-
Run: cli.RunnerNoArgs(func(ctx context.Context, _ *log.Logger) error {
92+
Run: cli.WrapRun(func(ctx context.Context, _ []string) error {
9493
//nolint:gosec // ok, is expected to not be user input.
9594
logsCmd := exec.CommandContext(ctx, "journalctl", "--unit="+serviceName)
9695
logsCmd.Stdout = os.Stdout
@@ -109,7 +108,7 @@ func WithInstallAndUninstallCommands(makeSvc func(string) (*systemdinstaller.Ser
109108
Use: "install",
110109
Short: "Installs the background service",
111110
Args: cobra.NoArgs,
112-
Run: cli.RunnerNoArgs(func(ctx context.Context, _ *log.Logger) error {
111+
Run: cli.WrapRun(func(_ context.Context, _ []string) error {
113112
svc, err := makeSvc(serviceName)
114113
if err != nil {
115114
return err

0 commit comments

Comments
 (0)