From be7aa750bcca67cd45b18ec57a0e6666f904ac1c Mon Sep 17 00:00:00 2001 From: Gerry Agbobada <10496163+gagbo@users.noreply.github.com> Date: Thu, 16 Nov 2023 13:09:29 +0100 Subject: [PATCH] Add Logging interface to Autometrics (#80) --- CHANGELOG.md | 3 ++ README.md | 33 +++++++++++++++ examples/otel/cmd/main.go | 1 + examples/web/cmd/main.go | 1 + examples/web/cmd/main.go.orig | 1 + internal/generate/context.go | 4 +- otel/autometrics/otel.go | 24 +++++++++-- pkg/autometrics/ctx.go | 6 ++- pkg/autometrics/global_state.go | 13 ++++++ pkg/autometrics/log/log.go | 63 ++++++++++++++++++++++++++++ prometheus/autometrics/prometheus.go | 22 ++++++++-- 11 files changed, 159 insertions(+), 12 deletions(-) create mode 100644 pkg/autometrics/log/log.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d5fe324..61a10c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,9 @@ specification. - [Generator] The generator now tries to keep going with instrumentation even if some instrumentation fails. On error, still _No file will be modified_, and the generator will exit with an error code and output all the collected errors instead of only the first one. +- [All] `autometrics.Init` function takes an additional argument, a logging interface to decide whether + and how users want autometrics to log events. Passing `nil` for the argument value will use a "No Op" + logger that does not do anything. ### Deprecated diff --git a/README.md b/README.md index a4e3f54..f0bc40c 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,7 @@ And then in your main function initialize the metrics autometrics.DefBuckets, autometrics.BuildInfo{Version: "0.4.0", Commit: "anySHA", Branch: "", Service: "myApp"}, nil, + nil, ) if err != nil { log.Fatalf("could not initialize autometrics: %s", err) @@ -273,6 +274,7 @@ func main() { autometrics.DefBuckets, autometrics.BuildInfo{Version: "0.4.0", Commit: "anySHA", Branch: "", Service: "myApp"}, nil, + nil, ) http.Handle("/metrics", promhttp.Handler()) } @@ -384,6 +386,7 @@ metric. You can use the name of the application or its version for example autometrics.DefBuckets, autometrics.BuildInfo{ Version: "2.1.37", Commit: "anySHA", Branch: "", Service: "myApp" }, nil, + nil, ) ``` @@ -447,6 +450,7 @@ passing a non `nil` argument to `autometrics.Init` for the `pushConfiguration`: + Period: 1 * time.Second, // Period is only relevant when using OpenTelemetry implementation + Timeout: 500 * time.Millisecond, // Timeout is only relevant when using OpenTelementry implementation + }, + nil, ) ``` @@ -456,6 +460,35 @@ can contact us so we can setup a managed instance of Prometheus for you. We will give you collector URLs, that will work with both OpenTelemetry and Prometheus; and can be visualized easily with our explorer as well! +#### Logging + +Monitoring/Observability must not crash the application. + +So when Autometrics encounters +errors, instead of bubbling it up until the program stops, it will log the error and absorb it. +To leave choice in the logging implementation (depending on your application dependencies), +Autometrics exposes a `Logger` interface you can implement and then inject in the `Init` call +to have the logging you want. + +The `Logger` interface is a subset of `slog.Logger` methods, so that most loggers can be used. +Autometrics also provides 2 simple loggers out of the box: +- `NoOpLogger`, which is the default logger and does nothing, +- `PrintLogger` which uses `fmt.Print` to write logging messages on stdout. + +To use the `PrintLogger` instead of the `NoOpLogger` for examble, you just have to change +the `Init` call: + +``` patch + shutdown, err := autometrics.Init( + nil, + autometrics.DefBuckets, + autometrics.BuildInfo{ Version: "2.1.37", Commit: "anySHA", Branch: "", Service: "myApp" }, + nil, +- nil, ++ autometrics.PrintLogger{}, + ) +``` + #### Git hook As autometrics is a Go generator that modifies the source code when run, it diff --git a/examples/otel/cmd/main.go b/examples/otel/cmd/main.go index 60a63ed..90c346d 100644 --- a/examples/otel/cmd/main.go +++ b/examples/otel/cmd/main.go @@ -51,6 +51,7 @@ func main() { Branch: Branch, }, pushConfiguration, + autometrics.PrintLogger{}, ) if err != nil { log.Fatalf("Failed initialization of autometrics: %s", err) diff --git a/examples/web/cmd/main.go b/examples/web/cmd/main.go index 99e0485..193e5c6 100644 --- a/examples/web/cmd/main.go +++ b/examples/web/cmd/main.go @@ -50,6 +50,7 @@ func main() { Branch: Branch, }, pushConfiguration, + autometrics.PrintLogger{}, ) if err != nil { log.Fatalf("Failed initialization of autometrics: %s", err) diff --git a/examples/web/cmd/main.go.orig b/examples/web/cmd/main.go.orig index a93f423..89adbc6 100644 --- a/examples/web/cmd/main.go.orig +++ b/examples/web/cmd/main.go.orig @@ -50,6 +50,7 @@ func main() { Service: "autometrics-go-example-prometheus" }, pushConfiguration, + autometrics.PrintLogger{}, ) if err != nil { log.Fatalf("Failed initialization of autometrics: %s", err) diff --git a/internal/generate/context.go b/internal/generate/context.go index 1d220be..5e57e4d 100644 --- a/internal/generate/context.go +++ b/internal/generate/context.go @@ -320,9 +320,7 @@ func detectContextSelectorImpl(ctx *internal.GeneratorContext, argName string, s } } } else { - // TODO: log that autometrics cannot detect multi-nested contexts instead of errorring - // continue - return true, fmt.Errorf("expecting parent to be an identifier, got %s instead", reflect.TypeOf(selector.X).String()) + am.GetLogger().Error("expecting parent to be an identifier, got %s instead", reflect.TypeOf(selector.X).String()) } return false, nil } diff --git a/otel/autometrics/otel.go b/otel/autometrics/otel.go index dea2f4d..c404401 100644 --- a/otel/autometrics/otel.go +++ b/otel/autometrics/otel.go @@ -4,12 +4,12 @@ import ( "context" "errors" "fmt" - "log" "os" "sync" "time" "github.com/autometrics-dev/autometrics-go/pkg/autometrics" + "github.com/autometrics-dev/autometrics-go/pkg/autometrics/log" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc" @@ -120,6 +120,17 @@ func completeMeterName(meterName string) string { // the current (otel) package imported at the call site. type BuildInfo = autometrics.BuildInfo +// Logger is an interface for logging autometrics-related events. +// +// This is a reexport to allow using only the current package at call site. +type Logger = log.Logger + +// This is a reexport to allow using only the current package at call site. +type PrintLogger = log.PrintLogger + +// This is a reexport to allow using only the current package at call site. +type NoOpLogger = log.NoOpLogger + // PushConfiguration holds meta information about the push-to-collector configuration of the instrumented code. type PushConfiguration struct { // URL of the collector to push to. It must be non-empty if this struct is built. @@ -180,7 +191,7 @@ type PushConfiguration struct { // Make sure that all the latency targets you want to use for SLOs are // present in the histogramBuckets array, otherwise the alerts will fail // to work (they will never trigger). -func Init(meterName string, histogramBuckets []float64, buildInformation BuildInfo, pushConfiguration *PushConfiguration) (context.CancelCauseFunc, error) { +func Init(meterName string, histogramBuckets []float64, buildInformation BuildInfo, pushConfiguration *PushConfiguration, logger log.Logger) (context.CancelCauseFunc, error) { var err error newCtx, cancelFunc := context.WithCancelCause(context.Background()) amCtx = newCtx @@ -188,6 +199,11 @@ func Init(meterName string, histogramBuckets []float64, buildInformation BuildIn autometrics.SetCommit(buildInformation.Commit) autometrics.SetVersion(buildInformation.Version) autometrics.SetBranch(buildInformation.Branch) + if logger == nil { + autometrics.SetLogger(log.NoOpLogger{}) + } else { + autometrics.SetLogger(logger) + } var pushExporter metric.Exporter if pushConfiguration != nil { @@ -334,7 +350,7 @@ func initProvider(pushExporter metric.Exporter, pushConfiguration *PushConfigura metric.WithResource(autometricsSrc), ), nil } else { - log.Printf("autometrics: opentelemetry: setting up OTLP push configuration, pushing %s to %s\n", + autometrics.GetLogger().Debug("opentelemetry: setting up OTLP push configuration, pushing %s to %s\n", autometrics.GetPushJobName(), autometrics.GetPushJobURL(), ) @@ -368,7 +384,7 @@ func initProvider(pushExporter metric.Exporter, pushConfiguration *PushConfigura } func initPushExporter(pushConfiguration *PushConfiguration) (metric.Exporter, error) { - log.Println("autometrics: opentelemetry: Init: detected push configuration") + autometrics.GetLogger().Debug("opentelemetry: Init: detected push configuration") if pushConfiguration.CollectorURL == "" { return nil, errors.New("invalid PushConfiguration: the CollectorURL must be set.") } diff --git a/pkg/autometrics/ctx.go b/pkg/autometrics/ctx.go index 73cb129..8425aba 100644 --- a/pkg/autometrics/ctx.go +++ b/pkg/autometrics/ctx.go @@ -269,8 +269,10 @@ func FillTracingAndCallerInfo(ctx context.Context) context.Context { // NOTE: This also means that goroutines that outlive their as the caller will not have access to parent // caller information, but hopefully by that point we got all the necessary accesses done. // If not, it is a convenience we accept to give up to prevent memory usage from exploding. - // TODO: once settled on a login library, log the error instead of ignoring it - _ = PushFunctionName(ctx, callInfo.Current) + err := PushFunctionName(ctx, callInfo.Current) + if err != nil { + GetLogger().Error("adding a function name to the known spans: %s", err) + } return ctx } diff --git a/pkg/autometrics/global_state.go b/pkg/autometrics/global_state.go index 88f5c71..0501880 100644 --- a/pkg/autometrics/global_state.go +++ b/pkg/autometrics/global_state.go @@ -2,6 +2,8 @@ package autometrics // import "github.com/autometrics-dev/autometrics-go/pkg/aut import ( "fmt" + + "github.com/autometrics-dev/autometrics-go/pkg/autometrics/log" ) // These variables are describing the state of the application being autometricized, @@ -36,6 +38,7 @@ var ( pushJobName string pushJobURL string instrumentedSpans map[spanKey]FunctionID = make(map[spanKey]FunctionID) + logger log.Logger ) type spanKey struct { @@ -43,6 +46,16 @@ type spanKey struct { sid SpanID } +// GetLogger returns the current logging interface for Autometrics +func GetLogger() log.Logger { + return logger +} + +// SetLogger sets the logging interface for Autometrics. +func SetLogger(newLogger log.Logger) { + logger = newLogger +} + // GetVersion returns the version of the codebase being instrumented. func GetVersion() string { return version diff --git a/pkg/autometrics/log/log.go b/pkg/autometrics/log/log.go new file mode 100644 index 0000000..ac2cb17 --- /dev/null +++ b/pkg/autometrics/log/log.go @@ -0,0 +1,63 @@ +package log // import "github.com/autometrics-dev/autometrics-go/pkg/autometrics/log" + +import ( + "context" + "fmt" +) + +// Logger is an interface to implement to be able to inject a logger to Autometrics. +// The interface follows the interface of slog.Logger +type Logger interface { + Debug(msg string, args ...any) + DebugContext(ctx context.Context, msg string, args ...any) + Info(msg string, args ...any) + InfoContext(ctx context.Context, msg string, args ...any) + Warn(msg string, args ...any) + WarnContext(ctx context.Context, msg string, args ...any) + Error(msg string, args ...any) + ErrorContext(ctx context.Context, msg string, args ...any) +} + +// NoOpLogger is the default logger for Autometrics. It does nothing. +type NoOpLogger struct{} + +var _ Logger = NoOpLogger{} + +func (_ NoOpLogger) Debug(msg string, args ...any) {} +func (_ NoOpLogger) DebugContext(ctx context.Context, msg string, args ...any) {} +func (_ NoOpLogger) Info(msg string, args ...any) {} +func (_ NoOpLogger) InfoContext(ctx context.Context, msg string, args ...any) {} +func (_ NoOpLogger) Warn(msg string, args ...any) {} +func (_ NoOpLogger) WarnContext(ctx context.Context, msg string, args ...any) {} +func (_ NoOpLogger) Error(msg string, args ...any) {} +func (_ NoOpLogger) ErrorContext(ctx context.Context, msg string, args ...any) {} + +// PrintLogger is a simple logger implementation that simply prints the events to stdout +type PrintLogger struct{} + +var _ Logger = PrintLogger{} + +func (_ PrintLogger) Debug(msg string, args ...any) { + fmt.Printf("Autometrics - Debug: %v", fmt.Sprintf(msg, args...)) +} +func (_ PrintLogger) DebugContext(ctx context.Context, msg string, args ...any) { + fmt.Printf("Autometrics - Debug: %v", fmt.Sprintf(msg, args...)) +} +func (_ PrintLogger) Info(msg string, args ...any) { + fmt.Printf("Autometrics - Info: %v", fmt.Sprintf(msg, args...)) +} +func (_ PrintLogger) InfoContext(ctx context.Context, msg string, args ...any) { + fmt.Printf("Autometrics - Info: %v", fmt.Sprintf(msg, args...)) +} +func (_ PrintLogger) Warn(msg string, args ...any) { + fmt.Printf("Autometrics - Warn: %v", fmt.Sprintf(msg, args...)) +} +func (_ PrintLogger) WarnContext(ctx context.Context, msg string, args ...any) { + fmt.Printf("Autometrics - Warn: %v", fmt.Sprintf(msg, args...)) +} +func (_ PrintLogger) Error(msg string, args ...any) { + fmt.Printf("Autometrics - Error: %v", fmt.Sprintf(msg, args...)) +} +func (_ PrintLogger) ErrorContext(ctx context.Context, msg string, args ...any) { + fmt.Printf("Autometrics - Error: %v", fmt.Sprintf(msg, args...)) +} diff --git a/prometheus/autometrics/prometheus.go b/prometheus/autometrics/prometheus.go index 73414d6..6f763e6 100644 --- a/prometheus/autometrics/prometheus.go +++ b/prometheus/autometrics/prometheus.go @@ -4,11 +4,11 @@ import ( "context" "errors" "fmt" - "log" "os" "sync" "github.com/autometrics-dev/autometrics-go/pkg/autometrics" + "github.com/autometrics-dev/autometrics-go/pkg/autometrics/log" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/push" "github.com/prometheus/common/expfmt" @@ -105,6 +105,17 @@ const ( // the current (prometheus) package imported at the call site. type BuildInfo = autometrics.BuildInfo +// Logger is an interface for logging autometrics-related events. +// +// This is a reexport to allow using only the current package at call site. +type Logger = log.Logger + +// This is a reexport to allow using only the current package at call site. +type PrintLogger = log.PrintLogger + +// This is a reexport to allow using only the current package at call site. +type NoOpLogger = log.NoOpLogger + // PushConfiguration holds meta information about the push-to-collector configuration of the instrumented code. // // This is a reexport of the autometrics type to allow [Init] to work with only @@ -131,17 +142,22 @@ type PushConfiguration = autometrics.PushConfiguration // Make sure that all the latency targets you want to use for SLOs are // present in the histogramBuckets array, otherwise the alerts will fail // to work (they will never trigger.) -func Init(reg *prometheus.Registry, histogramBuckets []float64, buildInformation BuildInfo, pushConfiguration *PushConfiguration) (context.CancelCauseFunc, error) { +func Init(reg *prometheus.Registry, histogramBuckets []float64, buildInformation BuildInfo, pushConfiguration *PushConfiguration, logger log.Logger) (context.CancelCauseFunc, error) { newCtx, cancelFunc := context.WithCancelCause(context.Background()) amCtx = newCtx autometrics.SetCommit(buildInformation.Commit) autometrics.SetVersion(buildInformation.Version) autometrics.SetBranch(buildInformation.Branch) + if logger == nil { + autometrics.SetLogger(log.NoOpLogger{}) + } else { + autometrics.SetLogger(logger) + } pusher = nil if pushConfiguration != nil { - log.Printf("autometrics: Init: detected push configuration to %s", pushConfiguration.CollectorURL) + autometrics.GetLogger().Debug("Init: detected push configuration to %s", pushConfiguration.CollectorURL) if pushConfiguration.CollectorURL == "" { return nil, errors.New("invalid PushConfiguration: the CollectorURL must be set.")