chore: refactor logger to be structured (#555)

* chore: refactor logger to be structured

* fix: linter complains

pkg/instance/build.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+	"github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
 
 	"github.com/celestiaorg/knuu/pkg/builder"
@@ -135,7 +136,10 @@ func (b *build) SetUser(user string) error {
 	if err := b.builderFactory.SetUser(user); err != nil {
 		return ErrSettingUser.WithParams(user, b.instance.name).Wrap(err)
 	}
-	b.instance.Logger.Debugf("Set user '%s' for instance '%s'", user, b.instance.name)
+	b.instance.Logger.WithFields(logrus.Fields{
+		"instance": b.instance.name,
+		"user":     user,
+	}).Debugf("Set user for instance")
 	return nil
 }
 
@@ -148,7 +152,10 @@ func (b *build) Commit(ctx context.Context) error {
 
 	if !b.builderFactory.Changed() {
 		b.imageName = b.builderFactory.ImageNameFrom()
-		b.instance.Logger.Debugf("No need to build and push image for instance '%s'", b.instance.name)
+		b.instance.Logger.WithFields(logrus.Fields{
+			"instance": b.instance.name,
+			"image":    b.imageName,
+		}).Debugf("no need to build and push image for instance")
 
 		b.instance.SetState(StateCommitted)
 		return nil
@@ -169,18 +176,31 @@ func (b *build) Commit(ctx context.Context) error {
 	cachedImageName, exists := b.checkImageHashInCache(imageHash)
 	if exists {
 		b.imageName = cachedImageName
-		b.instance.Logger.Debugf("Using cached image for instance '%s'", b.instance.name)
-	} else {
-		b.instance.Logger.Debugf("Cannot use any cached image for instance '%s'", b.instance.name)
-		err = b.builderFactory.PushBuilderImage(ctx, imageName)
-		if err != nil {
-			return ErrPushingImage.WithParams(b.instance.name).Wrap(err)
-		}
-		b.updateImageCacheWithHash(imageHash, imageName)
-		b.imageName = imageName
-		b.instance.Logger.Debugf("Pushed new image for instance '%s'", b.instance.name)
+
+		b.instance.Logger.WithFields(logrus.Fields{
+			"instance": b.instance.name,
+			"image":    b.imageName,
+		}).Debugf("using cached image for instance")
+
+		b.instance.SetState(StateCommitted)
+		return nil
 	}
 
+	b.instance.Logger.WithFields(logrus.Fields{
+		"instance": b.instance.name,
+	}).Debugf("cannot use any cached image for instance")
+	err = b.builderFactory.PushBuilderImage(ctx, imageName)
+	if err != nil {
+		return ErrPushingImage.WithParams(b.instance.name).Wrap(err)
+	}
+	b.updateImageCacheWithHash(imageHash, imageName)
+	b.imageName = imageName
+
+	b.instance.Logger.WithFields(logrus.Fields{
+		"instance": b.instance.name,
+		"image":    b.imageName,
+	}).Debugf("pushed new image for instance")
+
 	b.instance.SetState(StateCommitted)
 	return nil
 }
@@ -220,11 +240,15 @@ func (b *build) SetEnvironmentVariable(key, value string) error {
 	if !b.instance.IsInState(StatePreparing, StateCommitted) {
 		return ErrSettingEnvNotAllowed.WithParams(b.instance.state.String())
 	}
-	b.instance.Logger.Debugf("Setting environment variable '%s' in instance '%s'", key, b.instance.name)
+	b.instance.Logger.WithFields(logrus.Fields{
+		"instance": b.instance.name,
+		"key":      key,
+		// value is not logged to avoid leaking sensitive information
+	}).Debugf("Setting environment variable")
+
 	if b.instance.state == StatePreparing {
 		return b.builderFactory.SetEnvVar(key, value)
 	}
-
 	b.env[key] = value
 	return nil
 }

pkg/instance/execution.go

@@ -73,7 +73,10 @@ func (e *execution) StartWithCallback(ctx context.Context, callback func()) erro
 	go func() {
 		err := e.WaitInstanceIsRunning(ctx)
 		if err != nil {
-			e.instance.Logger.Errorf("Error waiting for instance '%s' to be running: %s", e.instance.k8sName, err)
+			e.instance.Logger.WithFields(logrus.Fields{
+				"instance": e.instance.k8sName,
+				"error":    err,
+			}).Errorf("waiting for instance to be running")
 			return
 		}
 		callback()
@@ -117,10 +120,8 @@ func (e *execution) StartAsync(ctx context.Context) error {
 		return ErrDeployingPodForInstance.WithParams(e.instance.k8sName).Wrap(err)
 	}
 
-	e.instance.state = StateStarted
+	e.instance.SetState(StateStarted)
 	e.instance.sidecars.setStateForSidecars(StateStarted)
-	e.instance.Logger.Debugf("Set state of instance '%s' to '%s'", e.instance.k8sName, e.instance.state.String())
-
 	return nil
 }
 
@@ -212,10 +213,9 @@ func (e *execution) Stop(ctx context.Context) error {
 	if err := e.destroyPod(ctx); err != nil {
 		return ErrDestroyingPod.WithParams(e.instance.k8sName).Wrap(err)
 	}
-	e.instance.state = StateStopped
-	e.instance.sidecars.setStateForSidecars(StateStopped)
-	e.instance.Logger.Debugf("Set state of instance '%s' to '%s'", e.instance.k8sName, e.instance.state.String())
 
+	e.instance.SetState(StateStopped)
+	e.instance.sidecars.setStateForSidecars(StateStopped)
 	return nil
 }
 
@@ -252,17 +252,18 @@ func (e *execution) Destroy(ctx context.Context) error {
 
 	err := e.instance.sidecars.applyFunctionToSidecars(
 		func(sidecar SidecarManager) error {
-			e.instance.Logger.Debugf("Destroying sidecar resources from '%s'", sidecar.Instance().k8sName)
+			e.instance.Logger.WithFields(logrus.Fields{
+				"instance": e.instance.k8sName,
+				"sidecar":  sidecar.Instance().k8sName,
+			}).Debugf("destroying sidecar resources")
 			return sidecar.Instance().resources.destroyResources(ctx)
 		})
 	if err != nil {
 		return ErrDestroyingResourcesForSidecars.WithParams(e.instance.k8sName).Wrap(err)
 	}
 
-	e.instance.state = StateDestroyed
+	e.instance.SetState(StateDestroyed)
 	e.instance.sidecars.setStateForSidecars(StateDestroyed)
-	e.instance.Logger.Debugf("Set state of instance '%s' to '%s'", e.instance.k8sName, e.instance.state.String())
-
 	return nil
 }
 
@@ -344,9 +345,7 @@ func (e *execution) deployPod(ctx context.Context) error {
 	e.instance.kubernetesReplicaSet = replicaSet
 
 	// Log the deployment of the pod
-	e.instance.Logger.Debugf("Started statefulSet '%s'", e.instance.k8sName)
-	e.instance.Logger.Debugf("Set state of instance '%s' to '%s'", e.instance.k8sName, e.instance.state.String())
-
+	e.instance.Logger.WithField("instance", e.instance.k8sName).Debugf("started statefulSet")
 	return nil
 }
 

pkg/instance/monitoring.go

@@ -1,6 +1,9 @@
 package instance
 
-import v1 "k8s.io/api/core/v1"
+import (
+	"github.com/sirupsen/logrus"
+	v1 "k8s.io/api/core/v1"
+)
 
 type monitoring struct {
 	instance       *Instance
@@ -22,7 +25,10 @@ func (m *monitoring) SetLivenessProbe(livenessProbe *v1.Probe) error {
 		return err
 	}
 	m.livenessProbe = livenessProbe
-	m.instance.Logger.Debugf("Set liveness probe to '%s' in instance '%s'", livenessProbe, m.instance.name)
+	m.instance.Logger.WithFields(logrus.Fields{
+		"instance":       m.instance.name,
+		"liveness_probe": livenessProbe,
+	}).Debug("set liveness probe")
 	return nil
 }
 
@@ -35,7 +41,10 @@ func (m *monitoring) SetReadinessProbe(readinessProbe *v1.Probe) error {
 		return err
 	}
 	m.readinessProbe = readinessProbe
-	m.instance.Logger.Debugf("Set readiness probe to '%s' in instance '%s'", readinessProbe, m.instance.name)
+	m.instance.Logger.WithFields(logrus.Fields{
+		"instance":        m.instance.name,
+		"readiness_probe": readinessProbe,
+	}).Debug("set readiness probe")
 	return nil
 }
 
@@ -48,7 +57,10 @@ func (m *monitoring) SetStartupProbe(startupProbe *v1.Probe) error {
 		return err
 	}
 	m.startupProbe = startupProbe
-	m.instance.Logger.Debugf("Set startup probe to '%s' in instance '%s'", startupProbe, m.instance.name)
+	m.instance.Logger.WithFields(logrus.Fields{
+		"instance":      m.instance.name,
+		"startup_probe": startupProbe,
+	}).Debug("set startup probe")
 	return nil
 }
 

pkg/instance/network.go

@@ -5,6 +5,7 @@ import (
 	"net"
 	"time"
 
+	"github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
 )
 
@@ -34,7 +35,10 @@ func (n *network) AddPortTCP(port int) error {
 	}
 
 	n.portsTCP = append(n.portsTCP, port)
-	n.instance.Logger.Debugf("Added TCP port '%d' to instance '%s'", port, n.instance.name)
+	n.instance.Logger.WithFields(logrus.Fields{
+		"instance": n.instance.name,
+		"port":     port,
+	}).Debug("added tcp port to instance")
 	return nil
 }
 
@@ -79,7 +83,14 @@ func (n *network) PortForwardTCP(ctx context.Context, port int) (int, error) {
 		if attempt == maxRetries {
 			return -1, ErrForwardingPort.WithParams(maxRetries)
 		}
-		n.instance.Logger.Debugf("Forwarding port %d failed, cause: %v, retrying after %v (retry %d/%d)", port, err, retryInterval, attempt, maxRetries)
+		n.instance.Logger.WithFields(logrus.Fields{
+			"instance":       n.instance.name,
+			"port":           port,
+			"error":          err,
+			"attempt":        attempt,
+			"max":            maxRetries,
+			"retry_interval": retryInterval.String(),
+		}).Debug("forwarding port failed, retrying")
 	}
 	return localPort, nil
 }
@@ -99,7 +110,10 @@ func (n *network) AddPortUDP(port int) error {
 	}
 	n.portsUDP = append(n.portsUDP, port)
 
-	n.instance.Logger.Debugf("Added UDP port '%d' to instance '%s'", port, n.instance.k8sName)
+	n.instance.Logger.WithFields(logrus.Fields{
+		"instance": n.instance.name,
+		"port":     port,
+	}).Debug("added udp port to instance")
 	return nil
 }
 
@@ -152,7 +166,10 @@ func (n *network) deployService(ctx context.Context, portsTCP, portsUDP []int) e
 		return ErrDeployingService.WithParams(n.instance.k8sName).Wrap(err)
 	}
 	n.kubernetesService = srv
-	n.instance.Logger.Debugf("Started service '%s'", n.instance.k8sName)
+	n.instance.Logger.WithFields(logrus.Fields{
+		"instance": n.instance.name,
+		"service":  serviceName,
+	}).Debug("started service")
 	return nil
 }
 
@@ -174,7 +191,10 @@ func (n *network) patchService(ctx context.Context, portsTCP, portsUDP []int) er
 		return ErrPatchingService.WithParams(serviceName).Wrap(err)
 	}
 	n.kubernetesService = srv
-	n.instance.Logger.Debugf("Patched service '%s'", serviceName)
+	n.instance.Logger.WithFields(logrus.Fields{
+		"instance": n.instance.name,
+		"service":  serviceName,
+	}).Debug("patched service")
 	return nil
 }
 
@@ -257,7 +277,7 @@ func (n *network) deployOrPatchService(ctx context.Context, portsTCP, portsUDP [
 		return nil
 	}
 
-	n.instance.Logger.Debugf("Ports not empty, deploying service for instance '%s'", n.instance.k8sName)
+	n.instance.Logger.WithField("instance", n.instance.name).Debug("ports not empty, deploying service")
 	svc, _ := n.instance.K8sClient.GetService(ctx, n.instance.k8sName)
 	if svc == nil {
 		if err := n.deployService(ctx, portsTCP, portsUDP); err != nil {
@@ -275,15 +295,21 @@ func (n *network) deployOrPatchService(ctx context.Context, portsTCP, portsUDP [
 func (n *network) enableIfDisabled(ctx context.Context) error {
 	disableNetwork, err := n.IsDisabled(ctx)
 	if err != nil {
-		n.instance.Logger.Errorf("error checking network status for instance")
+		n.instance.Logger.WithFields(logrus.Fields{
+			"instance": n.instance.name,
+			"error":    err,
+		}).Error("error checking network status for instance")
 		return ErrCheckingNetworkStatusForInstance.WithParams(n.instance.k8sName).Wrap(err)
 	}
 
 	if !disableNetwork {
 		return nil
 	}
 	if err := n.Enable(ctx); err != nil {
-		n.instance.Logger.Errorf("error enabling network for instance")
+		n.instance.Logger.WithFields(logrus.Fields{
+			"instance": n.instance.name,
+			"error":    err,
+		}).Error("error enabling network for instance")
 		return ErrEnablingNetworkForInstance.WithParams(n.instance.k8sName).Wrap(err)
 	}
 	return nil

pkg/instance/resources.go

@@ -3,6 +3,7 @@ package instance
 import (
 	"context"
 
+	"github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
@@ -26,7 +27,11 @@ func (r *resources) SetMemory(request, limit resource.Quantity) error {
 	}
 	r.memoryRequest = request
 	r.memoryLimit = limit
-	r.instance.Logger.Debugf("Set memory to '%s' and limit to '%s' in instance '%s'", request.String(), limit.String(), r.instance.name)
+	r.instance.Logger.WithFields(logrus.Fields{
+		"instance":       r.instance.name,
+		"memory_request": request.String(),
+		"memory_limit":   limit.String(),
+	}).Debug("set memory for instance")
 	return nil
 }
 
@@ -37,7 +42,10 @@ func (r *resources) SetCPU(request resource.Quantity) error {
 		return ErrSettingCPUNotAllowed.WithParams(r.instance.state.String())
 	}
 	r.cpuRequest = request
-	r.instance.Logger.Debugf("Set cpu to '%s' in instance '%s'", request.String(), r.instance.name)
+	r.instance.Logger.WithFields(logrus.Fields{
+		"instance":    r.instance.name,
+		"cpu_request": request.String(),
+	}).Debug("set cpu for instance")
 	return nil
 }
 

pkg/instance/security.go

@@ -1,6 +1,9 @@
 package instance
 
 import (
+	"strings"
+
+	"github.com/sirupsen/logrus"
 	v1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 )
@@ -40,7 +43,10 @@ func (s *security) SetPrivileged(privileged bool) error {
 		return ErrSettingPrivilegedNotAllowed.WithParams(s.instance.state.String())
 	}
 	s.privileged = privileged
-	s.instance.Logger.Debugf("Set privileged to '%t' for instance '%s'", privileged, s.instance.name)
+	s.instance.Logger.WithFields(logrus.Fields{
+		"instance":   s.instance.name,
+		"privileged": privileged,
+	}).Debug("set privileged for instance")
 	return nil
 }
 
@@ -51,7 +57,10 @@ func (s *security) AddKubernetesCapability(capability string) error {
 		return ErrAddingCapabilityNotAllowed.WithParams(s.instance.state.String())
 	}
 	s.capabilitiesAdd = append(s.capabilitiesAdd, capability)
-	s.instance.Logger.Debugf("Added capability '%s' to instance '%s'", capability, s.instance.name)
+	s.instance.Logger.WithFields(logrus.Fields{
+		"instance":   s.instance.name,
+		"capability": capability,
+	}).Debug("added capability to instance")
 	return nil
 }
 
@@ -61,10 +70,12 @@ func (s *security) AddKubernetesCapabilities(capabilities []string) error {
 	if !s.instance.IsInState(StatePreparing, StateCommitted) {
 		return ErrAddingCapabilitiesNotAllowed.WithParams(s.instance.state.String())
 	}
-	for _, capability := range capabilities {
-		s.capabilitiesAdd = append(s.capabilitiesAdd, capability)
-		s.instance.Logger.Debugf("Added capability '%s' to instance '%s'", capability, s.instance.name)
-	}
+	s.capabilitiesAdd = append(s.capabilitiesAdd, capabilities...)
+
+	s.instance.Logger.WithFields(logrus.Fields{
+		"instance":     s.instance.name,
+		"capabilities": strings.Join(capabilities, ", "),
+	}).Debug("added capabilities to instance")
 	return nil
 }