From ab9a6cc12570d3f1aa0688b16bacc530fda93b06 Mon Sep 17 00:00:00 2001 From: Cedric Kienzler Date: Tue, 6 Jun 2023 14:53:26 +0200 Subject: [PATCH] refactor to otel zapLog --- controllers/redirect_controller.go | 17 ++++---- controllers/shortlink_controller.go | 11 +++-- main.go | 46 +++++++++----------- pkg/client/authenticated_shortlink_client.go | 5 +-- pkg/client/redirect_client.go | 5 +-- pkg/client/shortlink_client.go | 5 +-- pkg/controller/handle_create_shortlink.go | 13 +++--- pkg/controller/handle_delete_shortlink.go | 11 ++--- pkg/controller/handle_get_shortlink.go | 9 ++-- pkg/controller/handle_list_shortlink.go | 9 ++-- pkg/controller/handle_shortlink.go | 7 +-- pkg/controller/handle_update_shortlink.go | 15 ++++--- pkg/controller/shortlink-controller.go | 7 +-- pkg/observability/helpers.go | 10 +++-- pkg/observability/opentelemetry.go | 5 ++- pkg/router/router.go | 5 ++- 16 files changed, 86 insertions(+), 94 deletions(-) diff --git a/controllers/redirect_controller.go b/controllers/redirect_controller.go index 911ebeb..55ae55e 100644 --- a/controllers/redirect_controller.go +++ b/controllers/redirect_controller.go @@ -35,6 +35,7 @@ import ( "github.com/cedi/urlshortener/pkg/observability" redirectpkg "github.com/cedi/urlshortener/pkg/redirect" "github.com/pkg/errors" + "github.com/uptrace/opentelemetry-go-extra/otelzap" ) // RedirectReconciler reconciles a Redirect object @@ -43,17 +44,15 @@ type RedirectReconciler struct { rClient *redirectclient.RedirectClient scheme *runtime.Scheme - zapLog *zap.Logger tracer trace.Tracer } // NewRedirectReconciler returns a new RedirectReconciler -func NewRedirectReconciler(client client.Client, rClient *redirectclient.RedirectClient, scheme *runtime.Scheme, zapLog *zap.Logger, tracer trace.Tracer) *RedirectReconciler { +func NewRedirectReconciler(client client.Client, rClient *redirectclient.RedirectClient, scheme *runtime.Scheme, tracer trace.Tracer) *RedirectReconciler { return &RedirectReconciler{ client: client, rClient: rClient, scheme: scheme, - zapLog: zapLog, tracer: tracer, } } @@ -86,7 +85,7 @@ func (r *RedirectReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c span.SetAttributes(attribute.String("redirect", req.NamespacedName.String())) - log := r.zapLog.Sugar().With(zap.String("name", "reconciler"), zap.String("redirect", req.NamespacedName.String())) + log := otelzap.L().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 +99,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(ctx, 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(ctx, 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(ctx, span, log, err, "Failed to upsert redirect ingress") } // Update the Redirect status with the ingress name and the target @@ -123,7 +122,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(ctx, span, log, err, "Failed to list ingresses") return ctrl.Result{}, err } @@ -132,7 +131,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(ctx, 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 cc3e1d4..495d54a 100644 --- a/controllers/shortlink_controller.go +++ b/controllers/shortlink_controller.go @@ -31,22 +31,21 @@ import ( v1alpha1 "github.com/cedi/urlshortener/api/v1alpha1" shortlinkclient "github.com/cedi/urlshortener/pkg/client" "github.com/cedi/urlshortener/pkg/observability" + "github.com/uptrace/opentelemetry-go-extra/otelzap" ) // ShortLinkReconciler reconciles a ShortLink object type ShortLinkReconciler struct { client *shortlinkclient.ShortlinkClient scheme *runtime.Scheme - zapLog *zap.Logger tracer trace.Tracer } // NewShortLinkReconciler returns a new ShortLinkReconciler -func NewShortLinkReconciler(client *shortlinkclient.ShortlinkClient, scheme *runtime.Scheme, zapLog *zap.Logger, tracer trace.Tracer) *ShortLinkReconciler { +func NewShortLinkReconciler(client *shortlinkclient.ShortlinkClient, scheme *runtime.Scheme, tracer trace.Tracer) *ShortLinkReconciler { return &ShortLinkReconciler{ client: client, scheme: scheme, - zapLog: zapLog, tracer: tracer, } } @@ -76,15 +75,15 @@ func (r *ShortLinkReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( span.SetAttributes(attribute.String("shortlink", req.Name)) - log := r.zapLog.Sugar().With(zap.String("name", "reconciler"), zap.String("shortlink", req.NamespacedName.String())) + log := otelzap.L().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(ctx, 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(ctx, span, log, err, "Failed to fetch ShortLink resource") } } diff --git a/main.go b/main.go index cd38582..9b50061 100644 --- a/main.go +++ b/main.go @@ -37,6 +37,7 @@ import ( "github.com/gin-gonic/gin" "github.com/go-logr/zapr" + "github.com/uptrace/opentelemetry-go-extra/otelzap" v1alpha1 "github.com/cedi/urlshortener/api/v1alpha1" "github.com/cedi/urlshortener/controllers" @@ -89,16 +90,16 @@ func main() { flag.Parse() // Initialize Logging - zapLog, otelLogger, undo := observability.InitLogging(debug) + otelLogger, undo := observability.InitLogging(debug) defer otelLogger.Sync() defer undo() - ctrl.SetLogger(zapr.NewLogger(zapLog)) + ctrl.SetLogger(zapr.NewLogger(otelzap.L().Logger)) // Initialize Tracing (OpenTelemetry) traceProvider, tracer, err := observability.InitTracer(serviceName, serviceVersion) if err != nil { - zapLog.Sugar().Errorw("failed initializing tracing", + otelzap.L().Sugar().Errorw("failed initializing tracing", zap.Error(err), ) os.Exit(1) @@ -106,7 +107,7 @@ func main() { defer func() { if err := traceProvider.Shutdown(context.Background()); err != nil { - zapLog.Sugar().Errorw("Error shutting down tracer provider", + otelzap.L().Sugar().Errorw("Error shutting down tracer provider", zap.Error(err), ) } @@ -121,7 +122,7 @@ func main() { namespaceByte, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/namespace") if err != nil { span.RecordError(err) - zapLog.Sugar().Errorw("Error shutting down tracer provider", + otelzap.L().Sugar().Errorw("Error shutting down tracer provider", zap.Error(err), ) os.Exit(1) @@ -145,7 +146,7 @@ func main() { if err != nil { span.RecordError(err) - zapLog.Sugar().Errorw("unable to start urlshortener", + otelzap.L().Sugar().Errorw("unable to start urlshortener", zap.Error(err), ) os.Exit(1) @@ -153,26 +154,23 @@ func main() { sClient := shortlinkClient.NewShortlinkClient( mgr.GetClient(), - &ctrl.Log, tracer, ) rClient := shortlinkClient.NewRedirectClient( mgr.GetClient(), - &ctrl.Log, tracer, ) shortlinkReconciler := controllers.NewShortLinkReconciler( sClient, mgr.GetScheme(), - zapLog, tracer, ) if err = shortlinkReconciler.SetupWithManager(mgr); err != nil { span.RecordError(err) - zapLog.Sugar().Errorw("unable to create controller", + otelzap.L().Sugar().Errorw("unable to create controller", zap.Error(err), zap.String("controller", "ShortLink"), ) @@ -183,13 +181,12 @@ func main() { mgr.GetClient(), rClient, mgr.GetScheme(), - zapLog, tracer, ) if err = redirectReconciler.SetupWithManager(mgr); err != nil { span.RecordError(err) - zapLog.Sugar().Errorw("unable to create controller", + otelzap.L().Sugar().Errorw("unable to create controller", zap.Error(err), zap.String("controller", "Redirect"), ) @@ -200,14 +197,14 @@ func main() { span.End() if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil { - zapLog.Sugar().Errorw("unable to set up health check", + otelzap.L().Sugar().Errorw("unable to set up health check", zap.Error(err), ) os.Exit(1) } if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { - zapLog.Sugar().Errorw("unable to set up ready check", + otelzap.L().Sugar().Errorw("unable to set up ready check", zap.Error(err), ) os.Exit(1) @@ -215,10 +212,10 @@ func main() { // run our urlshortener mgr in a separate go routine go func() { - zapLog.Info("starting urlshortener") + otelzap.L().Info("starting urlshortener") if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { - zapLog.Sugar().Errorw("unable starting urlshortener", + otelzap.L().Sugar().Errorw("unable starting urlshortener", zap.Error(err), ) os.Exit(1) @@ -226,35 +223,34 @@ func main() { }() shortlinkController := apiController.NewShortlinkController( - zapLog, tracer, sClient, ) // Init Gin Framework gin.SetMode(gin.ReleaseMode) - r, srv := router.NewGinGonicHTTPServer(bindAddr, zapLog, serviceName) + r, srv := router.NewGinGonicHTTPServer(bindAddr, serviceName) - zapLog.Info("Load API routes") + otelzap.L().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) { - zapLog.Sugar().Errorw("failed to listen and serve", + otelzap.L().Sugar().Errorw("failed to listen and serve", zap.Error(err), ) } }() - handleShutdown(srv, zapLog) + handleShutdown(srv) - zapLog.Info("Server exiting") + otelzap.L().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, zapLog *zap.Logger) { +func handleShutdown(srv *http.Server) { quit := make(chan os.Signal, 1) signal.Notify( @@ -266,7 +262,7 @@ func handleShutdown(srv *http.Server, zapLog *zap.Logger) { // wait (and block) until shutdown signal is received <-quit - zapLog.Info("Shutting down server...") + otelzap.L().Info("Shutting down server...") // The context is used to inform the server it has 5 seconds to finish // the request it is currently handling @@ -277,7 +273,7 @@ func handleShutdown(srv *http.Server, zapLog *zap.Logger) { // then srv.Shutdown(ctx) will return an error, causing us to force // the shutdown if err := srv.Shutdown(ctx); err != nil { - zapLog.Sugar().Errorw("Server forced to shutdown", + otelzap.L().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 1b501dd..1074d3b 100644 --- a/pkg/client/authenticated_shortlink_client.go +++ b/pkg/client/authenticated_shortlink_client.go @@ -5,7 +5,6 @@ import ( "github.com/cedi/urlshortener/api/v1alpha1" "github.com/cedi/urlshortener/pkg/model" - "go.uber.org/zap" "github.com/pkg/errors" "go.opentelemetry.io/otel/attribute" @@ -13,14 +12,12 @@ import ( ) type ShortlinkClientAuth struct { - zapLog *zap.Logger tracer trace.Tracer client *ShortlinkClient } -func NewAuthenticatedShortlinkClient(zapLog *zap.Logger, tracer trace.Tracer, client *ShortlinkClient) *ShortlinkClientAuth { +func NewAuthenticatedShortlinkClient(tracer trace.Tracer, client *ShortlinkClient) *ShortlinkClientAuth { return &ShortlinkClientAuth{ - zapLog: zapLog, tracer: tracer, client: client, } diff --git a/pkg/client/redirect_client.go b/pkg/client/redirect_client.go index d69f9e1..344d76a 100644 --- a/pkg/client/redirect_client.go +++ b/pkg/client/redirect_client.go @@ -5,7 +5,6 @@ import ( "os" "github.com/cedi/urlshortener/api/v1alpha1" - "github.com/go-logr/logr" "github.com/pkg/errors" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -18,15 +17,13 @@ import ( // RedirectClient is a Kubernetes client for easy CRUD operations type RedirectClient struct { client client.Client - log *logr.Logger tracer trace.Tracer } // NewRedirectClient creates a new Redirect Client -func NewRedirectClient(client client.Client, log *logr.Logger, tracer trace.Tracer) *RedirectClient { +func NewRedirectClient(client client.Client, tracer trace.Tracer) *RedirectClient { return &RedirectClient{ client: client, - log: log, tracer: tracer, } } diff --git a/pkg/client/shortlink_client.go b/pkg/client/shortlink_client.go index 444e6a8..e01d426 100644 --- a/pkg/client/shortlink_client.go +++ b/pkg/client/shortlink_client.go @@ -5,7 +5,6 @@ import ( "os" "github.com/cedi/urlshortener/api/v1alpha1" - "github.com/go-logr/logr" "github.com/pkg/errors" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -16,15 +15,13 @@ import ( // ShortlinkClient is a Kubernetes client for easy CRUD operations type ShortlinkClient struct { client client.Client - log *logr.Logger tracer trace.Tracer } // NewShortlinkClient creates a new shortlink Client -func NewShortlinkClient(client client.Client, log *logr.Logger, tracer trace.Tracer) *ShortlinkClient { +func NewShortlinkClient(client client.Client, tracer trace.Tracer) *ShortlinkClient { return &ShortlinkClient{ client: client, - log: log, tracer: tracer, } } diff --git a/pkg/controller/handle_create_shortlink.go b/pkg/controller/handle_create_shortlink.go index 36e8966..6c3eef4 100644 --- a/pkg/controller/handle_create_shortlink.go +++ b/pkg/controller/handle_create_shortlink.go @@ -10,6 +10,7 @@ import ( "github.com/cedi/urlshortener/api/v1alpha1" "github.com/cedi/urlshortener/pkg/observability" "github.com/gin-gonic/gin" + "github.com/uptrace/opentelemetry-go-extra/otelzap" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" @@ -56,7 +57,7 @@ func (s *ShortlinkController) HandleCreateShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) - log := s.zapLog.Sugar().With(zap.String("shortlink", shortlinkName), + log := otelzap.L().Sugar().With(zap.String("shortlink", shortlinkName), zap.String("operation", "create"), ) @@ -65,14 +66,14 @@ func (s *ShortlinkController) HandleCreateShortLink(ct *gin.Context) { bearerToken = strings.TrimPrefix(bearerToken, "token") if len(bearerToken) == 0 { err := fmt.Errorf("no credentials provided") - span.RecordError(err) + observability.RecordError(ctx, span, log, err, "no credentials provided") ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error()) return } githubUser, err := getGitHubUserInfo(ctx, bearerToken) if err != nil { - span.RecordError(err) + observability.RecordError(ctx, span, log, err, "GitHub User Info invalid") ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error()) return } @@ -86,19 +87,19 @@ func (s *ShortlinkController) HandleCreateShortLink(ct *gin.Context) { jsonData, err := io.ReadAll(ct.Request.Body) if err != nil { - observability.RecordError(span, log, err, "Failed to read request-body") + observability.RecordError(ctx, 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, log, err, "Failed to read spec-json") + observability.RecordError(ctx, 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, log, err, "Failed to create ShortLink") + observability.RecordError(ctx, 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 9cba78f..3ed5a73 100644 --- a/pkg/controller/handle_delete_shortlink.go +++ b/pkg/controller/handle_delete_shortlink.go @@ -7,6 +7,7 @@ import ( "github.com/cedi/urlshortener/pkg/observability" "github.com/gin-gonic/gin" + "github.com/uptrace/opentelemetry-go-extra/otelzap" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" @@ -46,7 +47,7 @@ func (s *ShortlinkController) HandleDeleteShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) - log := s.zapLog.Sugar().With(zap.String("shortlink", shortlinkName), + log := otelzap.L().Sugar().With(zap.String("shortlink", shortlinkName), zap.String("operation", "delete"), ) @@ -55,21 +56,21 @@ func (s *ShortlinkController) HandleDeleteShortLink(ct *gin.Context) { bearerToken = strings.TrimPrefix(bearerToken, "token") if len(bearerToken) == 0 { err := fmt.Errorf("no credentials provided") - span.RecordError(err) + observability.RecordError(ctx, span, log, err, "no credentials provided") ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error()) return } githubUser, err := getGitHubUserInfo(ctx, bearerToken) if err != nil { - span.RecordError(err) + observability.RecordError(ctx, span, log, err, "GitHub User Info invalid") ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error()) return } shortlink, err := s.authenticatedClient.Get(ctx, githubUser.Login, shortlinkName) if err != nil { - observability.RecordError(span, log, err, "Failed to get ShortLink") + observability.RecordError(ctx, span, log, err, "Failed to get ShortLink") statusCode := http.StatusInternalServerError @@ -94,7 +95,7 @@ func (s *ShortlinkController) HandleDeleteShortLink(ct *gin.Context) { statusCode = http.StatusNotFound } - observability.RecordError(span, log, err, "Failed to delete ShortLink") + observability.RecordError(ctx, 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 d593217..cb12d59 100644 --- a/pkg/controller/handle_get_shortlink.go +++ b/pkg/controller/handle_get_shortlink.go @@ -7,6 +7,7 @@ import ( "github.com/cedi/urlshortener/pkg/observability" "github.com/gin-gonic/gin" + "github.com/uptrace/opentelemetry-go-extra/otelzap" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" @@ -46,7 +47,7 @@ func (s *ShortlinkController) HandleGetShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) - log := s.zapLog.Sugar().With(zap.String("shortlink", shortlinkName), + log := otelzap.L().Sugar().With(zap.String("shortlink", shortlinkName), zap.String("operation", "create"), ) @@ -55,21 +56,21 @@ func (s *ShortlinkController) HandleGetShortLink(ct *gin.Context) { bearerToken = strings.TrimPrefix(bearerToken, "token") if len(bearerToken) == 0 { err := fmt.Errorf("no credentials provided") - span.RecordError(err) + observability.RecordError(ctx, span, log, err, "no credentials provided") ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error()) return } githubUser, err := getGitHubUserInfo(ctx, bearerToken) if err != nil { - span.RecordError(err) + observability.RecordError(ctx, span, log, err, "GitHub User Info invalid") ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error()) return } shortlink, err := s.authenticatedClient.Get(ctx, githubUser.Login, shortlinkName) if err != nil { - observability.RecordError(span, log, err, "Failed to get ShortLink") + observability.RecordError(ctx, 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 dc346a0..d238bf1 100644 --- a/pkg/controller/handle_list_shortlink.go +++ b/pkg/controller/handle_list_shortlink.go @@ -7,6 +7,7 @@ import ( "github.com/cedi/urlshortener/pkg/observability" "github.com/gin-gonic/gin" + "github.com/uptrace/opentelemetry-go-extra/otelzap" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" @@ -44,28 +45,28 @@ func (s *ShortlinkController) HandleListShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) - log := s.zapLog.Sugar().With(zap.String("operation", "list")) + log := otelzap.L().Sugar().With(zap.String("operation", "list")) bearerToken := ct.Request.Header.Get("Authorization") bearerToken = strings.TrimPrefix(bearerToken, "Bearer") bearerToken = strings.TrimPrefix(bearerToken, "token") if len(bearerToken) == 0 { err := fmt.Errorf("no credentials provided") - span.RecordError(err) + observability.RecordError(ctx, span, log, err, "no credentials provided") ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error()) return } githubUser, err := getGitHubUserInfo(ctx, bearerToken) if err != nil { - span.RecordError(err) + observability.RecordError(ctx, span, log, err, "GitHub User Info invalid") ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error()) return } shortlinkList, err := s.authenticatedClient.List(ctx, githubUser.Login) if err != nil { - observability.RecordError(span, log, err, "Failed to list ShortLink") + observability.RecordError(ctx, 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 125cb7f..a11895c 100644 --- a/pkg/controller/handle_shortlink.go +++ b/pkg/controller/handle_shortlink.go @@ -7,6 +7,7 @@ import ( "github.com/cedi/urlshortener/pkg/observability" "github.com/gin-gonic/gin" + "github.com/uptrace/opentelemetry-go-extra/otelzap" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" @@ -49,7 +50,7 @@ func (s *ShortlinkController) HandleShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) - log := s.zapLog.Sugar().With(zap.String("shortlink", shortlinkName), + log := otelzap.L().Sugar().With(zap.String("shortlink", shortlinkName), zap.String("operation", "shortlink"), ) @@ -58,12 +59,12 @@ func (s *ShortlinkController) HandleShortLink(ct *gin.Context) { shortlink, err := s.client.Get(ctx, shortlinkName) if err != nil { if strings.Contains(err.Error(), "not found") { - observability.RecordError(span, log, err, "Path not found") + observability.RecordError(ctx, 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, log, err, "Failed to get ShortLink") + observability.RecordError(ctx, 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 1d3e625..29ead2f 100644 --- a/pkg/controller/handle_update_shortlink.go +++ b/pkg/controller/handle_update_shortlink.go @@ -10,6 +10,7 @@ import ( "github.com/cedi/urlshortener/api/v1alpha1" "github.com/cedi/urlshortener/pkg/observability" "github.com/gin-gonic/gin" + "github.com/uptrace/opentelemetry-go-extra/otelzap" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" @@ -50,7 +51,7 @@ func (s *ShortlinkController) HandleUpdateShortLink(ct *gin.Context) { attribute.String("referrer", ct.Request.Referer()), ) - log := s.zapLog.Sugar().With(zap.String("shortlink", shortlinkName), + log := otelzap.L().Sugar().With(zap.String("shortlink", shortlinkName), zap.String("operation", "update"), ) @@ -59,21 +60,21 @@ func (s *ShortlinkController) HandleUpdateShortLink(ct *gin.Context) { bearerToken = strings.TrimPrefix(bearerToken, "token") if len(bearerToken) == 0 { err := fmt.Errorf("no credentials provided") - span.RecordError(err) + observability.RecordError(ctx, span, log, err, "no credentials provided") ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error()) return } githubUser, err := getGitHubUserInfo(ctx, bearerToken) if err != nil { - span.RecordError(err) + observability.RecordError(ctx, span, log, err, "GitHub User Info invalid") ginReturnError(ct, http.StatusUnauthorized, contentType, err.Error()) return } shortlink, err := s.authenticatedClient.Get(ctx, githubUser.Login, shortlinkName) if err != nil { - observability.RecordError(span, log, err, "Failed to get ShortLink") + observability.RecordError(ctx, span, log, err, "Failed to get ShortLink") statusCode := http.StatusInternalServerError @@ -95,14 +96,14 @@ func (s *ShortlinkController) HandleUpdateShortLink(ct *gin.Context) { jsonData, err := io.ReadAll(ct.Request.Body) if err != nil { - observability.RecordError(span, log, err, "Failed to read request-body") + observability.RecordError(ctx, 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, log, err, "Failed to read ShortLink Spec JSON") + observability.RecordError(ctx, span, log, err, "Failed to read ShortLink Spec JSON") ginReturnError(ct, http.StatusInternalServerError, contentType, err.Error()) return @@ -111,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, log, err, "Failed to update ShortLink") + observability.RecordError(ctx, 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 13aabc7..83b18a6 100644 --- a/pkg/controller/shortlink-controller.go +++ b/pkg/controller/shortlink-controller.go @@ -2,7 +2,6 @@ package controller import ( shortlinkClient "github.com/cedi/urlshortener/pkg/client" - "go.uber.org/zap" "go.opentelemetry.io/otel/trace" ) @@ -11,17 +10,15 @@ import ( type ShortlinkController struct { client *shortlinkClient.ShortlinkClient authenticatedClient *shortlinkClient.ShortlinkClientAuth - zapLog *zap.Logger tracer trace.Tracer } // NewShortlinkController creates a new ShortlinkController -func NewShortlinkController(zapLog *zap.Logger, tracer trace.Tracer, client *shortlinkClient.ShortlinkClient) *ShortlinkController { +func NewShortlinkController(tracer trace.Tracer, client *shortlinkClient.ShortlinkClient) *ShortlinkController { controller := &ShortlinkController{ - zapLog: zapLog, tracer: tracer, client: client, - authenticatedClient: shortlinkClient.NewAuthenticatedShortlinkClient(zapLog, tracer, client), + authenticatedClient: shortlinkClient.NewAuthenticatedShortlinkClient(tracer, client), } return controller diff --git a/pkg/observability/helpers.go b/pkg/observability/helpers.go index 8564ef1..7fcea70 100644 --- a/pkg/observability/helpers.go +++ b/pkg/observability/helpers.go @@ -1,25 +1,27 @@ package observability import ( + "context" "fmt" "github.com/pkg/errors" + "github.com/uptrace/opentelemetry-go-extra/otelzap" "go.opentelemetry.io/otel/trace" "go.uber.org/zap" ) -func RecordError(span trace.Span, zapLog *zap.SugaredLogger, err error, msg string, args ...any) error { +func RecordError(ctx context.Context, span trace.Span, zapLog *otelzap.SugaredLogger, err error, msg string, args ...any) error { message := fmt.Sprintf(msg, args...) span.AddEvent(message) - zapLog.Errorw(msg, zap.Error(err)) + zapLog.Ctx(ctx).Errorw(msg, zap.Error(err), zap.String("span_id", span.SpanContext().SpanID().String())) err = errors.Wrap(err, message) span.RecordError(err) return err } -func RecordInfo(span trace.Span, zapLog *zap.SugaredLogger, msg string, args ...any) { - zapLog.Infof(msg, args...) +func RecordInfo(ctx context.Context, span trace.Span, zapLog *otelzap.SugaredLogger, msg string, args ...any) { + zapLog.Ctx(ctx).Infow(fmt.Sprintf(msg, args...)) span.AddEvent(fmt.Sprintf(msg, args...)) } diff --git a/pkg/observability/opentelemetry.go b/pkg/observability/opentelemetry.go index 808b2f6..a418b99 100644 --- a/pkg/observability/opentelemetry.go +++ b/pkg/observability/opentelemetry.go @@ -83,7 +83,7 @@ func InitTracer(serviceName, serviceVersion string) (*sdkTrace.TracerProvider, t return traceProvider, trace, nil } -func InitLogging(debug bool) (*zap.Logger, *otelzap.Logger, func()) { +func InitLogging(debug bool) (*otelzap.Logger, func()) { var zapLog *zap.Logger var err error @@ -101,9 +101,10 @@ func InitLogging(debug bool) (*zap.Logger, *otelzap.Logger, func()) { otelzap.WithTraceIDField(true), otelzap.WithCaller(true), otelzap.WithErrorStatusLevel(zap.ErrorLevel), + otelzap.WithStackTrace(false), ) undo := otelzap.ReplaceGlobals(otelZap) - return zapLog, otelZap, undo + return otelZap, undo } diff --git a/pkg/router/router.go b/pkg/router/router.go index 5108901..4b82646 100644 --- a/pkg/router/router.go +++ b/pkg/router/router.go @@ -5,6 +5,7 @@ import ( docs "github.com/cedi/urlshortener/docs" urlShortenerController "github.com/cedi/urlshortener/pkg/controller" + "github.com/uptrace/opentelemetry-go-extra/otelzap" "go.uber.org/zap" "github.com/gin-gonic/contrib/secure" @@ -32,7 +33,7 @@ import ( // @in header // @name Authorization -func NewGinGonicHTTPServer(bindAddr string, zapLog *zap.Logger, serviceName string) (*gin.Engine, *http.Server) { +func NewGinGonicHTTPServer(bindAddr string, serviceName string) (*gin.Engine, *http.Server) { router := gin.New() router.Use( otelgin.Middleware(serviceName), @@ -54,7 +55,7 @@ func NewGinGonicHTTPServer(bindAddr string, zapLog *zap.Logger, serviceName stri //static path router.Static("assets", "./html/assets") - zapLog.Sugar().Infow("Starting gin-tonic router", + otelzap.L().Sugar().Infow("Starting gin-tonic router", zap.String("bindAddr", bindAddr), )