From 0f091ab3b4e2e69d025fa4dac68ff68d5d7493d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Mon, 26 Sep 2022 12:45:53 +0100 Subject: [PATCH 1/8] separates functionality by packages this allows clients importing just the bits they need without bringing all the transitive module dependencies --- cobrautil.go | 7 +++++-- example_test.go | 15 ++++++++------- go.mod | 2 +- grpc.go => grpc/grpc.go | 22 ++++++++++++---------- http.go => http/http.go | 17 +++++++++-------- otel.go => otel/otel.go | 25 +++++++++++++------------ zerolog.go => zerolog/zerolog.go | 15 ++++++++------- 7 files changed, 56 insertions(+), 47 deletions(-) rename grpc.go => grpc/grpc.go (80%) rename http.go => http/http.go (80%) rename otel.go => otel/otel.go (87%) rename zerolog.go => zerolog/zerolog.go (77%) diff --git a/cobrautil.go b/cobrautil.go index 6be3e13..2bed329 100644 --- a/cobrautil.go +++ b/cobrautil.go @@ -53,7 +53,7 @@ func SyncViperPreRunE(prefix string) CobraRunFunc { // CobraRunFunc is the signature of cobra.Command RunFuncs. type CobraRunFunc func(cmd *cobra.Command, args []string) error -// RunFuncStack chains together a collection of CobraCommandFuncs into one. +// CommandStack chains together a collection of CobraCommandFuncs into one. func CommandStack(cmdfns ...CobraRunFunc) CobraRunFunc { return func(cmd *cobra.Command, args []string) error { for _, cmdfn := range cmdfns { @@ -65,7 +65,10 @@ func CommandStack(cmdfns ...CobraRunFunc) CobraRunFunc { } } -func prefixJoiner(prefix string) func(...string) string { +// PrefixJoiner joins a list of strings with the "-" separator, including the provided prefix string +// +// example: PrefixJoiner("hi")("how", "are", "you") = "hi-how-are-you" +func PrefixJoiner(prefix string) func(...string) string { return func(xs ...string) string { return stringz.Join("-", append([]string{prefix}, xs...)...) } diff --git a/example_test.go b/example_test.go index c3e01f4..5489dc5 100644 --- a/example_test.go +++ b/example_test.go @@ -4,7 +4,8 @@ import ( "github.com/rs/zerolog" "github.com/spf13/cobra" - "github.com/jzelinskie/cobrautil" + "github.com/jzelinskie/cobrautil/v2" + zl "github.com/jzelinskie/cobrautil/v2/zerolog" ) func ExampleCommandStack() { @@ -12,27 +13,27 @@ func ExampleCommandStack() { Use: "mycmd", PreRunE: cobrautil.CommandStack( cobrautil.SyncViperPreRunE("myprogram"), - cobrautil.ZeroLogRunE("log", zerolog.InfoLevel), + zl.ZeroLogRunE("log", zerolog.InfoLevel), ), } - cobrautil.RegisterZeroLogFlags(cmd.PersistentFlags(), "log") + zl.RegisterZeroLogFlags(cmd.PersistentFlags(), "log") } func ExampleRegisterZeroLogFlags() { cmd := &cobra.Command{ Use: "mycmd", - PreRunE: cobrautil.ZeroLogRunE("log", zerolog.InfoLevel), + PreRunE: zl.ZeroLogRunE("log", zerolog.InfoLevel), } - cobrautil.RegisterZeroLogFlags(cmd.PersistentFlags(), "log") + zl.RegisterZeroLogFlags(cmd.PersistentFlags(), "log") } func ExampleZeroLogRunE() { cmd := &cobra.Command{ Use: "mycmd", - PreRunE: cobrautil.ZeroLogRunE("log", zerolog.InfoLevel), + PreRunE: zl.ZeroLogRunE("log", zerolog.InfoLevel), } - cobrautil.RegisterZeroLogFlags(cmd.PersistentFlags(), "log") + zl.RegisterZeroLogFlags(cmd.PersistentFlags(), "log") } diff --git a/go.mod b/go.mod index a62a8bf..a516956 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/jzelinskie/cobrautil +module github.com/jzelinskie/cobrautil/v2 go 1.18 diff --git a/grpc.go b/grpc/grpc.go similarity index 80% rename from grpc.go rename to grpc/grpc.go index 88c3fe4..dd03267 100644 --- a/grpc.go +++ b/grpc/grpc.go @@ -1,10 +1,12 @@ -package cobrautil +package grpc import ( "fmt" + "net" "time" + "github.com/jzelinskie/cobrautil/v2" "github.com/jzelinskie/stringz" "github.com/rs/zerolog" "github.com/rs/zerolog/log" @@ -24,7 +26,7 @@ import ( func RegisterGrpcServerFlags(flags *pflag.FlagSet, flagPrefix, serviceName, defaultAddr string, defaultEnabled bool) { serviceName = stringz.DefaultEmpty(serviceName, "grpc") defaultAddr = stringz.DefaultEmpty(defaultAddr, ":50051") - prefixed := prefixJoiner(stringz.DefaultEmpty(flagPrefix, "grpc")) + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "grpc")) flags.String(prefixed("addr"), defaultAddr, "address to listen on to serve "+serviceName) flags.String(prefixed("network"), "tcp", "network type to serve "+serviceName+` ("tcp", "tcp4", "tcp6", "unix", "unixpacket")`) @@ -37,14 +39,14 @@ func RegisterGrpcServerFlags(flags *pflag.FlagSet, flagPrefix, serviceName, defa // GrpcServerFromFlags creates an *grpc.Server as configured by the flags from // RegisterGrpcServerFlags(). func GrpcServerFromFlags(cmd *cobra.Command, flagPrefix string, opts ...grpc.ServerOption) (*grpc.Server, error) { - prefixed := prefixJoiner(stringz.DefaultEmpty(flagPrefix, "grpc")) + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "grpc")) opts = append(opts, grpc.KeepaliveParams(keepalive.ServerParameters{ - MaxConnectionAge: MustGetDuration(cmd, prefixed("max-conn-age")), + MaxConnectionAge: cobrautil.MustGetDuration(cmd, prefixed("max-conn-age")), })) - certPath := MustGetStringExpanded(cmd, prefixed("tls-cert-path")) - keyPath := MustGetStringExpanded(cmd, prefixed("tls-key-path")) + certPath := cobrautil.MustGetStringExpanded(cmd, prefixed("tls-cert-path")) + keyPath := cobrautil.MustGetStringExpanded(cmd, prefixed("tls-key-path")) switch { case certPath == "" && keyPath == "": @@ -71,14 +73,14 @@ func GrpcServerFromFlags(cmd *cobra.Command, flagPrefix string, opts ...grpc.Ser // GrpcListenFromFlags listens on an gRPC server using the configuration stored // in the cobra command that was registered with RegisterGrpcServerFlags. func GrpcListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *grpc.Server, level zerolog.Level) error { - prefixed := prefixJoiner(stringz.DefaultEmpty(flagPrefix, "grpc")) + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "grpc")) - if !MustGetBool(cmd, prefixed("enabled")) { + if !cobrautil.MustGetBool(cmd, prefixed("enabled")) { return nil } - network := MustGetString(cmd, prefixed("network")) - addr := MustGetStringExpanded(cmd, prefixed("addr")) + network := cobrautil.MustGetString(cmd, prefixed("network")) + addr := cobrautil.MustGetStringExpanded(cmd, prefixed("addr")) l, err := net.Listen(network, addr) if err != nil { diff --git a/http.go b/http/http.go similarity index 80% rename from http.go rename to http/http.go index a7bf71e..1c8eaef 100644 --- a/http.go +++ b/http/http.go @@ -1,10 +1,11 @@ -package cobrautil +package http import ( "errors" "fmt" "net/http" + "github.com/jzelinskie/cobrautil/v2" "github.com/jzelinskie/stringz" "github.com/rs/zerolog" "github.com/rs/zerolog/log" @@ -21,7 +22,7 @@ import ( func RegisterHTTPServerFlags(flags *pflag.FlagSet, flagPrefix, serviceName, defaultAddr string, defaultEnabled bool) { serviceName = stringz.DefaultEmpty(serviceName, "http") defaultAddr = stringz.DefaultEmpty(defaultAddr, ":8443") - prefixed := prefixJoiner(stringz.DefaultEmpty(flagPrefix, "http")) + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "http")) flags.String(prefixed("addr"), defaultAddr, "address to listen on to serve "+serviceName) flags.String(prefixed("tls-cert-path"), "", "local path to the TLS certificate used to serve "+serviceName) @@ -32,23 +33,23 @@ func RegisterHTTPServerFlags(flags *pflag.FlagSet, flagPrefix, serviceName, defa // HTTPServerFromFlags creates an *http.Server as configured by the flags from // RegisterHttpServerFlags(). func HTTPServerFromFlags(cmd *cobra.Command, flagPrefix string) *http.Server { - prefixed := prefixJoiner(stringz.DefaultEmpty(flagPrefix, "http")) + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "http")) return &http.Server{ - Addr: MustGetStringExpanded(cmd, prefixed("addr")), + Addr: cobrautil.MustGetStringExpanded(cmd, prefixed("addr")), } } // HTTPListenFromFlags listens on an HTTP server using the configuration stored // in the cobra command that was registered with RegisterHttpServerFlags. func HTTPListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *http.Server, level zerolog.Level) error { - prefixed := prefixJoiner(stringz.DefaultEmpty(flagPrefix, "http")) - if !MustGetBool(cmd, prefixed("enabled")) { + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "http")) + if !cobrautil.MustGetBool(cmd, prefixed("enabled")) { return nil } - certPath := MustGetStringExpanded(cmd, prefixed("tls-cert-path")) - keyPath := MustGetStringExpanded(cmd, prefixed("tls-key-path")) + certPath := cobrautil.MustGetStringExpanded(cmd, prefixed("tls-cert-path")) + keyPath := cobrautil.MustGetStringExpanded(cmd, prefixed("tls-key-path")) switch { case certPath == "" && keyPath == "": diff --git a/otel.go b/otel/otel.go similarity index 87% rename from otel.go rename to otel/otel.go index 6cf29e1..522e15d 100644 --- a/otel.go +++ b/otel/otel.go @@ -1,4 +1,4 @@ -package cobrautil +package otel import ( "context" @@ -7,6 +7,7 @@ import ( "runtime/debug" "strings" + "github.com/jzelinskie/cobrautil/v2" "github.com/jzelinskie/stringz" "github.com/rs/zerolog" "github.com/rs/zerolog/log" @@ -33,7 +34,7 @@ import ( func RegisterOpenTelemetryFlags(flags *pflag.FlagSet, flagPrefix, serviceName string) { bi, _ := debug.ReadBuildInfo() serviceName = stringz.DefaultEmpty(serviceName, bi.Main.Path) - prefixed := prefixJoiner(stringz.DefaultEmpty(flagPrefix, "otel")) + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "otel")) flags.String(prefixed("provider"), "none", `OpenTelemetry provider for tracing ("none", "jaeger, otlphttp", "otlpgrpc")`) flags.String(prefixed("endpoint"), "", "OpenTelemetry collector endpoint - the endpoint can also be set by using enviroment variables") @@ -57,18 +58,18 @@ func RegisterOpenTelemetryFlags(flags *pflag.FlagSet, flagPrefix, serviceName st // // The required flags can be added to a command by using // RegisterOpenTelemetryFlags(). -func OpenTelemetryRunE(flagPrefix string, prerunLevel zerolog.Level) CobraRunFunc { - prefixed := prefixJoiner(stringz.DefaultEmpty(flagPrefix, "otel")) +func OpenTelemetryRunE(flagPrefix string, prerunLevel zerolog.Level) cobrautil.CobraRunFunc { + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "otel")) return func(cmd *cobra.Command, args []string) error { - if IsBuiltinCommand(cmd) { + if cobrautil.IsBuiltinCommand(cmd) { return nil // No-op for builtins } - provider := strings.ToLower(MustGetString(cmd, prefixed("provider"))) - serviceName := MustGetString(cmd, prefixed("service-name")) - endpoint := MustGetString(cmd, prefixed("endpoint")) - insecure := MustGetBool(cmd, prefixed("insecure")) - propagators := strings.Split(MustGetString(cmd, prefixed("trace-propagator")), ",") + provider := strings.ToLower(cobrautil.MustGetString(cmd, prefixed("provider"))) + serviceName := cobrautil.MustGetString(cmd, prefixed("service-name")) + endpoint := cobrautil.MustGetString(cmd, prefixed("endpoint")) + insecure := cobrautil.MustGetBool(cmd, prefixed("insecure")) + propagators := strings.Split(cobrautil.MustGetString(cmd, prefixed("trace-propagator")), ",") var exporter trace.SpanExporter var err error @@ -82,8 +83,8 @@ func OpenTelemetryRunE(flagPrefix string, prerunLevel zerolog.Level) CobraRunFun // Nothing. case "jaeger": // Legacy flags! Will eventually be dropped! - endpoint = stringz.DefaultEmpty(endpoint, MustGetString(cmd, "otel-jaeger-endpoint")) - serviceName = stringz.Default(serviceName, MustGetString(cmd, "otel-jaeger-service-name"), "", cmd.Flags().Lookup(prefixed("service-name")).DefValue) + endpoint = stringz.DefaultEmpty(endpoint, cobrautil.MustGetString(cmd, "otel-jaeger-endpoint")) + serviceName = stringz.Default(serviceName, cobrautil.MustGetString(cmd, "otel-jaeger-service-name"), "", cmd.Flags().Lookup(prefixed("service-name")).DefValue) var opts []jaeger.CollectorEndpointOption diff --git a/zerolog.go b/zerolog/zerolog.go similarity index 77% rename from zerolog.go rename to zerolog/zerolog.go index 327f142..8fa8f28 100644 --- a/zerolog.go +++ b/zerolog/zerolog.go @@ -1,10 +1,11 @@ -package cobrautil +package zerolog import ( "fmt" "os" "strings" + "github.com/jzelinskie/cobrautil/v2" "github.com/jzelinskie/stringz" "github.com/mattn/go-isatty" "github.com/rs/zerolog" @@ -17,7 +18,7 @@ import ( // - "$PREFIX-level" // - "$PREFIX-format" func RegisterZeroLogFlags(flags *pflag.FlagSet, flagPrefix string) { - prefixed := prefixJoiner(stringz.DefaultEmpty(flagPrefix, "log")) + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "log")) flags.String(prefixed("level"), "info", `verbosity of logging ("trace", "debug", "info", "warn", "error")`) flags.String(prefixed("format"), "auto", `format of logs ("auto", "console", "json")`) } @@ -27,19 +28,19 @@ func RegisterZeroLogFlags(flags *pflag.FlagSet, flagPrefix string) { // // The required flags can be added to a command by using // RegisterLoggingPersistentFlags(). -func ZeroLogRunE(flagPrefix string, prerunLevel zerolog.Level) CobraRunFunc { - prefixed := prefixJoiner(stringz.DefaultEmpty(flagPrefix, "log")) +func ZeroLogRunE(flagPrefix string, prerunLevel zerolog.Level) cobrautil.CobraRunFunc { + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "log")) return func(cmd *cobra.Command, args []string) error { - if IsBuiltinCommand(cmd) { + if cobrautil.IsBuiltinCommand(cmd) { return nil // No-op for builtins } - format := MustGetString(cmd, prefixed("format")) + format := cobrautil.MustGetString(cmd, prefixed("format")) if format == "console" || format == "auto" && isatty.IsTerminal(os.Stdout.Fd()) { log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr}) } - level := strings.ToLower(MustGetString(cmd, prefixed("level"))) + level := strings.ToLower(cobrautil.MustGetString(cmd, prefixed("level"))) switch level { case "trace": zerolog.SetGlobalLevel(zerolog.TraceLevel) From 14eed442877b2fff1815f97b869b57d503500ece Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Mon, 26 Sep 2022 15:21:01 +0100 Subject: [PATCH 2/8] introduces a fluent zerolog cobrautil package the functionality is largely the same as v1. The gist of this redesign is configurability: v1 didn't offer any API to have cobrautil consumer adjust the logger was configured. This is important as zerolog provides a vast API with many knobs to tweak. - a new fluent API is offered so that cobrautil/zerolog can be extended with options as the need arises. It's used to implement two new options in the package - v1 assumed the consumer would use the zerolog/log global instance package. This forces the consuming application to use that global instance. The WithTarget option offers a callback when the logger is instantiated, so e.g. consumers can cache the instance somewhere else - v1 used zerolog global level. This is an unexpected side-effect the consumer may not want, specially given cobrautil is creating an instance of a zerolog Logger. The implementation now adjusts the level on the logger instance instead. - singleton methods are provided to offer an easy path of migration to v2. - WithAsync configures the logger to be non-blocking, implemented using diode buffered-ring. This is default by default to retain v1 behaviour. --- zerolog/zerolog.go | 110 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 100 insertions(+), 10 deletions(-) diff --git a/zerolog/zerolog.go b/zerolog/zerolog.go index 8fa8f28..e7e4d7b 100644 --- a/zerolog/zerolog.go +++ b/zerolog/zerolog.go @@ -2,18 +2,47 @@ package zerolog import ( "fmt" + "io" "os" "strings" + "time" "github.com/jzelinskie/cobrautil/v2" "github.com/jzelinskie/stringz" "github.com/mattn/go-isatty" "github.com/rs/zerolog" + "github.com/rs/zerolog/diode" "github.com/rs/zerolog/log" "github.com/spf13/cobra" "github.com/spf13/pflag" ) +// ConfigureFunc is a function used to configure this CobraUtil +type ConfigureFunc = func(cu *CobraUtil) + +// New creates a configuration that exposes RegisterZeroLogFlags and ZeroLogRunE +// to integrate with cobra +func New(flagPrefix string, configurations ...ConfigureFunc) *CobraUtil { + cu := CobraUtil{ + flagPrefix: flagPrefix, + preRunLevel: zerolog.DebugLevel, + } + for _, configure := range configurations { + configure(&cu) + } + return &cu +} + +// CobraUtil carries the configuration for a zerolog CobraRunFunc +type CobraUtil struct { + flagPrefix string + target func(zerolog.Logger) + async bool + asyncSize int + asyncPollInterval time.Duration + preRunLevel zerolog.Level +} + // RegisterZeroLogFlags adds flags for use in with ZeroLogPreRunE: // - "$PREFIX-level" // - "$PREFIX-format" @@ -29,39 +58,100 @@ func RegisterZeroLogFlags(flags *pflag.FlagSet, flagPrefix string) { // The required flags can be added to a command by using // RegisterLoggingPersistentFlags(). func ZeroLogRunE(flagPrefix string, prerunLevel zerolog.Level) cobrautil.CobraRunFunc { - prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "log")) + return New(flagPrefix, WithPreRunLevel(prerunLevel)).RunE() +} + +// RegisterFlags adds flags for use in with ZeroLogPreRunE: +// - "$PREFIX-level" +// - "$PREFIX-format" +func (cu CobraUtil) RegisterFlags(flags *pflag.FlagSet) { + RegisterZeroLogFlags(flags, cu.flagPrefix) +} + +// RunE returns a Cobra run func that configures the corresponding +// log level from a command. +// +// The required flags can be added to a command by using +// RegisterLoggingPersistentFlags(). +func (cu CobraUtil) RunE() cobrautil.CobraRunFunc { + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(cu.flagPrefix, "log")) return func(cmd *cobra.Command, args []string) error { if cobrautil.IsBuiltinCommand(cmd) { return nil // No-op for builtins } + var output io.Writer + format := cobrautil.MustGetString(cmd, prefixed("format")) if format == "console" || format == "auto" && isatty.IsTerminal(os.Stdout.Fd()) { - log.Logger = log.Output(zerolog.ConsoleWriter{Out: os.Stderr}) + output = zerolog.ConsoleWriter{Out: os.Stderr} + } else { + output = os.Stderr + } + + if cu.async { + output = diode.NewWriter(output, 1000, 10*time.Millisecond, func(missed int) { + fmt.Printf("Logger Dropped %d messages", missed) + }) } + l := zerolog.New(output).With().Timestamp().Logger() + level := strings.ToLower(cobrautil.MustGetString(cmd, prefixed("level"))) switch level { case "trace": - zerolog.SetGlobalLevel(zerolog.TraceLevel) + l = l.Level(zerolog.TraceLevel) case "debug": - zerolog.SetGlobalLevel(zerolog.DebugLevel) + l = l.Level(zerolog.DebugLevel) case "info": - zerolog.SetGlobalLevel(zerolog.InfoLevel) + l = l.Level(zerolog.InfoLevel) case "warn": - zerolog.SetGlobalLevel(zerolog.WarnLevel) + l = l.Level(zerolog.WarnLevel) case "error": - zerolog.SetGlobalLevel(zerolog.ErrorLevel) + l = l.Level(zerolog.ErrorLevel) case "fatal": - zerolog.SetGlobalLevel(zerolog.FatalLevel) + l = l.Level(zerolog.FatalLevel) case "panic": - zerolog.SetGlobalLevel(zerolog.PanicLevel) + l = l.Level(zerolog.PanicLevel) default: return fmt.Errorf("unknown log level: %s", level) } - log.WithLevel(prerunLevel).Str("new level", level).Msg("set log level") + if cu.target != nil { + cu.target(l) + } else { + log.Logger = l + } + l.WithLevel(cu.preRunLevel). + Str("format", format). + Str("log_level", level). + Bool("async", cu.async). + Msg("configured zerolog") return nil } } + +// WithPreRunLevel defines the logging level used for pre-run log messages. Debug by default. +func WithPreRunLevel(preRunLevel zerolog.Level) ConfigureFunc { + return func(cu *CobraUtil) { + cu.preRunLevel = preRunLevel + } +} + +// WithAsync enables non-blocking logging. Size of the buffer and polling interval can be configured. +// Disabled by default. +func WithAsync(size int, pollInterval time.Duration) ConfigureFunc { + return func(cu *CobraUtil) { + cu.async = true + cu.asyncSize = size + cu.asyncPollInterval = pollInterval + } +} + +// WithTarget callback that forwards the configured logger. Useful when we want to keep it in a global variable. +func WithTarget(fn func(zerolog.Logger)) ConfigureFunc { + return func(cu *CobraUtil) { + cu.target = fn + } +} From 6eeae4316bddc5a2d8b429f2218621f075d63241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Mon, 26 Sep 2022 16:37:30 +0100 Subject: [PATCH 3/8] moves OTel package to v2 design behaviour should be the same as v1, but: - removes dependency on zerolog and replaces with go-log/logr - allows defining the logger used by otel --- otel/otel.go | 73 ++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 62 insertions(+), 11 deletions(-) diff --git a/otel/otel.go b/otel/otel.go index 522e15d..8f65f42 100644 --- a/otel/otel.go +++ b/otel/otel.go @@ -7,10 +7,9 @@ import ( "runtime/debug" "strings" + "github.com/go-logr/logr" "github.com/jzelinskie/cobrautil/v2" "github.com/jzelinskie/stringz" - "github.com/rs/zerolog" - "github.com/rs/zerolog/log" "github.com/spf13/cobra" "github.com/spf13/pflag" "go.opentelemetry.io/contrib/propagators/b3" @@ -26,6 +25,32 @@ import ( semconv "go.opentelemetry.io/otel/semconv/v1.7.0" ) +// ConfigureFunc is a function used to configure this CobraUtil +type ConfigureFunc = func(cu *CobraUtil) + +// New creates a configuration that exposes RegisterFlags and RunE +// to integrate with cobra +func New(flagPrefix, serviceName string, configurations ...ConfigureFunc) *CobraUtil { + cu := CobraUtil{ + flagPrefix: flagPrefix, + serviceName: serviceName, + preRunLevel: 1, + logger: logr.Discard(), + } + for _, configure := range configurations { + configure(&cu) + } + return &cu +} + +// CobraUtil carries the configuration for a otel CobraRunFunc +type CobraUtil struct { + flagPrefix string + serviceName string + logger logr.Logger + preRunLevel int +} + // RegisterOpenTelemetryFlags adds the following flags for use with // OpenTelemetryPreRunE: // - "$PREFIX-provider" @@ -57,9 +82,27 @@ func RegisterOpenTelemetryFlags(flags *pflag.FlagSet, flagPrefix, serviceName st // corresponding otel provider from a command. // // The required flags can be added to a command by using +// RegisterOpenTelemetryFlags() +func OpenTelemetryRunE(flagPrefix string, preRunLevel int) cobrautil.CobraRunFunc { + return New(flagPrefix, "").RunE() +} + +// RegisterFlags adds the following flags for use with +// OpenTelemetryPreRunE: +// - "$PREFIX-provider" +// - "$PREFIX-endpoint" +// - "$PREFIX-service-name" +func (cu CobraUtil) RegisterFlags(flags *pflag.FlagSet) { + RegisterOpenTelemetryFlags(flags, cu.flagPrefix, cu.serviceName) +} + +// RunE returns a Cobra run func that configures the +// corresponding otel provider from a command. +// +// The required flags can be added to a command by using // RegisterOpenTelemetryFlags(). -func OpenTelemetryRunE(flagPrefix string, prerunLevel zerolog.Level) cobrautil.CobraRunFunc { - prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "otel")) +func (cu CobraUtil) RunE() cobrautil.CobraRunFunc { + prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(cu.flagPrefix, "otel")) return func(cmd *cobra.Command, args []string) error { if cobrautil.IsBuiltinCommand(cmd) { return nil // No-op for builtins @@ -70,6 +113,10 @@ func OpenTelemetryRunE(flagPrefix string, prerunLevel zerolog.Level) cobrautil.C endpoint := cobrautil.MustGetString(cmd, prefixed("endpoint")) insecure := cobrautil.MustGetBool(cmd, prefixed("insecure")) propagators := strings.Split(cobrautil.MustGetString(cmd, prefixed("trace-propagator")), ",") + var noLogger logr.Logger + if cu.logger != noLogger { + otel.SetLogger(cu.logger) + } var exporter trace.SpanExporter var err error @@ -144,17 +191,21 @@ func OpenTelemetryRunE(flagPrefix string, prerunLevel zerolog.Level) cobrautil.C return fmt.Errorf("unknown tracing provider: %s", provider) } - log. - WithLevel(prerunLevel). - Str("provider", provider). - Str("endpoint", endpoint). - Str("service", serviceName). - Bool("insecure", insecure). - Msg("setup opentelemetry tracing") + cu.logger.V(cu.preRunLevel). + Info("setup opentelemetry tracing", "provider", provider, + "endpoint", endpoint, + "service", serviceName, + "insecure", insecure) return nil } } +func WithLogger(logger logr.Logger) ConfigureFunc { + return func(cu *CobraUtil) { + cu.logger = logger + } +} + func initOtelTracer(exporter trace.SpanExporter, serviceName string, propagators []string) error { res, err := resource.New( context.Background(), From 8627e248992b3e239ba8a6d817d78af48576dc52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Mon, 26 Sep 2022 17:53:17 +0100 Subject: [PATCH 4/8] fix linter errors --- example_test.go | 8 ++++---- go.mod | 2 +- grpc/grpc.go | 9 ++++----- http/http.go | 8 ++++---- zerolog/zerolog.go | 4 ++-- 5 files changed, 15 insertions(+), 16 deletions(-) diff --git a/example_test.go b/example_test.go index 5489dc5..d96d2b4 100644 --- a/example_test.go +++ b/example_test.go @@ -13,7 +13,7 @@ func ExampleCommandStack() { Use: "mycmd", PreRunE: cobrautil.CommandStack( cobrautil.SyncViperPreRunE("myprogram"), - zl.ZeroLogRunE("log", zerolog.InfoLevel), + zl.RunE("log", zerolog.InfoLevel), ), } @@ -23,16 +23,16 @@ func ExampleCommandStack() { func ExampleRegisterZeroLogFlags() { cmd := &cobra.Command{ Use: "mycmd", - PreRunE: zl.ZeroLogRunE("log", zerolog.InfoLevel), + PreRunE: zl.RunE("log", zerolog.InfoLevel), } zl.RegisterZeroLogFlags(cmd.PersistentFlags(), "log") } -func ExampleZeroLogRunE() { +func ExampleRunE() { cmd := &cobra.Command{ Use: "mycmd", - PreRunE: zl.ZeroLogRunE("log", zerolog.InfoLevel), + PreRunE: zl.RunE("log", zerolog.InfoLevel), } zl.RegisterZeroLogFlags(cmd.PersistentFlags(), "log") diff --git a/go.mod b/go.mod index a516956..9941b1f 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/jzelinskie/cobrautil/v2 go 1.18 require ( + github.com/go-logr/logr v1.2.3 github.com/jzelinskie/stringz v0.0.1 github.com/mattn/go-isatty v0.0.16 github.com/rs/zerolog v1.28.0 @@ -23,7 +24,6 @@ require ( require ( github.com/cenkalti/backoff/v4 v4.1.3 // indirect github.com/fsnotify/fsnotify v1.5.4 // indirect - github.com/go-logr/logr v1.2.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/golang/protobuf v1.5.2 // indirect github.com/grpc-ecosystem/grpc-gateway/v2 v2.7.0 // indirect diff --git a/grpc/grpc.go b/grpc/grpc.go index dd03267..1ae0ca6 100644 --- a/grpc/grpc.go +++ b/grpc/grpc.go @@ -2,7 +2,6 @@ package grpc import ( "fmt" - "net" "time" @@ -36,9 +35,9 @@ func RegisterGrpcServerFlags(flags *pflag.FlagSet, flagPrefix, serviceName, defa flags.Bool(prefixed("enabled"), defaultEnabled, "enable "+serviceName+" gRPC server") } -// GrpcServerFromFlags creates an *grpc.Server as configured by the flags from +// ServerFromFlags creates an *grpc.Server as configured by the flags from // RegisterGrpcServerFlags(). -func GrpcServerFromFlags(cmd *cobra.Command, flagPrefix string, opts ...grpc.ServerOption) (*grpc.Server, error) { +func ServerFromFlags(cmd *cobra.Command, flagPrefix string, opts ...grpc.ServerOption) (*grpc.Server, error) { prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "grpc")) opts = append(opts, grpc.KeepaliveParams(keepalive.ServerParameters{ @@ -70,9 +69,9 @@ func GrpcServerFromFlags(cmd *cobra.Command, flagPrefix string, opts ...grpc.Ser } } -// GrpcListenFromFlags listens on an gRPC server using the configuration stored +// ListenFromFlags listens on an gRPC server using the configuration stored // in the cobra command that was registered with RegisterGrpcServerFlags. -func GrpcListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *grpc.Server, level zerolog.Level) error { +func ListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *grpc.Server, level zerolog.Level) error { prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "grpc")) if !cobrautil.MustGetBool(cmd, prefixed("enabled")) { diff --git a/http/http.go b/http/http.go index 1c8eaef..c7d84ad 100644 --- a/http/http.go +++ b/http/http.go @@ -30,9 +30,9 @@ func RegisterHTTPServerFlags(flags *pflag.FlagSet, flagPrefix, serviceName, defa flags.Bool(prefixed("enabled"), defaultEnabled, "enable "+serviceName+" http server") } -// HTTPServerFromFlags creates an *http.Server as configured by the flags from +// ServerFromFlags creates an *http.Server as configured by the flags from // RegisterHttpServerFlags(). -func HTTPServerFromFlags(cmd *cobra.Command, flagPrefix string) *http.Server { +func ServerFromFlags(cmd *cobra.Command, flagPrefix string) *http.Server { prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "http")) return &http.Server{ @@ -40,9 +40,9 @@ func HTTPServerFromFlags(cmd *cobra.Command, flagPrefix string) *http.Server { } } -// HTTPListenFromFlags listens on an HTTP server using the configuration stored +// ListenFromFlags listens on an HTTP server using the configuration stored // in the cobra command that was registered with RegisterHttpServerFlags. -func HTTPListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *http.Server, level zerolog.Level) error { +func ListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *http.Server, level zerolog.Level) error { prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "http")) if !cobrautil.MustGetBool(cmd, prefixed("enabled")) { return nil diff --git a/zerolog/zerolog.go b/zerolog/zerolog.go index e7e4d7b..c991960 100644 --- a/zerolog/zerolog.go +++ b/zerolog/zerolog.go @@ -52,12 +52,12 @@ func RegisterZeroLogFlags(flags *pflag.FlagSet, flagPrefix string) { flags.String(prefixed("format"), "auto", `format of logs ("auto", "console", "json")`) } -// ZeroLogRunE returns a Cobra run func that configures the corresponding +// RunE returns a Cobra run func that configures the corresponding // log level from a command. // // The required flags can be added to a command by using // RegisterLoggingPersistentFlags(). -func ZeroLogRunE(flagPrefix string, prerunLevel zerolog.Level) cobrautil.CobraRunFunc { +func RunE(flagPrefix string, prerunLevel zerolog.Level) cobrautil.CobraRunFunc { return New(flagPrefix, WithPreRunLevel(prerunLevel)).RunE() } From 587e82b7fd556d9fa5a5a61deec96bf6fc13b3be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Tue, 27 Sep 2022 09:16:14 +0100 Subject: [PATCH 5/8] also turn flagPrefix into a flag - since it's optional and has a sensible default - also align preRunLevel in OTel with zerolog --- otel/otel.go | 21 +++++++++++++++++---- zerolog/zerolog.go | 12 +++++++++--- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/otel/otel.go b/otel/otel.go index 8f65f42..b1fcfc5 100644 --- a/otel/otel.go +++ b/otel/otel.go @@ -30,9 +30,8 @@ type ConfigureFunc = func(cu *CobraUtil) // New creates a configuration that exposes RegisterFlags and RunE // to integrate with cobra -func New(flagPrefix, serviceName string, configurations ...ConfigureFunc) *CobraUtil { +func New(serviceName string, configurations ...ConfigureFunc) *CobraUtil { cu := CobraUtil{ - flagPrefix: flagPrefix, serviceName: serviceName, preRunLevel: 1, logger: logr.Discard(), @@ -83,8 +82,8 @@ func RegisterOpenTelemetryFlags(flags *pflag.FlagSet, flagPrefix, serviceName st // // The required flags can be added to a command by using // RegisterOpenTelemetryFlags() -func OpenTelemetryRunE(flagPrefix string, preRunLevel int) cobrautil.CobraRunFunc { - return New(flagPrefix, "").RunE() +func OpenTelemetryRunE(flagPrefix, serviceName string, preRunLevel int) cobrautil.CobraRunFunc { + return New(serviceName, WithFlagPrefix(flagPrefix), WithPreRunLevel(preRunLevel)).RunE() } // RegisterFlags adds the following flags for use with @@ -206,6 +205,20 @@ func WithLogger(logger logr.Logger) ConfigureFunc { } } +// WithFlagPrefix defines prefix used with the generated flags. Defaults to "log". +func WithFlagPrefix(flagPrefix string) ConfigureFunc { + return func(cu *CobraUtil) { + cu.flagPrefix = flagPrefix + } +} + +// WithPreRunLevel defines the logging level used for pre-run log messages. Debug by default. +func WithPreRunLevel(preRunLevel int) ConfigureFunc { + return func(cu *CobraUtil) { + cu.preRunLevel = preRunLevel + } +} + func initOtelTracer(exporter trace.SpanExporter, serviceName string, propagators []string) error { res, err := resource.New( context.Background(), diff --git a/zerolog/zerolog.go b/zerolog/zerolog.go index c991960..5587b10 100644 --- a/zerolog/zerolog.go +++ b/zerolog/zerolog.go @@ -22,9 +22,8 @@ type ConfigureFunc = func(cu *CobraUtil) // New creates a configuration that exposes RegisterZeroLogFlags and ZeroLogRunE // to integrate with cobra -func New(flagPrefix string, configurations ...ConfigureFunc) *CobraUtil { +func New(configurations ...ConfigureFunc) *CobraUtil { cu := CobraUtil{ - flagPrefix: flagPrefix, preRunLevel: zerolog.DebugLevel, } for _, configure := range configurations { @@ -58,7 +57,7 @@ func RegisterZeroLogFlags(flags *pflag.FlagSet, flagPrefix string) { // The required flags can be added to a command by using // RegisterLoggingPersistentFlags(). func RunE(flagPrefix string, prerunLevel zerolog.Level) cobrautil.CobraRunFunc { - return New(flagPrefix, WithPreRunLevel(prerunLevel)).RunE() + return New(WithFlagPrefix(flagPrefix), WithPreRunLevel(prerunLevel)).RunE() } // RegisterFlags adds flags for use in with ZeroLogPreRunE: @@ -132,6 +131,13 @@ func (cu CobraUtil) RunE() cobrautil.CobraRunFunc { } } +// WithFlagPrefix defines prefix used with the generated flags. Defaults to "log". +func WithFlagPrefix(flagPrefix string) ConfigureFunc { + return func(cu *CobraUtil) { + cu.flagPrefix = flagPrefix + } +} + // WithPreRunLevel defines the logging level used for pre-run log messages. Debug by default. func WithPreRunLevel(preRunLevel zerolog.Level) ConfigureFunc { return func(cu *CobraUtil) { From 553eadd33199fada4c0682ec8bb1fbd6fdbb3dd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Tue, 27 Sep 2022 11:04:58 +0100 Subject: [PATCH 6/8] refactor http package to follow v2 design - turned all optional arguments into a ConfigureFunc - added WithHandler to provide a http.Handler to inject into ServerFromFlags - added ListenFromFlags that instantiates the http.Server - migrated go go-log/logr facade --- http/http.go | 120 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 108 insertions(+), 12 deletions(-) diff --git a/http/http.go b/http/http.go index c7d84ad..4bb509a 100644 --- a/http/http.go +++ b/http/http.go @@ -5,14 +5,44 @@ import ( "fmt" "net/http" + "github.com/go-logr/logr" "github.com/jzelinskie/cobrautil/v2" "github.com/jzelinskie/stringz" - "github.com/rs/zerolog" - "github.com/rs/zerolog/log" "github.com/spf13/cobra" "github.com/spf13/pflag" ) +// ConfigureFunc is a function used to configure this CobraUtil +type ConfigureFunc = func(cu *CobraUtil) + +// New creates a configuration that exposes RegisterFlags and RunE +// to integrate with cobra +func New(serviceName string, configurations ...ConfigureFunc) *CobraUtil { + cu := CobraUtil{ + serviceName: stringz.DefaultEmpty(serviceName, "http"), + preRunLevel: 1, + logger: logr.Discard(), + defaultAddr: ":8443", + defaultEnabled: false, + flagPrefix: "http", + } + for _, configure := range configurations { + configure(&cu) + } + return &cu +} + +// CobraUtil carries the configuration for a otel CobraRunFunc +type CobraUtil struct { + flagPrefix string + serviceName string + defaultAddr string + defaultEnabled bool + logger logr.Logger + preRunLevel int + handler http.Handler +} + // RegisterHTTPServerFlags adds the following flags for use with // HttpServerFromFlags: // - "$PREFIX-addr" @@ -20,7 +50,6 @@ import ( // - "$PREFIX-tls-key-path" // - "$PREFIX-enabled" func RegisterHTTPServerFlags(flags *pflag.FlagSet, flagPrefix, serviceName, defaultAddr string, defaultEnabled bool) { - serviceName = stringz.DefaultEmpty(serviceName, "http") defaultAddr = stringz.DefaultEmpty(defaultAddr, ":8443") prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "http")) @@ -33,17 +62,40 @@ func RegisterHTTPServerFlags(flags *pflag.FlagSet, flagPrefix, serviceName, defa // ServerFromFlags creates an *http.Server as configured by the flags from // RegisterHttpServerFlags(). func ServerFromFlags(cmd *cobra.Command, flagPrefix string) *http.Server { - prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "http")) + return New("", WithFlagPrefix(flagPrefix)).ServerFromFlags(cmd) +} + +// ListenFromFlags listens on an HTTP server using the configuration stored +// in the cobra command that was registered with RegisterHttpServerFlags. +func ListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *http.Server, preRunLevel int) error { + return New("", WithFlagPrefix(flagPrefix), WithPreRunLevel(preRunLevel)).ListenWithServerFromFlags(cmd, srv) +} + +// RegisterHTTPServerFlags adds the following flags for use with +// HttpServerFromFlags: +// - "$PREFIX-addr" +// - "$PREFIX-tls-cert-path" +// - "$PREFIX-tls-key-path" +// - "$PREFIX-enabled" +func (cu CobraUtil) RegisterHTTPServerFlags(flags *pflag.FlagSet) { + RegisterHTTPServerFlags(flags, cu.flagPrefix, cu.serviceName, cu.defaultAddr, cu.defaultEnabled) +} + +// ServerFromFlags creates an *http.Server as configured by the flags from +// RegisterHttpServerFlags(). +func (cu CobraUtil) ServerFromFlags(cmd *cobra.Command) *http.Server { + prefixed := cobrautil.PrefixJoiner(cu.flagPrefix) return &http.Server{ - Addr: cobrautil.MustGetStringExpanded(cmd, prefixed("addr")), + Addr: cobrautil.MustGetStringExpanded(cmd, prefixed("addr")), + Handler: cu.handler, } } -// ListenFromFlags listens on an HTTP server using the configuration stored +// ListenWithServerFromFlags listens on the provided HTTP server using the configuration stored // in the cobra command that was registered with RegisterHttpServerFlags. -func ListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *http.Server, level zerolog.Level) error { - prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "http")) +func (cu CobraUtil) ListenWithServerFromFlags(cmd *cobra.Command, srv *http.Server) error { + prefixed := cobrautil.PrefixJoiner(cu.flagPrefix) if !cobrautil.MustGetBool(cmd, prefixed("enabled")) { return nil } @@ -53,14 +105,14 @@ func ListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *http.Server, le switch { case certPath == "" && keyPath == "": - log.Warn().Str("addr", srv.Addr).Str("prefix", flagPrefix).Msg("http server serving plaintext") + cu.logger.V(1).Info("http server serving plaintext", "addr", srv.Addr, "prefix", cu.flagPrefix) if err := srv.ListenAndServe(); err != nil && errors.Is(err, http.ErrServerClosed) { return fmt.Errorf("failed while serving http: %w", err) } return nil case certPath != "" && keyPath != "": - log.WithLevel(level).Str("addr", srv.Addr).Str("prefix", flagPrefix).Msg("https server started serving") + cu.logger.V(cu.preRunLevel).Info("https server started serving", "addr", srv.Addr, "prefix", cu.flagPrefix) if err := srv.ListenAndServeTLS(certPath, keyPath); err != nil && errors.Is(err, http.ErrServerClosed) { return fmt.Errorf("failed while serving https: %w", err) } @@ -69,8 +121,52 @@ func ListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *http.Server, le default: return fmt.Errorf( "failed to start http server: must provide both --%s-tls-cert-path and --%s-tls-key-path", - flagPrefix, - flagPrefix, + cu.flagPrefix, + cu.flagPrefix, ) } } + +// WithLogger defines the logger used to log messages in this package +func WithLogger(logger logr.Logger) ConfigureFunc { + return func(cu *CobraUtil) { + cu.logger = logger + } +} + +// WithDefaultAddress defines the default value of the address the server will listen at. +// Defaults to ":8443" +func WithDefaultAddress(addr string) ConfigureFunc { + return func(cu *CobraUtil) { + cu.defaultAddr = addr + } +} + +// WithDefaultEnabled defines whether the http server is enabled by default. Defaults to "false". +func WithDefaultEnabled(enabled bool) ConfigureFunc { + return func(cu *CobraUtil) { + cu.defaultEnabled = enabled + } +} + +// WithFlagPrefix defines prefix used with the generated flags. Defaults to "http". +func WithFlagPrefix(flagPrefix string) ConfigureFunc { + return func(cu *CobraUtil) { + cu.flagPrefix = flagPrefix + } +} + +// WithPreRunLevel defines the logging level used for pre-run log messages. Defaults to "debug".. +func WithPreRunLevel(preRunLevel int) ConfigureFunc { + return func(cu *CobraUtil) { + cu.preRunLevel = preRunLevel + } +} + +// WithHandler defines the HTTP server handler to inject in the http.Server in ServerFromFlags method. +// No handler is set by default. The value will be ignored in ListenFromFlags. +func WithHandler(handler http.Handler) ConfigureFunc { + return func(cu *CobraUtil) { + cu.handler = handler + } +} From d133cf110a51d8f10751610d40da39271477dede Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Tue, 27 Sep 2022 11:31:15 +0100 Subject: [PATCH 7/8] refactor gRPC package to follow v2 design - turned all optional arguments into a ConfigureFunc - migrated go go-log/logr facade --- grpc/grpc.go | 114 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 101 insertions(+), 13 deletions(-) diff --git a/grpc/grpc.go b/grpc/grpc.go index 1ae0ca6..51e106a 100644 --- a/grpc/grpc.go +++ b/grpc/grpc.go @@ -6,9 +6,9 @@ import ( "time" "github.com/jzelinskie/cobrautil/v2" + + "github.com/go-logr/logr" "github.com/jzelinskie/stringz" - "github.com/rs/zerolog" - "github.com/rs/zerolog/log" "github.com/spf13/cobra" "github.com/spf13/pflag" "google.golang.org/grpc" @@ -16,6 +16,36 @@ import ( "google.golang.org/grpc/keepalive" ) +// ConfigureFunc is a function used to configure this CobraUtil +type ConfigureFunc = func(cu *CobraUtil) + +// New creates a configuration that exposes RegisterFlags and RunE +// to integrate with cobra +func New(serviceName string, configurations ...ConfigureFunc) *CobraUtil { + cu := CobraUtil{ + serviceName: stringz.DefaultEmpty(serviceName, "grpc"), + preRunLevel: 1, + logger: logr.Discard(), + defaultAddr: ":50051", + defaultEnabled: false, + flagPrefix: "grpc", + } + for _, configure := range configurations { + configure(&cu) + } + return &cu +} + +// CobraUtil carries the configuration for a otel CobraRunFunc +type CobraUtil struct { + flagPrefix string + serviceName string + defaultAddr string + defaultEnabled bool + logger logr.Logger + preRunLevel int +} + // RegisterGrpcServerFlags adds the following flags for use with // GrpcServerFromFlags: // - "$PREFIX-addr" @@ -35,10 +65,26 @@ func RegisterGrpcServerFlags(flags *pflag.FlagSet, flagPrefix, serviceName, defa flags.Bool(prefixed("enabled"), defaultEnabled, "enable "+serviceName+" gRPC server") } +// RegisterGrpcServerFlags adds the following flags for use with +// GrpcServerFromFlags: +// - "$PREFIX-addr" +// - "$PREFIX-tls-cert-path" +// - "$PREFIX-tls-key-path" +// - "$PREFIX-max-conn-age" +func (cu CobraUtil) RegisterGrpcServerFlags(flags *pflag.FlagSet) { + RegisterGrpcServerFlags(flags, cu.flagPrefix, cu.serviceName, cu.defaultAddr, cu.defaultEnabled) +} + // ServerFromFlags creates an *grpc.Server as configured by the flags from // RegisterGrpcServerFlags(). func ServerFromFlags(cmd *cobra.Command, flagPrefix string, opts ...grpc.ServerOption) (*grpc.Server, error) { - prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "grpc")) + return New("", WithFlagPrefix(flagPrefix)).ServerFromFlags(cmd, opts...) +} + +// ServerFromFlags creates an *grpc.Server as configured by the flags from +// RegisterGrpcServerFlags(). +func (cu CobraUtil) ServerFromFlags(cmd *cobra.Command, opts ...grpc.ServerOption) (*grpc.Server, error) { + prefixed := cobrautil.PrefixJoiner(cu.flagPrefix) opts = append(opts, grpc.KeepaliveParams(keepalive.ServerParameters{ MaxConnectionAge: cobrautil.MustGetDuration(cmd, prefixed("max-conn-age")), @@ -49,7 +95,7 @@ func ServerFromFlags(cmd *cobra.Command, flagPrefix string, opts ...grpc.ServerO switch { case certPath == "" && keyPath == "": - log.Warn().Str("prefix", flagPrefix).Msg("grpc server serving plaintext") + cu.logger.V(cu.preRunLevel).Info("grpc server serving plaintext", "prefix", cu.flagPrefix) return grpc.NewServer(opts...), nil case certPath != "" && keyPath != "": @@ -63,16 +109,22 @@ func ServerFromFlags(cmd *cobra.Command, flagPrefix string, opts ...grpc.ServerO default: return nil, fmt.Errorf( "failed to start gRPC server: must provide both --%s-tls-cert-path and --%s-tls-key-path", - flagPrefix, - flagPrefix, + cu.flagPrefix, + cu.flagPrefix, ) } } // ListenFromFlags listens on an gRPC server using the configuration stored // in the cobra command that was registered with RegisterGrpcServerFlags. -func ListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *grpc.Server, level zerolog.Level) error { - prefixed := cobrautil.PrefixJoiner(stringz.DefaultEmpty(flagPrefix, "grpc")) +func ListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *grpc.Server, preRunLevel int) error { + return New("", WithPreRunLevel(preRunLevel), WithFlagPrefix(flagPrefix)).ListenFromFlags(cmd, srv) +} + +// ListenFromFlags listens on an gRPC server using the configuration stored +// in the cobra command that was registered with RegisterGrpcServerFlags. +func (cu CobraUtil) ListenFromFlags(cmd *cobra.Command, srv *grpc.Server) error { + prefixed := cobrautil.PrefixJoiner(cu.flagPrefix) if !cobrautil.MustGetBool(cmd, prefixed("enabled")) { return nil @@ -86,11 +138,11 @@ func ListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *grpc.Server, le return fmt.Errorf("failed to listen on addr for gRPC server: %w", err) } - log.WithLevel(level). - Str("addr", addr). - Str("network", network). - Str("prefix", flagPrefix). - Msg("grpc server started listening") + cu.logger.V(cu.preRunLevel).Info( + "grpc server started listening", + "addr", addr, + "network", network, + "prefix", cu.flagPrefix) if err := srv.Serve(l); err != nil { return fmt.Errorf("failed to serve gRPC: %w", err) @@ -98,3 +150,39 @@ func ListenFromFlags(cmd *cobra.Command, flagPrefix string, srv *grpc.Server, le return nil } + +// WithLogger defines the logger used to log messages in this package +func WithLogger(logger logr.Logger) ConfigureFunc { + return func(cu *CobraUtil) { + cu.logger = logger + } +} + +// WithDefaultAddress defines the default value of the address the server will listen at. +// Defaults to ":50051" +func WithDefaultAddress(addr string) ConfigureFunc { + return func(cu *CobraUtil) { + cu.defaultAddr = addr + } +} + +// WithDefaultEnabled defines whether the gRPC server is enabled by default. Defaults to "false". +func WithDefaultEnabled(enabled bool) ConfigureFunc { + return func(cu *CobraUtil) { + cu.defaultEnabled = enabled + } +} + +// WithFlagPrefix defines prefix used with the generated flags. Defaults to "grpc". +func WithFlagPrefix(flagPrefix string) ConfigureFunc { + return func(cu *CobraUtil) { + cu.flagPrefix = flagPrefix + } +} + +// WithPreRunLevel defines the logging level used for pre-run log messages. Defaults to "debug". +func WithPreRunLevel(preRunLevel int) ConfigureFunc { + return func(cu *CobraUtil) { + cu.preRunLevel = preRunLevel + } +} From b76e13ce155b7f2626837b6a6cc07da852eb4420 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Tue, 27 Sep 2022 12:34:49 +0100 Subject: [PATCH 8/8] tune logging messages - make sure messaging is consistent - make use of same logging level across packages - do not warn on insecure http/gRPC. Instead load additional key-value - log scheme in http package --- grpc/grpc.go | 20 +++++++++++++++----- http/http.go | 14 +++++++++++--- otel/otel.go | 5 +++-- zerolog/zerolog.go | 5 +++-- 4 files changed, 32 insertions(+), 12 deletions(-) diff --git a/grpc/grpc.go b/grpc/grpc.go index 51e106a..08aae0a 100644 --- a/grpc/grpc.go +++ b/grpc/grpc.go @@ -24,7 +24,7 @@ type ConfigureFunc = func(cu *CobraUtil) func New(serviceName string, configurations ...ConfigureFunc) *CobraUtil { cu := CobraUtil{ serviceName: stringz.DefaultEmpty(serviceName, "grpc"), - preRunLevel: 1, + preRunLevel: 0, logger: logr.Discard(), defaultAddr: ":50051", defaultEnabled: false, @@ -94,11 +94,10 @@ func (cu CobraUtil) ServerFromFlags(cmd *cobra.Command, opts ...grpc.ServerOptio keyPath := cobrautil.MustGetStringExpanded(cmd, prefixed("tls-key-path")) switch { - case certPath == "" && keyPath == "": - cu.logger.V(cu.preRunLevel).Info("grpc server serving plaintext", "prefix", cu.flagPrefix) + case isInsecure(certPath, keyPath): return grpc.NewServer(opts...), nil - case certPath != "" && keyPath != "": + case isSecure(certPath, keyPath): creds, err := credentials.NewServerTLSFromFile(certPath, keyPath) if err != nil { return nil, err @@ -138,11 +137,14 @@ func (cu CobraUtil) ListenFromFlags(cmd *cobra.Command, srv *grpc.Server) error return fmt.Errorf("failed to listen on addr for gRPC server: %w", err) } + certPath := cobrautil.MustGetStringExpanded(cmd, prefixed("tls-cert-path")) + keyPath := cobrautil.MustGetStringExpanded(cmd, prefixed("tls-key-path")) cu.logger.V(cu.preRunLevel).Info( "grpc server started listening", "addr", addr, "network", network, - "prefix", cu.flagPrefix) + "prefix", cu.flagPrefix, + "insecure", isInsecure(certPath, keyPath)) if err := srv.Serve(l); err != nil { return fmt.Errorf("failed to serve gRPC: %w", err) @@ -186,3 +188,11 @@ func WithPreRunLevel(preRunLevel int) ConfigureFunc { cu.preRunLevel = preRunLevel } } + +func isInsecure(certPath, keyPath string) bool { + return certPath == "" && keyPath == "" +} + +func isSecure(certPath, keyPath string) bool { + return certPath != "" && keyPath != "" +} diff --git a/http/http.go b/http/http.go index 4bb509a..b525d73 100644 --- a/http/http.go +++ b/http/http.go @@ -20,7 +20,7 @@ type ConfigureFunc = func(cu *CobraUtil) func New(serviceName string, configurations ...ConfigureFunc) *CobraUtil { cu := CobraUtil{ serviceName: stringz.DefaultEmpty(serviceName, "http"), - preRunLevel: 1, + preRunLevel: 0, logger: logr.Discard(), defaultAddr: ":8443", defaultEnabled: false, @@ -105,14 +105,22 @@ func (cu CobraUtil) ListenWithServerFromFlags(cmd *cobra.Command, srv *http.Serv switch { case certPath == "" && keyPath == "": - cu.logger.V(1).Info("http server serving plaintext", "addr", srv.Addr, "prefix", cu.flagPrefix) + cu.logger.V(cu.preRunLevel).Info("http server started serving", + "addr", srv.Addr, + "prefix", cu.flagPrefix, + "scheme", "http", + "insecure", "true") if err := srv.ListenAndServe(); err != nil && errors.Is(err, http.ErrServerClosed) { return fmt.Errorf("failed while serving http: %w", err) } return nil case certPath != "" && keyPath != "": - cu.logger.V(cu.preRunLevel).Info("https server started serving", "addr", srv.Addr, "prefix", cu.flagPrefix) + cu.logger.V(cu.preRunLevel).Info("http server started serving", + "addr", srv.Addr, + "prefix", cu.flagPrefix, + "scheme", "https", + "insecure", "false") if err := srv.ListenAndServeTLS(certPath, keyPath); err != nil && errors.Is(err, http.ErrServerClosed) { return fmt.Errorf("failed while serving https: %w", err) } diff --git a/otel/otel.go b/otel/otel.go index b1fcfc5..7b075fa 100644 --- a/otel/otel.go +++ b/otel/otel.go @@ -33,7 +33,7 @@ type ConfigureFunc = func(cu *CobraUtil) func New(serviceName string, configurations ...ConfigureFunc) *CobraUtil { cu := CobraUtil{ serviceName: serviceName, - preRunLevel: 1, + preRunLevel: 0, logger: logr.Discard(), } for _, configure := range configurations { @@ -191,7 +191,8 @@ func (cu CobraUtil) RunE() cobrautil.CobraRunFunc { } cu.logger.V(cu.preRunLevel). - Info("setup opentelemetry tracing", "provider", provider, + Info("configured opentelemetry tracing", + "provider", provider, "endpoint", endpoint, "service", serviceName, "insecure", insecure) diff --git a/zerolog/zerolog.go b/zerolog/zerolog.go index 5587b10..4ba35c9 100644 --- a/zerolog/zerolog.go +++ b/zerolog/zerolog.go @@ -24,7 +24,7 @@ type ConfigureFunc = func(cu *CobraUtil) // to integrate with cobra func New(configurations ...ConfigureFunc) *CobraUtil { cu := CobraUtil{ - preRunLevel: zerolog.DebugLevel, + preRunLevel: zerolog.InfoLevel, } for _, configure := range configurations { configure(&cu) @@ -125,8 +125,9 @@ func (cu CobraUtil) RunE() cobrautil.CobraRunFunc { l.WithLevel(cu.preRunLevel). Str("format", format). Str("log_level", level). + Str("provider", "zerolog"). Bool("async", cu.async). - Msg("configured zerolog") + Msg("configured logging") return nil } }