diff --git a/controllers/redirect_controller.go b/controllers/redirect_controller.go index 8f0eaf3..911ebeb 100644 --- a/controllers/redirect_controller.go +++ b/controllers/redirect_controller.go @@ -22,6 +22,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" networkingv1 "k8s.io/api/networking/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -33,7 +34,6 @@ import ( redirectclient "github.com/cedi/urlshortener/pkg/client" "github.com/cedi/urlshortener/pkg/observability" redirectpkg "github.com/cedi/urlshortener/pkg/redirect" - "github.com/go-logr/logr" "github.com/pkg/errors" ) @@ -43,17 +43,17 @@ type RedirectReconciler struct { rClient *redirectclient.RedirectClient scheme *runtime.Scheme - log *logr.Logger + zapLog *zap.Logger tracer trace.Tracer } // NewRedirectReconciler returns a new RedirectReconciler -func NewRedirectReconciler(client client.Client, rClient *redirectclient.RedirectClient, scheme *runtime.Scheme, log *logr.Logger, tracer trace.Tracer) *RedirectReconciler { +func NewRedirectReconciler(client client.Client, rClient *redirectclient.RedirectClient, scheme *runtime.Scheme, zapLog *zap.Logger, tracer trace.Tracer) *RedirectReconciler { return &RedirectReconciler{ client: client, rClient: rClient, scheme: scheme, - log: log, + zapLog: zapLog, tracer: tracer, } } @@ -86,7 +86,7 @@ func (r *RedirectReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c span.SetAttributes(attribute.String("redirect", req.NamespacedName.String())) - log := r.log.WithName("reconciler").WithValues("redirect", req.NamespacedName) + log := r.zapLog.Sugar().With(zap.String("name", "reconciler"), zap.String("redirect", req.NamespacedName.String())) // Monitor the number of redirects if redirectList, err := r.rClient.List(ctx); redirectList != nil && err == nil { @@ -100,19 +100,19 @@ func (r *RedirectReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c // Request object not found, could have been deleted after reconcile request. // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. // Return and don't requeue - observability.RecordInfo(span, &log, "Shortlink resource not found. Ignoring since object must be deleted") + observability.RecordInfo(span, log, "Shortlink resource not found. Ignoring since object must be deleted") return ctrl.Result{}, nil } // Error reading the object - requeue the request. - observability.RecordError(span, &log, err, "Failed to fetch Redirect resource") + observability.RecordError(span, log, err, "Failed to fetch Redirect resource") return ctrl.Result{}, err } // Check if the ingress already exists, if not create a new one ingress, err := r.upsertRedirectIngress(ctx, redirect) if err != nil { - observability.RecordError(span, &log, err, "Failed to upsert redirect ingress") + observability.RecordError(span, log, err, "Failed to upsert redirect ingress") } // Update the Redirect status with the ingress name and the target @@ -123,7 +123,7 @@ func (r *RedirectReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } if err = r.client.List(ctx, ingressList, listOpts...); err != nil { - observability.RecordError(span, &log, err, "Failed to list ingresses") + observability.RecordError(span, log, err, "Failed to list ingresses") return ctrl.Result{}, err } @@ -132,7 +132,7 @@ func (r *RedirectReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c redirect.Status.Target = ingress.ObjectMeta.Annotations["nginx.ingress.kubernetes.io/permanent-redirect"] err = r.client.Status().Update(ctx, redirect) if err != nil { - observability.RecordError(span, &log, err, "Failed to update Redirect status") + observability.RecordError(span, log, err, "Failed to update Redirect status") return ctrl.Result{}, err } diff --git a/controllers/shortlink_controller.go b/controllers/shortlink_controller.go index c7ea8ea..cc3e1d4 100644 --- a/controllers/shortlink_controller.go +++ b/controllers/shortlink_controller.go @@ -22,6 +22,7 @@ import ( "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" @@ -30,23 +31,22 @@ import ( v1alpha1 "github.com/cedi/urlshortener/api/v1alpha1" shortlinkclient "github.com/cedi/urlshortener/pkg/client" "github.com/cedi/urlshortener/pkg/observability" - "github.com/go-logr/logr" ) // ShortLinkReconciler reconciles a ShortLink object type ShortLinkReconciler struct { client *shortlinkclient.ShortlinkClient scheme *runtime.Scheme - log *logr.Logger + zapLog *zap.Logger tracer trace.Tracer } // NewShortLinkReconciler returns a new ShortLinkReconciler -func NewShortLinkReconciler(client *shortlinkclient.ShortlinkClient, scheme *runtime.Scheme, log *logr.Logger, tracer trace.Tracer) *ShortLinkReconciler { +func NewShortLinkReconciler(client *shortlinkclient.ShortlinkClient, scheme *runtime.Scheme, zapLog *zap.Logger, tracer trace.Tracer) *ShortLinkReconciler { return &ShortLinkReconciler{ client: client, scheme: scheme, - log: log, + zapLog: zapLog, tracer: tracer, } } @@ -76,15 +76,15 @@ func (r *ShortLinkReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( span.SetAttributes(attribute.String("shortlink", req.Name)) - log := r.log.WithName("reconciler").WithValues("shortlink", req.NamespacedName.String()) + log := r.zapLog.Sugar().With(zap.String("name", "reconciler"), zap.String("shortlink", req.NamespacedName.String())) // Get ShortLink from etcd shortlink, err := r.client.GetNamespaced(ctx, req.NamespacedName) if err != nil || shortlink == nil { if errors.IsNotFound(err) { - observability.RecordInfo(span, &log, "Shortlink resource not found. Ignoring since object must be deleted") + observability.RecordInfo(span, log, "Shortlink resource not found. Ignoring since object must be deleted") } else { - observability.RecordError(span, &log, err, "Failed to fetch ShortLink resource") + observability.RecordError(span, log, err, "Failed to fetch ShortLink resource") } } diff --git a/go.mod b/go.mod index 66bee27..5cd3feb 100644 --- a/go.mod +++ b/go.mod @@ -13,12 +13,12 @@ require ( github.com/swaggo/gin-swagger v1.5.3 github.com/swaggo/swag v1.8.10 go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin v0.39.0 - go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.40.0 - go.opentelemetry.io/otel v1.15.1 + go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0 + go.opentelemetry.io/otel v1.16.0 go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.15.1 go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.15.1 go.opentelemetry.io/otel/sdk v1.15.1 - go.opentelemetry.io/otel/trace v1.15.1 + go.opentelemetry.io/otel/trace v1.16.0 golang.org/x/exp v0.0.0-20230304125523-9ff063c70017 k8s.io/api v0.26.2 k8s.io/apimachinery v0.26.2 @@ -28,7 +28,7 @@ require ( require ( github.com/felixge/httpsnoop v1.0.3 // indirect - go.opentelemetry.io/otel/metric v0.37.0 // indirect + go.opentelemetry.io/otel/metric v1.16.0 // indirect ) require ( @@ -83,10 +83,12 @@ require ( github.com/spf13/pflag v1.0.5 // indirect github.com/twitchyliquid64/golang-asm v0.15.1 // indirect github.com/ugorji/go/codec v1.2.9 // indirect + github.com/uptrace/opentelemetry-go-extra/otelutil v0.2.1 // indirect + github.com/uptrace/opentelemetry-go-extra/otelzap v0.2.1 // indirect go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.15.1 // indirect go.opentelemetry.io/proto/otlp v0.19.0 // indirect - go.uber.org/atomic v1.10.0 // indirect - go.uber.org/multierr v1.9.0 // indirect + go.uber.org/atomic v1.11.0 // indirect + go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.24.0 // indirect golang.org/x/arch v0.0.0-20210923205945-b76863e36670 // indirect golang.org/x/crypto v0.6.0 // indirect diff --git a/go.sum b/go.sum index 46ca070..3271fb7 100644 --- a/go.sum +++ b/go.sum @@ -387,6 +387,10 @@ github.com/ugorji/go v1.2.7/go.mod h1:nF9osbDWLy6bDVv/Rtoh6QgnvNDpmCalQV5urGCCS6 github.com/ugorji/go/codec v1.2.7/go.mod h1:WGN1fab3R1fzQlVQTkfxVtIBhWDRqOviHU95kRgeqEY= github.com/ugorji/go/codec v1.2.9 h1:rmenucSohSTiyL09Y+l2OCk+FrMxGMzho2+tjr5ticU= github.com/ugorji/go/codec v1.2.9/go.mod h1:UNopzCgEMSXjBc6AOMqYvWC1ktqTAfzJZUZgYf6w6lg= +github.com/uptrace/opentelemetry-go-extra/otelutil v0.2.1 h1:qjljyY//UH064+gQDHh5U7M1Jh6b+iQpJUWVAuRJ04A= +github.com/uptrace/opentelemetry-go-extra/otelutil v0.2.1/go.mod h1:7YSrHCmYPHIXjTWnKSU7EGT0TFEcm3WwSeQquwCGg38= +github.com/uptrace/opentelemetry-go-extra/otelzap v0.2.1 h1:HhKd/kmL1JuBK3zPr3gT/Ku7lvvBsnsy8NtQ+uG5rRM= +github.com/uptrace/opentelemetry-go-extra/otelzap v0.2.1/go.mod h1:GiIWZ+UVlFnAik/QCTc7TjsLA+YV1m94ls3G1Q7fKRY= github.com/urfave/cli/v2 v2.3.0/go.mod h1:LJmUH05zAU44vOAcrfzZQKsZbVcdbOG8rtL3/XcUArI= github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f/go.mod h1:N2zxlSyiKSe5eX1tZViRH5QA0qijqEDrYZiPEAiq3wU= github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415/go.mod h1:GwrjFmJcFw6At/Gs6z4yjiIwzuJ1/+UwLxMQDVQXShQ= @@ -406,10 +410,14 @@ go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin v0. go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin v0.39.0/go.mod h1:dbx2pPD/jZWsnCz7ogHKY2mmHHnRU4bkjOVsw1V8x/o= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.40.0 h1:lE9EJyw3/JhrjWH/hEy9FptnalDQgj7vpbgC2KCCCxE= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.40.0/go.mod h1:pcQ3MM3SWvrA71U4GDqv9UFDJ3HQsW7y5ZO3tDTlUdI= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0 h1:pginetY7+onl4qN1vl0xW/V/v6OBZ0vVdH+esuJgvmM= +go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.42.0/go.mod h1:XiYsayHc36K3EByOO6nbAXnAWbrUxdjUROCEeeROOH8= go.opentelemetry.io/contrib/propagators/b3 v1.14.0 h1:0SBc35DESy/YXShxFtu3634OwcEWJoGzSA8Hx/NbOo8= go.opentelemetry.io/otel v1.7.0/go.mod h1:5BdUoMIz5WEs0vt0CUEMtSSaTSHBBVwrhnz7+nrD5xk= go.opentelemetry.io/otel v1.15.1 h1:3Iwq3lfRByPaws0f6bU3naAqOR1n5IeDWd9390kWHa8= go.opentelemetry.io/otel v1.15.1/go.mod h1:mHHGEHVDLal6YrKMmk9LqC4a3sF5g+fHfrttQIB1NTc= +go.opentelemetry.io/otel v1.16.0 h1:Z7GVAX/UkAXPKsy94IU+i6thsQS4nb7LviLpnaNeW8s= +go.opentelemetry.io/otel v1.16.0/go.mod h1:vl0h9NUa1D5s1nv3A5vZOYWn8av4K8Ml6JDeHrT/bx4= go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.15.1 h1:XYDQtNzdb2T4uM1pku2m76eSMDJgqhJ+6KzkqgQBALc= go.opentelemetry.io/otel/exporters/otlp/internal/retry v1.15.1/go.mod h1:uOTV75+LOzV+ODmL8ahRLWkFA3eQcSC2aAsbxIu4duk= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.15.1 h1:tyoeaUh8REKay72DVYsSEBYV18+fGONe+YYPaOxgLoE= @@ -418,23 +426,31 @@ go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.15.1 h1:pnJfH go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.15.1/go.mod h1:cC3Eu2V56zXY09YlijmqDhOUnL2jVL6KKJg4PGh++dU= go.opentelemetry.io/otel/metric v0.37.0 h1:pHDQuLQOZwYD+Km0eb657A25NaRzy0a+eLyKfDXedEs= go.opentelemetry.io/otel/metric v0.37.0/go.mod h1:DmdaHfGt54iV6UKxsV9slj2bBRJcKC1B1uvDLIioc1s= +go.opentelemetry.io/otel/metric v1.16.0 h1:RbrpwVG1Hfv85LgnZ7+txXioPDoh6EdbZHo26Q3hqOo= +go.opentelemetry.io/otel/metric v1.16.0/go.mod h1:QE47cpOmkwipPiefDwo2wDzwJrlfxxNYodqc4xnGCo4= go.opentelemetry.io/otel/sdk v1.7.0/go.mod h1:uTEOTwaqIVuTGiJN7ii13Ibp75wJmYUDe374q6cZwUU= go.opentelemetry.io/otel/sdk v1.15.1 h1:5FKR+skgpzvhPQHIEfcwMYjCBr14LWzs3uSqKiQzETI= go.opentelemetry.io/otel/sdk v1.15.1/go.mod h1:8rVtxQfrbmbHKfqzpQkT5EzZMcbMBwTzNAggbEAM0KA= go.opentelemetry.io/otel/trace v1.7.0/go.mod h1:fzLSB9nqR2eXzxPXb2JW9IKE+ScyXA48yyE4TNvoHqU= go.opentelemetry.io/otel/trace v1.15.1 h1:uXLo6iHJEzDfrNC0L0mNjItIp06SyaBQxu5t3xMlngY= go.opentelemetry.io/otel/trace v1.15.1/go.mod h1:IWdQG/5N1x7f6YUlmdLeJvH9yxtuJAfc4VW5Agv9r/8= +go.opentelemetry.io/otel/trace v1.16.0 h1:8JRpaObFoW0pxuVPapkgH8UhHQj+bJW8jJsCZEu5MQs= +go.opentelemetry.io/otel/trace v1.16.0/go.mod h1:Yt9vYq1SdNz3xdjZZK7wcXv1qv2pwLkqr2QVwea0ef0= go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI= go.opentelemetry.io/proto/otlp v0.19.0 h1:IVN6GR+mhC4s5yfcTbmzHYODqvWAp3ZedA2SJPI1Nnw= go.opentelemetry.io/proto/otlp v0.19.0/go.mod h1:H7XAot3MsfNsj7EXtrA2q5xSNQ10UqI405h3+duxN4U= go.uber.org/atomic v1.7.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= go.uber.org/atomic v1.10.0 h1:9qC72Qh0+3MqyJbAn8YU5xVq1frD8bn3JtD2oXtafVQ= go.uber.org/atomic v1.10.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= +go.uber.org/atomic v1.11.0 h1:ZvwS0R+56ePWxUNi+Atn9dWONBPp/AUETXlHW0DxSjE= +go.uber.org/atomic v1.11.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= go.uber.org/goleak v1.2.0 h1:xqgm/S+aQvhWFTtR0XK3Jvg7z8kGV8P4X14IzwN3Eqk= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI= go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ= +go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= +go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.19.0/go.mod h1:xg/QME4nWcxGxrpdeYfq7UvYrLh66cuVKdrbD1XF/NI= go.uber.org/zap v1.24.0 h1:FiJd5l1UOLj0wCgbSE0rwwXHzEdAZS6hiiSnxJN/D60= go.uber.org/zap v1.24.0/go.mod h1:2kMP+WWQ8aoFoedH3T2sq6iJ2yDWpHbP0f6MQbS9Gkg= diff --git a/main.go b/main.go index 746cc77..cd38582 100644 --- a/main.go +++ b/main.go @@ -27,16 +27,16 @@ import ( // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) // to ensure that exec-entrypoint and run can make use of them. + "go.uber.org/zap" "k8s.io/apimachinery/pkg/runtime" utilRuntime "k8s.io/apimachinery/pkg/util/runtime" clientGoScheme "k8s.io/client-go/kubernetes/scheme" _ "k8s.io/client-go/plugin/pkg/client/auth" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/healthz" - "sigs.k8s.io/controller-runtime/pkg/log/zap" "github.com/gin-gonic/gin" - "github.com/go-logr/logr" + "github.com/go-logr/zapr" v1alpha1 "github.com/cedi/urlshortener/api/v1alpha1" "github.com/cedi/urlshortener/controllers" @@ -78,34 +78,37 @@ func main() { var probeAddr string var bindAddr string var namespaced bool + var debug bool flag.StringVar(&metricsAddr, "metrics-bind-address", ":9110", "The address the metric endpoint binds to.") flag.StringVar(&probeAddr, "health-probe-bind-address", ":9081", "The address the probe endpoint binds to.") flag.StringVar(&bindAddr, "bind-address", ":8443", "The address the service binds to.") flag.BoolVar(&namespaced, "namespaced", true, "Restrict the urlshortener to only list resources in the current namespace") + flag.BoolVar(&debug, "debug", false, "Turn on debug logging") - opts := zap.Options{ - Development: false, // false = production mode = JSON log format - } - - opts.BindFlags(flag.CommandLine) flag.Parse() // Initialize Logging - ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) - setupLog := ctrl.Log.WithName("setup") - shutdownLog := ctrl.Log.WithName("shutdown") + zapLog, otelLogger, undo := observability.InitLogging(debug) + defer otelLogger.Sync() + defer undo() + + ctrl.SetLogger(zapr.NewLogger(zapLog)) // Initialize Tracing (OpenTelemetry) traceProvider, tracer, err := observability.InitTracer(serviceName, serviceVersion) if err != nil { - setupLog.Error(err, "failed initializing tracing") + zapLog.Sugar().Errorw("failed initializing tracing", + zap.Error(err), + ) os.Exit(1) } defer func() { if err := traceProvider.Shutdown(context.Background()); err != nil { - shutdownLog.Error(err, "Error shutting down tracer provider") + zapLog.Sugar().Errorw("Error shutting down tracer provider", + zap.Error(err), + ) } }() @@ -118,7 +121,9 @@ func main() { namespaceByte, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") if err != nil { span.RecordError(err) - setupLog.Error(err, "Unable to read current namespace") + zapLog.Sugar().Errorw("Error shutting down tracer provider", + zap.Error(err), + ) os.Exit(1) } span.End() @@ -140,7 +145,9 @@ func main() { if err != nil { span.RecordError(err) - setupLog.Error(err, "unable to start urlshortener") + zapLog.Sugar().Errorw("unable to start urlshortener", + zap.Error(err), + ) os.Exit(1) } @@ -159,13 +166,16 @@ func main() { shortlinkReconciler := controllers.NewShortLinkReconciler( sClient, mgr.GetScheme(), - &ctrl.Log, + zapLog, tracer, ) if err = shortlinkReconciler.SetupWithManager(mgr); err != nil { span.RecordError(err) - setupLog.Error(err, "unable to create controller", "controller", "ShortLink") + zapLog.Sugar().Errorw("unable to create controller", + zap.Error(err), + zap.String("controller", "ShortLink"), + ) os.Exit(1) } @@ -173,13 +183,16 @@ func main() { mgr.GetClient(), rClient, mgr.GetScheme(), - &ctrl.Log, + zapLog, tracer, ) if err = redirectReconciler.SetupWithManager(mgr); err != nil { span.RecordError(err) - setupLog.Error(err, "unable to create controller", "controller", "Redirect") + zapLog.Sugar().Errorw("unable to create controller", + zap.Error(err), + zap.String("controller", "Redirect"), + ) os.Exit(1) } //+kubebuilder:scaffold:builder @@ -187,52 +200,61 @@ func main() { span.End() if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { - setupLog.Error(err, "unable to set up health check") + zapLog.Sugar().Errorw("unable to set up health check", + zap.Error(err), + ) os.Exit(1) } if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { - setupLog.Error(err, "unable to set up ready check") + zapLog.Sugar().Errorw("unable to set up ready check", + zap.Error(err), + ) os.Exit(1) } // run our urlshortener mgr in a separate go routine go func() { - setupLog.Info("starting urlshortener") + zapLog.Info("starting urlshortener") + if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { - setupLog.Error(err, "problem running urlshortener") + zapLog.Sugar().Errorw("unable starting urlshortener", + zap.Error(err), + ) os.Exit(1) } }() shortlinkController := apiController.NewShortlinkController( - &ctrl.Log, + zapLog, tracer, sClient, ) // Init Gin Framework gin.SetMode(gin.ReleaseMode) - r, srv := router.NewGinGonicHTTPServer(bindAddr, &setupLog, serviceName) + r, srv := router.NewGinGonicHTTPServer(bindAddr, zapLog, serviceName) - setupLog.Info("Load API routes") + zapLog.Info("Load API routes") router.Load(r, shortlinkController) // run our gin server mgr in a separate go routine go func() { if err := srv.ListenAndServe(); err != nil && errors.Is(err, http.ErrServerClosed) { - setupLog.Error(err, "listen\n") + zapLog.Sugar().Errorw("failed to listen and serve", + zap.Error(err), + ) } }() - handleShutdown(srv, &shutdownLog) + handleShutdown(srv, zapLog) - shutdownLog.Info("Server exiting") + zapLog.Info("Server exiting") } // handleShutdown waits for interrupt signal and then tries to gracefully // shutdown the server with a timeout of 5 seconds. -func handleShutdown(srv *http.Server, shutdownLog *logr.Logger) { +func handleShutdown(srv *http.Server, zapLog *zap.Logger) { quit := make(chan os.Signal, 1) signal.Notify( @@ -244,7 +266,7 @@ func handleShutdown(srv *http.Server, shutdownLog *logr.Logger) { // wait (and block) until shutdown signal is received <-quit - shutdownLog.Info("Shutting down server...") + zapLog.Info("Shutting down server...") // The context is used to inform the server it has 5 seconds to finish // the request it is currently handling @@ -255,7 +277,9 @@ func handleShutdown(srv *http.Server, shutdownLog *logr.Logger) { // then srv.Shutdown(ctx) will return an error, causing us to force // the shutdown if err := srv.Shutdown(ctx); err != nil { - shutdownLog.Error(err, "Server forced to shutdown") + zapLog.Sugar().Errorw("Server forced to shutdown", + zap.Error(err), + ) os.Exit(1) } } diff --git a/pkg/client/authenticated_shortlink_client.go b/pkg/client/authenticated_shortlink_client.go index 1289aac..1b501dd 100644 --- a/pkg/client/authenticated_shortlink_client.go +++ b/pkg/client/authenticated_shortlink_client.go @@ -5,22 +5,22 @@ import ( "github.com/cedi/urlshortener/api/v1alpha1" "github.com/cedi/urlshortener/pkg/model" + "go.uber.org/zap" - "github.com/go-logr/logr" "github.com/pkg/errors" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" ) type ShortlinkClientAuth struct { - log *logr.Logger + zapLog *zap.Logger tracer trace.Tracer client *ShortlinkClient } -func NewAuthenticatedShortlinkClient(log *logr.Logger, tracer trace.Tracer, client *ShortlinkClient) *ShortlinkClientAuth { +func NewAuthenticatedShortlinkClient(zapLog *zap.Logger, tracer trace.Tracer, client *ShortlinkClient) *ShortlinkClientAuth { return &ShortlinkClientAuth{ - log: log, + zapLog: zapLog, tracer: tracer, client: client, } diff --git a/pkg/controller/handle_create_shortlink.go b/pkg/controller/handle_create_shortlink.go index 3deeddf..625dd5b 100644 --- a/pkg/controller/handle_create_shortlink.go +++ b/pkg/controller/handle_create_shortlink.go @@ -12,6 +12,7 @@ import ( "github.com/gin-gonic/gin" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -55,6 +56,11 @@ func (s *ShortlinkController) HandleCreateShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) + log := s.zapLog.Sugar() + log.With(zap.String("shortlink", shortlinkName), + zap.String("operation", "create"), + ) + bearerToken := ct.Request.Header.Get("Authorization") bearerToken = strings.TrimPrefix(bearerToken, "Bearer") bearerToken = strings.TrimPrefix(bearerToken, "token") @@ -81,19 +87,19 @@ func (s *ShortlinkController) HandleCreateShortLink(ct *gin.Context) { jsonData, err := io.ReadAll(ct.Request.Body) if err != nil { - observability.RecordError(span, s.log, err, "Failed to read request-body") + observability.RecordError(span, log, err, "Failed to read request-body") ginReturnError(ct, http.StatusInternalServerError, contentType, err.Error()) return } if err := json.Unmarshal([]byte(jsonData), &shortlink.Spec); err != nil { - observability.RecordError(span, s.log, err, "Failed to read spec-json") + observability.RecordError(span, log, err, "Failed to read spec-json") ginReturnError(ct, http.StatusInternalServerError, contentType, err.Error()) return } if err := s.authenticatedClient.Create(ctx, githubUser.Login, &shortlink); err != nil { - observability.RecordError(span, s.log, err, "Failed to create ShortLink") + observability.RecordError(span, log, err, "Failed to create ShortLink") ginReturnError(ct, http.StatusInternalServerError, contentType, err.Error()) return } diff --git a/pkg/controller/handle_delete_shortlink.go b/pkg/controller/handle_delete_shortlink.go index 7285ec8..bdd48de 100644 --- a/pkg/controller/handle_delete_shortlink.go +++ b/pkg/controller/handle_delete_shortlink.go @@ -9,6 +9,7 @@ import ( "github.com/gin-gonic/gin" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" ) // HandleDeleteShortLink handles the deletion of a shortlink @@ -45,6 +46,11 @@ func (s *ShortlinkController) HandleDeleteShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) + log := s.zapLog.Sugar() + log.With(zap.String("shortlink", shortlinkName), + zap.String("operation", "delete"), + ) + bearerToken := ct.Request.Header.Get("Authorization") bearerToken = strings.TrimPrefix(bearerToken, "Bearer") bearerToken = strings.TrimPrefix(bearerToken, "token") @@ -64,7 +70,7 @@ func (s *ShortlinkController) HandleDeleteShortLink(ct *gin.Context) { shortlink, err := s.authenticatedClient.Get(ctx, githubUser.Login, shortlinkName) if err != nil { - observability.RecordError(span, s.log, err, "Failed to get ShortLink") + observability.RecordError(span, log, err, "Failed to get ShortLink") statusCode := http.StatusInternalServerError @@ -89,7 +95,7 @@ func (s *ShortlinkController) HandleDeleteShortLink(ct *gin.Context) { statusCode = http.StatusNotFound } - observability.RecordError(span, s.log, err, "Failed to delete ShortLink") + observability.RecordError(span, log, err, "Failed to delete ShortLink") ginReturnError(ct, statusCode, contentType, err.Error()) return diff --git a/pkg/controller/handle_get_shortlink.go b/pkg/controller/handle_get_shortlink.go index 141084d..8b7c556 100644 --- a/pkg/controller/handle_get_shortlink.go +++ b/pkg/controller/handle_get_shortlink.go @@ -9,6 +9,7 @@ import ( "github.com/gin-gonic/gin" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" ) // HandleGetShortLink returns the shortlink @@ -45,6 +46,11 @@ func (s *ShortlinkController) HandleGetShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) + log := s.zapLog.Sugar() + log.With(zap.String("shortlink", shortlinkName), + zap.String("operation", "create"), + ) + bearerToken := ct.Request.Header.Get("Authorization") bearerToken = strings.TrimPrefix(bearerToken, "Bearer") bearerToken = strings.TrimPrefix(bearerToken, "token") @@ -64,7 +70,7 @@ func (s *ShortlinkController) HandleGetShortLink(ct *gin.Context) { shortlink, err := s.authenticatedClient.Get(ctx, githubUser.Login, shortlinkName) if err != nil { - observability.RecordError(span, s.log, err, "Failed to get ShortLink") + observability.RecordError(span, log, err, "Failed to get ShortLink") statusCode := http.StatusInternalServerError diff --git a/pkg/controller/handle_list_shortlink.go b/pkg/controller/handle_list_shortlink.go index af859cb..3cd3c41 100644 --- a/pkg/controller/handle_list_shortlink.go +++ b/pkg/controller/handle_list_shortlink.go @@ -9,6 +9,7 @@ import ( "github.com/gin-gonic/gin" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" ) // HandleListShortLink handles the listing of @@ -43,6 +44,9 @@ func (s *ShortlinkController) HandleListShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) + log := s.zapLog.Sugar() + log.With(zap.String("operation", "list")) + bearerToken := ct.Request.Header.Get("Authorization") bearerToken = strings.TrimPrefix(bearerToken, "Bearer") bearerToken = strings.TrimPrefix(bearerToken, "token") @@ -62,7 +66,7 @@ func (s *ShortlinkController) HandleListShortLink(ct *gin.Context) { shortlinkList, err := s.authenticatedClient.List(ctx, githubUser.Login) if err != nil { - observability.RecordError(span, s.log, err, "Failed to list ShortLink") + observability.RecordError(span, log, err, "Failed to list ShortLink") statusCode := http.StatusInternalServerError diff --git a/pkg/controller/handle_shortlink.go b/pkg/controller/handle_shortlink.go index c2ab5f4..3288309 100644 --- a/pkg/controller/handle_shortlink.go +++ b/pkg/controller/handle_shortlink.go @@ -9,6 +9,7 @@ import ( "github.com/gin-gonic/gin" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" ) // HandleShortlink handles the shortlink and redirects according to the configuration @@ -48,17 +49,22 @@ func (s *ShortlinkController) HandleShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) + log := s.zapLog.Sugar() + log.With(zap.String("shortlink", shortlinkName), + zap.String("operation", "shortlink"), + ) + ct.Header("Cache-Control", "public, max-age=900, stale-if-error=3600") // max-age = 15min; stale-if-error = 1h shortlink, err := s.client.Get(ctx, shortlinkName) if err != nil { if strings.Contains(err.Error(), "not found") { - observability.RecordError(span, s.log, err, "Path not found") + observability.RecordError(span, log, err, "Path not found") span.SetAttributes(attribute.String("path", ct.Request.URL.Path)) ct.HTML(http.StatusNotFound, "404.html", gin.H{}) } else { - observability.RecordError(span, s.log, err, "Failed to get ShortLink") + observability.RecordError(span, log, err, "Failed to get ShortLink") ct.HTML(http.StatusInternalServerError, "500.html", gin.H{}) } return diff --git a/pkg/controller/handle_update_shortlink.go b/pkg/controller/handle_update_shortlink.go index 3b91d12..adf366a 100644 --- a/pkg/controller/handle_update_shortlink.go +++ b/pkg/controller/handle_update_shortlink.go @@ -12,6 +12,7 @@ import ( "github.com/gin-gonic/gin" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" ) // HandleDeleteShortLink handles the update of a shortlink @@ -49,6 +50,11 @@ func (s *ShortlinkController) HandleUpdateShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) + log := s.zapLog.Sugar() + log.With(zap.String("shortlink", shortlinkName), + zap.String("operation", "update"), + ) + bearerToken := ct.Request.Header.Get("Authorization") bearerToken = strings.TrimPrefix(bearerToken, "Bearer") bearerToken = strings.TrimPrefix(bearerToken, "token") @@ -68,7 +74,7 @@ func (s *ShortlinkController) HandleUpdateShortLink(ct *gin.Context) { shortlink, err := s.authenticatedClient.Get(ctx, githubUser.Login, shortlinkName) if err != nil { - observability.RecordError(span, s.log, err, "Failed to get ShortLink") + observability.RecordError(span, log, err, "Failed to get ShortLink") statusCode := http.StatusInternalServerError @@ -90,14 +96,14 @@ func (s *ShortlinkController) HandleUpdateShortLink(ct *gin.Context) { jsonData, err := io.ReadAll(ct.Request.Body) if err != nil { - observability.RecordError(span, s.log, err, "Failed to read request-body") + observability.RecordError(span, log, err, "Failed to read request-body") ginReturnError(ct, http.StatusInternalServerError, contentType, err.Error()) return } if err := json.Unmarshal([]byte(jsonData), &shortlinkSpec); err != nil { - observability.RecordError(span, s.log, err, "Failed to read ShortLink Spec JSON") + observability.RecordError(span, log, err, "Failed to read ShortLink Spec JSON") ginReturnError(ct, http.StatusInternalServerError, contentType, err.Error()) return @@ -106,7 +112,7 @@ func (s *ShortlinkController) HandleUpdateShortLink(ct *gin.Context) { shortlink.Spec = shortlinkSpec if err := s.authenticatedClient.Update(ctx, githubUser.Login, shortlink); err != nil { - observability.RecordError(span, s.log, err, "Failed to update ShortLink") + observability.RecordError(span, log, err, "Failed to update ShortLink") ginReturnError(ct, http.StatusInternalServerError, contentType, err.Error()) return diff --git a/pkg/controller/shortlink-controller.go b/pkg/controller/shortlink-controller.go index f54133c..13aabc7 100644 --- a/pkg/controller/shortlink-controller.go +++ b/pkg/controller/shortlink-controller.go @@ -2,7 +2,7 @@ package controller import ( shortlinkClient "github.com/cedi/urlshortener/pkg/client" - "github.com/go-logr/logr" + "go.uber.org/zap" "go.opentelemetry.io/otel/trace" ) @@ -11,17 +11,17 @@ import ( type ShortlinkController struct { client *shortlinkClient.ShortlinkClient authenticatedClient *shortlinkClient.ShortlinkClientAuth - log *logr.Logger + zapLog *zap.Logger tracer trace.Tracer } // NewShortlinkController creates a new ShortlinkController -func NewShortlinkController(log *logr.Logger, tracer trace.Tracer, client *shortlinkClient.ShortlinkClient) *ShortlinkController { +func NewShortlinkController(zapLog *zap.Logger, tracer trace.Tracer, client *shortlinkClient.ShortlinkClient) *ShortlinkController { controller := &ShortlinkController{ - log: log, + zapLog: zapLog, tracer: tracer, client: client, - authenticatedClient: shortlinkClient.NewAuthenticatedShortlinkClient(log, tracer, client), + authenticatedClient: shortlinkClient.NewAuthenticatedShortlinkClient(zapLog, tracer, client), } return controller diff --git a/pkg/observability/helpers.go b/pkg/observability/helpers.go index cfe49fb..8564ef1 100644 --- a/pkg/observability/helpers.go +++ b/pkg/observability/helpers.go @@ -3,23 +3,23 @@ package observability import ( "fmt" - "github.com/go-logr/logr" "github.com/pkg/errors" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" ) -func RecordError(span trace.Span, log *logr.Logger, err error, msg string, args ...any) error { +func RecordError(span trace.Span, zapLog *zap.SugaredLogger, err error, msg string, args ...any) error { message := fmt.Sprintf(msg, args...) span.AddEvent(message) - log.WithValues("traceID", span.SpanContext().TraceID()).Error(err, message) + zapLog.Errorw(msg, zap.Error(err)) err = errors.Wrap(err, message) span.RecordError(err) return err } -func RecordInfo(span trace.Span, log *logr.Logger, msg string, args ...any) { - log.WithValues("traceID", span.SpanContext().TraceID()).Info(fmt.Sprintf(msg, args...)) +func RecordInfo(span trace.Span, zapLog *zap.SugaredLogger, msg string, args ...any) { + zapLog.Infof(msg, args...) span.AddEvent(fmt.Sprintf(msg, args...)) } diff --git a/pkg/observability/opentelemetry.go b/pkg/observability/opentelemetry.go index 86e83c8..808b2f6 100644 --- a/pkg/observability/opentelemetry.go +++ b/pkg/observability/opentelemetry.go @@ -2,11 +2,13 @@ package observability import ( "context" + "fmt" "os" "strings" "github.com/MrAlias/flow" "github.com/pkg/errors" + "github.com/uptrace/opentelemetry-go-extra/otelzap" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/exporters/otlp/otlptrace" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp" @@ -15,10 +17,10 @@ import ( sdkTrace "go.opentelemetry.io/otel/sdk/trace" semconv "go.opentelemetry.io/otel/semconv/v1.4.0" "go.opentelemetry.io/otel/trace" + "go.uber.org/zap" ) func InitTracer(serviceName, serviceVersion string) (*sdkTrace.TracerProvider, trace.Tracer, error) { - otlpEndpoint, ok := os.LookupEnv("OTLP_ENDPOINT") otlpInsecure := os.Getenv("OTLP_INSECURE") @@ -80,3 +82,28 @@ func InitTracer(serviceName, serviceVersion string) (*sdkTrace.TracerProvider, t return traceProvider, trace, nil } + +func InitLogging(debug bool) (*zap.Logger, *otelzap.Logger, func()) { + var zapLog *zap.Logger + var err error + + if debug { + zapLog, err = zap.NewDevelopment() + } else { + zapLog, err = zap.NewProduction() + } + + if err != nil { + panic(fmt.Sprintf("Failed to initialize logger (%v)", err)) + } + + otelZap := otelzap.New(zapLog, + otelzap.WithTraceIDField(true), + otelzap.WithCaller(true), + otelzap.WithErrorStatusLevel(zap.ErrorLevel), + ) + + undo := otelzap.ReplaceGlobals(otelZap) + + return zapLog, otelZap, undo +} diff --git a/pkg/router/router.go b/pkg/router/router.go index 4482315..5108901 100644 --- a/pkg/router/router.go +++ b/pkg/router/router.go @@ -1,16 +1,15 @@ package router import ( - "fmt" "net/http" docs "github.com/cedi/urlshortener/docs" urlShortenerController "github.com/cedi/urlshortener/pkg/controller" + "go.uber.org/zap" "github.com/gin-gonic/contrib/secure" "github.com/gin-gonic/gin" - "github.com/go-logr/logr" "go.opentelemetry.io/contrib/instrumentation/github.com/gin-gonic/gin/otelgin" swaggerFiles "github.com/swaggo/files" @@ -33,7 +32,7 @@ import ( // @in header // @name Authorization -func NewGinGonicHTTPServer(bindAddr string, setupLog *logr.Logger, serviceName string) (*gin.Engine, *http.Server) { +func NewGinGonicHTTPServer(bindAddr string, zapLog *zap.Logger, serviceName string) (*gin.Engine, *http.Server) { router := gin.New() router.Use( otelgin.Middleware(serviceName), @@ -55,7 +54,10 @@ func NewGinGonicHTTPServer(bindAddr string, setupLog *logr.Logger, serviceName s //static path router.Static("assets", "./html/assets") - setupLog.Info(fmt.Sprintf("Starting gin-tonic router on binAddr: '%s'", bindAddr)) + zapLog.Sugar().Infow("Starting gin-tonic router", + zap.String("bindAddr", bindAddr), + ) + srv := &http.Server{ Addr: bindAddr, Handler: router,