[chaosplt 207] namespace tag gone (#936)

* [chaosplt] get rid of bad bad log tags

* woops

* Apply suggestions from code review

api/v1beta1/disruption_cron_webhook.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	cLog "github.com/DataDog/chaos-controller/log"
 	"github.com/DataDog/chaos-controller/utils"
 	"github.com/robfig/cron"
 	"go.uber.org/zap"
@@ -71,7 +72,7 @@ var _ webhook.Defaulter = &DisruptionCron{}
 // Default implements webhook.Defaulter so a webhook will be registered for the type
 func (d *DisruptionCron) Default() {
 	if d.Spec.DelayedStartTolerance.Duration() == 0 {
-		logger.Infow(fmt.Sprintf("setting default delayedStartTolerance of %s in disruptionCron", defaultCronDelayedStartTolerance), "instance", d.Name, "namespace", d.Namespace)
+		logger.Infow(fmt.Sprintf("setting default delayedStartTolerance of %s in disruptionCron", defaultCronDelayedStartTolerance), cLog.DisruptionCronNameKey, d.Name, cLog.DisruptionCronNamespaceKey, d.Namespace)
 		d.Spec.DelayedStartTolerance = DisruptionDuration(defaultCronDelayedStartTolerance.String())
 	}
 }
@@ -82,7 +83,7 @@ var _ webhook.Validator = &DisruptionCron{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (d *DisruptionCron) ValidateCreate() (admission.Warnings, error) {
-	log := disruptionCronWebhookLogger.With("disruptionCronName", d.Name, "disruptionCronNamespace", d.Namespace)
+	log := disruptionCronWebhookLogger.With(cLog.DisruptionCronNameKey, d.Name, cLog.DisruptionCronNamespaceKey, d.Namespace)
 
 	log.Infow("validating created disruption cron", "spec", d.Spec)
 
@@ -114,7 +115,7 @@ func (d *DisruptionCron) ValidateCreate() (admission.Warnings, error) {
 }
 
 func (d *DisruptionCron) ValidateUpdate(oldObject runtime.Object) (admission.Warnings, error) {
-	log := logger.With("disruptionCronName", d.Name, "disruptionCronNamespace", d.Namespace)
+	log := logger.With(cLog.DisruptionCronNameKey, d.Name, cLog.DisruptionCronNamespaceKey, d.Namespace)
 
 	log.Infow("validating updated disruption cron", "spec", d.Spec)
 
@@ -146,7 +147,7 @@ func (d *DisruptionCron) ValidateUpdate(oldObject runtime.Object) (admission.War
 }
 
 func (d *DisruptionCron) ValidateDelete() (warnings admission.Warnings, err error) {
-	log := disruptionCronWebhookLogger.With("disruptionCronName", d.Name, "disruptionCronNamespace", d.Namespace)
+	log := disruptionCronWebhookLogger.With(cLog.DisruptionCronNameKey, d.Name, cLog.DisruptionCronNamespaceKey, d.Namespace)
 
 	log.Infow("validating deleted disruption cron", "spec", d.Spec)
 

api/v1beta1/disruption_webhook.go

@@ -18,6 +18,7 @@ import (
 	"github.com/DataDog/chaos-controller/cloudservice"
 	cloudtypes "github.com/DataDog/chaos-controller/cloudservice/types"
 	"github.com/DataDog/chaos-controller/ddmark"
+	cLog "github.com/DataDog/chaos-controller/log"
 	"github.com/DataDog/chaos-controller/o11y/metrics"
 	"github.com/DataDog/chaos-controller/o11y/tracer"
 	chaostypes "github.com/DataDog/chaos-controller/types"
@@ -121,7 +122,7 @@ var _ webhook.Validator = &Disruption{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type
 func (r *Disruption) ValidateCreate() (admission.Warnings, error) {
-	log := logger.With("disruptionName", r.Name, "disruptionNamespace", r.Namespace)
+	log := logger.With(cLog.DisruptionNameKey, r.Name, cLog.DisruptionNamespaceKey, r.Namespace)
 
 	ctx, err := r.SpanContext(context.Background())
 	if err != nil {
@@ -274,7 +275,7 @@ func (r *Disruption) ValidateCreate() (admission.Warnings, error) {
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
 func (r *Disruption) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
-	log := logger.With("disruptionName", r.Name, "disruptionNamespace", r.Namespace)
+	log := logger.With(cLog.DisruptionNameKey, r.Name, cLog.DisruptionNamespaceKey, r.Namespace)
 	log.Debugw("validating updated disruption", "spec", r.Spec)
 
 	var err error
@@ -384,7 +385,7 @@ func (r *Disruption) getMetricsTags() []string {
 
 	if userInfo, err := r.UserInfo(); !errors.Is(err, ErrNoUserInfo) {
 		if err != nil {
-			logger.Errorw("error retrieving user info from disruption, using empty user info", "error", err, "disruptionName", r.Name, "disruptionNamespace", r.Namespace)
+			logger.Errorw("error retrieving user info from disruption, using empty user info", "error", err, cLog.DisruptionNameKey, r.Name, cLog.DisruptionNamespaceKey, r.Namespace)
 		}
 
 		tags = append(tags, "username:"+userInfo.Username)
@@ -602,8 +603,8 @@ func safetyNetCountNotTooLarge(r *Disruption) (bool, string, error) {
 	}
 
 	logger.Debugw("comparing estimated target count to total existing targets",
-		"disruptionName", r.Name,
-		"disruptionNamespace", r.Namespace,
+		cLog.DisruptionNameKey, r.Name,
+		cLog.DisruptionNamespaceKey, r.Namespace,
 		"namespaceThreshold", namespaceThreshold,
 		"clusterThreshold", clusterThreshold,
 		"estimatedEligibleTargetsCount", targetCount,

cli/injector/main.go

@@ -148,14 +148,14 @@ func initLogger() {
 	}
 
 	log = log.With(
-		"disruptionName", disruptionArgs.DisruptionName,
-		"disruptionNamespace", disruptionArgs.DisruptionNamespace,
+		logger.DisruptionNameKey, disruptionArgs.DisruptionName,
+		logger.DisruptionNamespaceKey, disruptionArgs.DisruptionNamespace,
 		"targetName", disruptionArgs.TargetName,
 		"targetNodeName", disruptionArgs.TargetNodeName,
 	)
 
 	if parentPID != 0 {
-		log = log.With("parent_pid", parentPID)
+		log = log.With("parentPid", parentPID)
 	}
 }
 

controllers/cron_rollout_helpers.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	chaosv1beta1 "github.com/DataDog/chaos-controller/api/v1beta1"
+	cLog "github.com/DataDog/chaos-controller/log"
 	"go.uber.org/zap"
 	appsv1 "k8s.io/api/apps/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -163,7 +164,7 @@ func CreateDisruptionFromTemplate(ctx context.Context, cl client.Client, scheme
 	disruption.Labels[ownerNameLabel] = owner.GetName()
 
 	if err := setDisruptionAnnotations(disruption, owner, scheduledTime); err != nil {
-		log.Errorw("unable to set annotations for child disruption", "err", err, "disruptionName", disruption.Name)
+		log.Errorw("unable to set annotations for child disruption", "err", err, cLog.DisruptionNameKey, disruption.Name)
 	}
 
 	if err := overwriteDisruptionSelectors(ctx, cl, disruption, targetResource, owner.GetNamespace()); err != nil {
@@ -181,7 +182,7 @@ func CreateDisruptionFromTemplate(ctx context.Context, cl client.Client, scheme
 func getScheduledTimeForDisruption(log *zap.SugaredLogger, disruption *chaosv1beta1.Disruption) time.Time {
 	parsedTime, err := disruption.GetScheduledAtAnnotation()
 	if err != nil {
-		log.Errorw("unable to parse schedule time for child disruption", "err", err, "disruptionName", disruption.Name)
+		log.Errorw("unable to parse schedule time for child disruption", "err", err, cLog.DisruptionNameKey, disruption.Name)
 		return time.Time{}
 	}
 

controllers/disruption_controller.go

@@ -26,6 +26,7 @@ import (
 
 	chaosv1beta1 "github.com/DataDog/chaos-controller/api/v1beta1"
 	"github.com/DataDog/chaos-controller/cloudservice"
+	cLog "github.com/DataDog/chaos-controller/log"
 	"github.com/DataDog/chaos-controller/o11y/metrics"
 	"github.com/DataDog/chaos-controller/o11y/tracer"
 	"github.com/DataDog/chaos-controller/safemode"
@@ -96,7 +97,7 @@ func (r *DisruptionReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	// because the logger instance is pointer, concurrent reconciling would create a race condition
 	// where the logger context would change for all ongoing reconcile loops
 	// in the case we enable concurrent reconciling, we should create one logger instance per reconciling call
-	r.log = r.BaseLog.With("disruptionName", req.Name, "disruptionNamespace", req.Namespace)
+	r.log = r.BaseLog.With(cLog.DisruptionNameKey, req.Name, cLog.DisruptionNamespaceKey, req.Namespace)
 
 	// reconcile metrics
 	r.handleMetricSinkError(r.MetricsSink.MetricReconcile())

controllers/disruption_cron_controller.go

@@ -11,6 +11,7 @@ import (
 	"time"
 
 	chaosv1beta1 "github.com/DataDog/chaos-controller/api/v1beta1"
+	cLog "github.com/DataDog/chaos-controller/log"
 	"github.com/DataDog/chaos-controller/o11y/metrics"
 	chaostypes "github.com/DataDog/chaos-controller/types"
 
@@ -35,7 +36,7 @@ type DisruptionCronReconciler struct {
 }
 
 func (r *DisruptionCronReconciler) Reconcile(ctx context.Context, req ctrl.Request) (res ctrl.Result, err error) {
-	r.log = r.BaseLog.With("disruptionCronNamespace", req.Namespace, "disruptionCronName", req.Name)
+	r.log = r.BaseLog.With(cLog.DisruptionCronNamespaceKey, req.Namespace, cLog.DisruptionCronNameKey, req.Name)
 	r.log.Info("Reconciling DisruptionCron")
 
 	// reconcile metrics
@@ -192,7 +193,7 @@ func (r *DisruptionCronReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 
 	r.handleMetricSinkError(r.MetricsSink.MetricDisruptionScheduled(append(DisruptionCronTags, "disruptionName:"+disruption.Name)))
 
-	r.log.Infow("created Disruption for DisruptionCron run", "disruptionName", disruption.Name)
+	r.log.Infow("created Disruption for DisruptionCron run", cLog.DisruptionNameKey, disruption.Name)
 
 	// ------------------------------------------------------------------ //
 	// If this process restarts at this point (after posting a disruption, but

controllers/disruption_rollout_controller.go

@@ -10,6 +10,7 @@ import (
 	"math/rand"
 	"time"
 
+	cLog "github.com/DataDog/chaos-controller/log"
 	"github.com/DataDog/chaos-controller/o11y/metrics"
 
 	chaosv1beta1 "github.com/DataDog/chaos-controller/api/v1beta1"
@@ -150,7 +151,7 @@ func (r *DisruptionRolloutReconciler) Reconcile(ctx context.Context, req ctrl.Re
 
 	r.handleMetricSinkError(r.MetricsSink.MetricDisruptionScheduled(append(DisruptionRolloutTags, "disruptionName:"+disruption.Name)))
 
-	r.log.Infow("created Disruption for DisruptionRollout run", "disruptionName", disruption.Name)
+	r.log.Infow("created Disruption for DisruptionRollout run", cLog.DisruptionNameKey, disruption.Name)
 
 	// ------------------------------------------------------------------ //
 	// If this process restarts at this point (after posting a disruption, but

eventnotifier/datadog/datadog.go

@@ -12,6 +12,7 @@ import (
 	"github.com/DataDog/chaos-controller/api/v1beta1"
 	"github.com/DataDog/chaos-controller/eventnotifier/types"
 	"github.com/DataDog/chaos-controller/eventnotifier/utils"
+	cLog "github.com/DataDog/chaos-controller/log"
 	"github.com/DataDog/datadog-go/statsd"
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
@@ -74,7 +75,7 @@ func (n *Notifier) Notify(obj client.Object, event corev1.Event, notifType types
 }
 
 func (n *Notifier) notifyDisruption(d *v1beta1.Disruption, event corev1.Event, notifType types.NotificationType) error {
-	n.logger.With("disruptionName", d.Name)
+	n.logger.With(cLog.DisruptionNameKey, d.Name, cLog.DisruptionNamespaceKey, d.Namespace)
 
 	eventType := n.getEventAlertType(notifType)
 
@@ -86,7 +87,7 @@ func (n *Notifier) notifyDisruption(d *v1beta1.Disruption, event corev1.Event, n
 }
 
 func (n *Notifier) notifyDisruptionCron(d *v1beta1.DisruptionCron, event corev1.Event, notifType types.NotificationType) error {
-	n.logger.With("disruptionCronName", d.Name, "disruptionCronNamespace", d.Namespace)
+	n.logger.With(cLog.DisruptionCronNameKey, d.Name, cLog.DisruptionCronNamespaceKey, d.Namespace)
 
 	eventType := n.getEventAlertType(notifType)
 

eventnotifier/http/http.go

@@ -21,6 +21,7 @@ import (
 	"github.com/DataDog/chaos-controller/api/v1beta1"
 	"github.com/DataDog/chaos-controller/eventnotifier/types"
 	"github.com/DataDog/chaos-controller/eventnotifier/utils"
+	cLog "github.com/DataDog/chaos-controller/log"
 	"github.com/DataDog/jsonapi"
 	"go.uber.org/zap"
 	authv1 "k8s.io/api/authentication/v1"
@@ -248,7 +249,7 @@ func (n *Notifier) marshalUserGroups(groups []string) (string, error) {
 func (n *Notifier) notifyDisruption(dis v1beta1.Disruption, event corev1.Event, notifType types.NotificationType) error {
 	logger := n.logger
 
-	logger.With("disruptionName", dis.Name, "disruptionNamespace", dis.Namespace)
+	logger.With(cLog.DisruptionNameKey, dis.Name, cLog.DisruptionNamespaceKey, dis.Namespace)
 
 	if !n.disruptionConfig.Enabled {
 		return nil
@@ -292,7 +293,7 @@ func (n *Notifier) notifyDisruption(dis v1beta1.Disruption, event corev1.Event,
 func (n *Notifier) notifyDisruptionCron(disruptionCron v1beta1.DisruptionCron, event corev1.Event, notifType types.NotificationType) error {
 	logger := n.logger
 
-	logger.With("disruptionCronName", disruptionCron.Name, "disruptionCronNamespace", disruptionCron.Namespace)
+	logger.With(cLog.DisruptionCronNameKey, disruptionCron.Name, cLog.DisruptionCronNamespaceKey, disruptionCron.Namespace)
 
 	if !n.disruptionCronConfig.Enabled {
 		return nil

eventnotifier/slack/slack.go

@@ -17,6 +17,7 @@ import (
 	"github.com/DataDog/chaos-controller/api/v1beta1"
 	"github.com/DataDog/chaos-controller/eventnotifier/types"
 	"github.com/DataDog/chaos-controller/eventnotifier/utils"
+	cLog "github.com/DataDog/chaos-controller/log"
 	"github.com/slack-go/slack"
 	"go.uber.org/zap"
 	authv1 "k8s.io/api/authentication/v1"
@@ -120,7 +121,7 @@ func (n *Notifier) Notify(obj client.Object, event corev1.Event, notifType types
 }
 
 func (n *Notifier) notifyForDisruption(dis *v1beta1.Disruption, event corev1.Event, notifType types.NotificationType) error {
-	logger := n.logger.With("disruptionName", dis.Name, "disruptionNamespace", dis.Namespace, "eventType", event.Type)
+	logger := n.logger.With(cLog.DisruptionNameKey, dis.Name, cLog.DisruptionNamespaceKey, dis.Namespace, "eventType", event.Type)
 
 	slackMsg := n.buildSlackMessage(dis, event, notifType, dis.Spec.Reporting, logger)
 
@@ -153,7 +154,7 @@ func (n *Notifier) notifyForDisruption(dis *v1beta1.Disruption, event corev1.Eve
 }
 
 func (n *Notifier) notifyForDisruptionCron(disruptionCron *v1beta1.DisruptionCron, event corev1.Event, notifType types.NotificationType) error {
-	logger := n.logger.With("disruptionCronName", disruptionCron.Name, "disruptionCronNamespace", disruptionCron.Namespace, "eventType", event.Type)
+	logger := n.logger.With(cLog.DisruptionCronNameKey, disruptionCron.Name, cLog.DisruptionCronNamespaceKey, disruptionCron.Namespace, "eventType", event.Type)
 
 	slackMsg := n.buildSlackMessage(disruptionCron, event, notifType, disruptionCron.Spec.Reporting, logger)
 

log/log.go

@@ -12,6 +12,19 @@ import (
 	"go.uber.org/zap/zapcore"
 )
 
+// logger's keys
+const (
+	// Disruption
+	DisruptionPrefixKey    = "disruption"
+	DisruptionNameKey      = DisruptionPrefixKey + "Name"
+	DisruptionNamespaceKey = DisruptionPrefixKey + "Namespace"
+
+	// DisruptionCron
+	DisruptionCronPrefixKey    = "disruptionCron"
+	DisruptionCronNameKey      = DisruptionCronPrefixKey + "Name"
+	DisruptionCronNamespaceKey = DisruptionCronPrefixKey + "Namespace"
+)
+
 // NewZapLogger returns a zap production sugared logger with pre-configured encoder settings
 func NewZapLogger() (*zap.SugaredLogger, error) {
 	// configure logger

watchers/chaos_pod_handler.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 
 	chaosv1beta1 "github.com/DataDog/chaos-controller/api/v1beta1"
+	cLog "github.com/DataDog/chaos-controller/log"
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/tools/record"
@@ -98,8 +99,8 @@ func (w ChaosPodHandler) sendEvent(newPod *corev1.Pod) {
 	w.log.Debugw("ChaosPodHandler UPDATE - Send event",
 		"eventMessage", eventMessage,
 		"eventType", eventType,
-		"disruptionName", w.disruption.Name,
-		"disruptionNamespace", w.disruption.Namespace,
+		cLog.DisruptionNameKey, w.disruption.Name,
+		cLog.DisruptionNamespaceKey, w.disruption.Namespace,
 		"chaosPodName", newPod.Name,
 	)
 }

watchers/disruptions_watchers_manager.go

@@ -10,6 +10,7 @@ import (
 	"fmt"
 
 	"github.com/DataDog/chaos-controller/api/v1beta1"
+	cLog "github.com/DataDog/chaos-controller/log"
 	"go.uber.org/zap"
 	"k8s.io/apimachinery/pkg/types"
 	k8scontrollercache "sigs.k8s.io/controller-runtime/pkg/cache"
@@ -99,7 +100,7 @@ func (d disruptionsWatchersManager) CreateAllWatchers(disruption *v1beta1.Disrup
 			return err
 		}
 
-		d.log.Debugw("Watcher created", "watcherName", watcherName, "disruptionName", disruption.Name, "disruptionNamespace", disruption.Namespace)
+		d.log.Debugw("Watcher created", "watcherName", watcherName, cLog.DisruptionNameKey, disruption.Name, cLog.DisruptionNamespaceKey, disruption.Namespace)
 	}
 
 	return nil
@@ -114,7 +115,7 @@ func (d disruptionsWatchersManager) RemoveAllWatchers(disruption *v1beta1.Disrup
 
 	// If the Watcher Manager does not exist just do nothing.
 	if watcherManager == nil {
-		d.log.Debugw("could not remove all watchers", "disruptionName", disruption.Name, "disruptionNamespace", disruption.Namespace)
+		d.log.Debugw("could not remove all watchers", cLog.DisruptionNameKey, disruption.Name, cLog.DisruptionNamespaceKey, disruption.Namespace)
 		return
 	}
 
@@ -123,7 +124,7 @@ func (d disruptionsWatchersManager) RemoveAllWatchers(disruption *v1beta1.Disrup
 	// Remove the Watcher Manager from the map.
 	delete(d.watchersManagers, namespacedName)
 
-	d.log.Infow("all watchers have been removed", "disruptionName", disruption.Name, "disruptionNamespace", disruption.Namespace)
+	d.log.Infow("all watchers have been removed", cLog.DisruptionNameKey, disruption.Name, cLog.DisruptionNamespaceKey, disruption.Namespace)
 }
 
 // RemoveAllOrphanWatchers removes all Watchers associated with a none existing Disruption.
@@ -143,7 +144,7 @@ func (d disruptionsWatchersManager) RemoveAllOrphanWatchers() error {
 			// Remove the watcher manager from the stored managers
 			delete(d.watchersManagers, namespacedName)
 
-			d.log.Infow("all watchers have been removed", "disruptionName", namespacedName.Name, "disruptionNamespace", namespacedName.Namespace)
+			d.log.Infow("all watchers have been removed", cLog.DisruptionNameKey, namespacedName.Name, cLog.DisruptionNamespaceKey, namespacedName.Namespace)
 		}
 	}
 
@@ -206,12 +207,12 @@ func getDisruptionNamespacedName(disruption *v1beta1.Disruption) types.Namespace
 func (d disruptionsWatchersManager) getWatcherManager(disruptionNamespacedName types.NamespacedName) Manager {
 	// If we have already created a watcher manager for this disruption, use it
 	if cachedWatcherManager := d.watchersManagers[disruptionNamespacedName]; cachedWatcherManager != nil {
-		d.log.Debugw("Load watcher manager from the cache", "disruptionName", disruptionNamespacedName.Name, "disruptionNamespace", disruptionNamespacedName.Namespace)
+		d.log.Debugw("Load watcher manager from the cache", cLog.DisruptionNameKey, disruptionNamespacedName.Name, cLog.DisruptionNamespaceKey, disruptionNamespacedName.Namespace)
 
 		return cachedWatcherManager
 	}
 
-	d.log.Debugw("Creating a new watcher manager", "disruptionName", disruptionNamespacedName.Name, "disruptionNamespace", disruptionNamespacedName.Namespace)
+	d.log.Debugw("Creating a new watcher manager", cLog.DisruptionNameKey, disruptionNamespacedName.Name, cLog.DisruptionNamespaceKey, disruptionNamespacedName.Namespace)
 
 	// Otherwise, create a new watcher manager
 	return NewManager(d.reader, d.controller)