Skip to content

Commit

Permalink
chore(logs): standardize application logging
Browse files Browse the repository at this point in the history
Signed-off-by: Louis Gérard <[email protected]>
  • Loading branch information
dhawos committed Jan 9, 2025
1 parent b8508f2 commit d4f077a
Show file tree
Hide file tree
Showing 11 changed files with 93 additions and 88 deletions.
26 changes: 14 additions & 12 deletions applicationset/controllers/applicationset_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/predicate"

applog "github.com/argoproj/argo-cd/v2/util/app/log"

"github.com/argoproj/argo-cd/v2/applicationset/controllers/template"
"github.com/argoproj/argo-cd/v2/applicationset/generators"
"github.com/argoproj/argo-cd/v2/applicationset/metrics"
Expand Down Expand Up @@ -577,7 +579,7 @@ func (r *ApplicationSetReconciler) createOrUpdateInCluster(ctx context.Context,
var firstError error
// Creates or updates the application in appList
for _, generatedApp := range desiredApplications {
appLog := logCtx.WithFields(log.Fields{"app": generatedApp.QualifiedName()})
appLog := logCtx.WithFields(applog.GetAppLog(&generatedApp))

// Normalize to avoid fighting with the application controller.
generatedApp.Spec = *argoutil.NormalizeApplicationSpec(&generatedApp.Spec)
Expand Down Expand Up @@ -743,7 +745,7 @@ func (r *ApplicationSetReconciler) deleteInCluster(ctx context.Context, logCtx *
// Delete apps that are not in m[string]bool
var firstError error
for _, app := range current {
logCtx = logCtx.WithField("app", app.QualifiedName())
logCtx = logCtx.WithFields(applog.GetAppLog(&app))
_, exists := m[app.Name]

if !exists {
Expand Down Expand Up @@ -1471,23 +1473,23 @@ func getOwnsHandlerPredicates(enableProgressiveSyncs bool) predicate.Funcs {
// if we are the owner and there is a create event, we most likely created it and do not need to
// re-reconcile
if log.IsLevelEnabled(log.DebugLevel) {
var appName string
logFields := log.Fields{"app": ""}
app, isApp := e.Object.(*argov1alpha1.Application)
if isApp {
appName = app.QualifiedName()
logFields = applog.GetAppLog(app)
}
log.WithField("app", appName).Debugln("received create event from owning an application")
log.WithFields(logFields).Debugln("received create event from owning an application")
}
return false
},
DeleteFunc: func(e event.DeleteEvent) bool {
if log.IsLevelEnabled(log.DebugLevel) {
var appName string
logFields := log.Fields{"app": ""}
app, isApp := e.Object.(*argov1alpha1.Application)
if isApp {
appName = app.QualifiedName()
logFields = applog.GetAppLog(app)
}
log.WithField("app", appName).Debugln("received delete event from owning an application")
log.WithFields(logFields).Debugln("received delete event from owning an application")
}
return true
},
Expand All @@ -1496,7 +1498,7 @@ func getOwnsHandlerPredicates(enableProgressiveSyncs bool) predicate.Funcs {
if !isApp {
return false
}
logCtx := log.WithField("app", appOld.QualifiedName())
logCtx := log.WithFields(applog.GetAppLog(appOld))
logCtx.Debugln("received update event from owning an application")
appNew, isApp := e.ObjectNew.(*argov1alpha1.Application)
if !isApp {
Expand All @@ -1508,12 +1510,12 @@ func getOwnsHandlerPredicates(enableProgressiveSyncs bool) predicate.Funcs {
},
GenericFunc: func(e event.GenericEvent) bool {
if log.IsLevelEnabled(log.DebugLevel) {
var appName string
logFields := log.Fields{"app": ""}
app, isApp := e.Object.(*argov1alpha1.Application)
if isApp {
appName = app.QualifiedName()
logFields = applog.GetAppLog(app)
}
log.WithField("app", appName).Debugln("received generic event from owning an application")
log.WithFields(logFields).Debugln("received generic event from owning an application")
}
return true
},
Expand Down
6 changes: 4 additions & 2 deletions applicationset/controllers/applicationset_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
"sigs.k8s.io/controller-runtime/pkg/event"

applog "github.com/argoproj/argo-cd/v2/util/app/log"

"github.com/argoproj/gitops-engine/pkg/health"
"github.com/argoproj/gitops-engine/pkg/sync/common"

Expand Down Expand Up @@ -1181,7 +1183,7 @@ func TestRemoveFinalizerOnInvalidDestination_FinalizerTypes(t *testing.T) {
clusterList, err := utils.ListClusters(context.Background(), kubeclientset, "namespace")
require.NoError(t, err)

appLog := log.WithFields(log.Fields{"app": app.Name, "appSet": ""})
appLog := log.WithFields(applog.GetAppLog(&app)).WithField("appSet", "")

appInputParam := app.DeepCopy()

Expand Down Expand Up @@ -1338,7 +1340,7 @@ func TestRemoveFinalizerOnInvalidDestination_DestinationTypes(t *testing.T) {
clusterList, err := utils.ListClusters(context.Background(), kubeclientset, "argocd")
require.NoError(t, err)

appLog := log.WithFields(log.Fields{"app": app.Name, "appSet": ""})
appLog := log.WithFields(applog.GetAppLog(&app)).WithField("appSet", "")

appInputParam := app.DeepCopy()

Expand Down
55 changes: 24 additions & 31 deletions controller/appcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ import (
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/util/workqueue"

applog "github.com/argoproj/argo-cd/v2/util/app/log"

commitclient "github.com/argoproj/argo-cd/v2/commitserver/apiclient"
"github.com/argoproj/argo-cd/v2/common"
statecache "github.com/argoproj/argo-cd/v2/controller/cache"
Expand Down Expand Up @@ -98,15 +100,6 @@ func (a CompareWith) Pointer() *CompareWith {
return &a
}

func getAppLog(app *appv1.Application) *log.Entry {
return log.WithFields(log.Fields{
"application": app.Name,
"app-namespace": app.Namespace,
"app-qualified-name": app.QualifiedName(),
"project": app.Spec.Project,
})
}

// ApplicationController is the controller for application resources.
type ApplicationController struct {
cache *appstatecache.Cache
Expand Down Expand Up @@ -439,7 +432,7 @@ func (ctrl *ApplicationController) handleObjectUpdated(managedByApp map[string]b
continue
}

logCtx := getAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))
// Enforce application's permission for the source namespace
_, err = ctrl.getAppProj(app)
if err != nil {
Expand Down Expand Up @@ -475,7 +468,7 @@ func (ctrl *ApplicationController) handleObjectUpdated(managedByApp map[string]b
func (ctrl *ApplicationController) setAppManagedResources(a *appv1.Application, comparisonResult *comparisonResult) (*appv1.ApplicationTree, error) {
ts := stats.NewTimingStats()
defer func() {
logCtx := getAppLog(a)
logCtx := log.WithFields(applog.GetAppLog(a))
for k, v := range ts.Timings() {
logCtx = logCtx.WithField(k, v.Milliseconds())
}
Expand Down Expand Up @@ -532,7 +525,7 @@ func isKnownOrphanedResourceExclusion(key kube.ResourceKey, proj *appv1.AppProje
func (ctrl *ApplicationController) getResourceTree(a *appv1.Application, managedResources []*appv1.ResourceDiff) (*appv1.ApplicationTree, error) {
ts := stats.NewTimingStats()
defer func() {
logCtx := getAppLog(a)
logCtx := log.WithFields(applog.GetAppLog(a))
for k, v := range ts.Timings() {
logCtx = logCtx.WithField(k, v.Milliseconds())
}
Expand Down Expand Up @@ -662,7 +655,7 @@ func (ctrl *ApplicationController) getResourceTree(a *appv1.Application, managed
func (ctrl *ApplicationController) getAppHosts(a *appv1.Application, appNodes []appv1.ResourceNode) ([]appv1.HostInfo, error) {
ts := stats.NewTimingStats()
defer func() {
logCtx := getAppLog(a)
logCtx := log.WithFields(applog.GetAppLog(a))
for k, v := range ts.Timings() {
logCtx = logCtx.WithField(k, v.Milliseconds())
}
Expand Down Expand Up @@ -990,7 +983,7 @@ func (ctrl *ApplicationController) processAppOperationQueueItem() (processNext b
return
}
app := origApp.DeepCopy()
logCtx := getAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))
ts := stats.NewTimingStats()
defer func() {
for k, v := range ts.Timings() {
Expand Down Expand Up @@ -1154,7 +1147,7 @@ func (ctrl *ApplicationController) getPermittedAppLiveObjects(app *appv1.Applica
}

func (ctrl *ApplicationController) isValidDestination(app *appv1.Application) (bool, *appv1.Cluster) {
logCtx := getAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))
// Validate the cluster using the Application destination's `name` field, if applicable,
// and set the Server field, if needed.
if err := argo.ValidateDestination(context.Background(), &app.Spec.Destination, ctrl.db); err != nil {
Expand All @@ -1171,7 +1164,7 @@ func (ctrl *ApplicationController) isValidDestination(app *appv1.Application) (b
}

func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Application, projectClusters func(project string) ([]*appv1.Cluster, error)) error {
logCtx := getAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))
// Get refreshed application info, since informer app copy might be stale
app, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications(app.Namespace).Get(context.Background(), app.Name, metav1.GetOptions{})
if err != nil {
Expand Down Expand Up @@ -1329,7 +1322,7 @@ func (ctrl *ApplicationController) updateFinalizers(app *appv1.Application) erro
}

func (ctrl *ApplicationController) setAppCondition(app *appv1.Application, condition appv1.ApplicationCondition) {
logCtx := getAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))
// do nothing if app already has same condition
for _, c := range app.Status.Conditions {
if c.Message == condition.Message && c.Type == condition.Type {
Expand All @@ -1354,7 +1347,7 @@ func (ctrl *ApplicationController) setAppCondition(app *appv1.Application, condi
}

func (ctrl *ApplicationController) processRequestedAppOperation(app *appv1.Application) {
logCtx := getAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))
var state *appv1.OperationState
// Recover from any unexpected panics and automatically set the status to be failed
defer func() {
Expand Down Expand Up @@ -1489,7 +1482,7 @@ func (ctrl *ApplicationController) processRequestedAppOperation(app *appv1.Appli
}

func (ctrl *ApplicationController) setOperationState(app *appv1.Application, state *appv1.OperationState) {
logCtx := getAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))
if state.Phase == "" {
// expose any bugs where we neglect to set phase
panic("no phase was set")
Expand Down Expand Up @@ -1567,7 +1560,7 @@ func (ctrl *ApplicationController) setOperationState(app *appv1.Application, sta
// writeBackToInformer writes a just recently updated App back into the informer cache.
// This prevents the situation where the controller operates on a stale app and repeats work
func (ctrl *ApplicationController) writeBackToInformer(app *appv1.Application) {
logCtx := getAppLog(app).WithField("informer-writeBack", true)
logCtx := log.WithFields(applog.GetAppLog(app)).WithField("informer-writeBack", true)
err := ctrl.appInformer.GetStore().Update(app)
if err != nil {
logCtx.Errorf("failed to update informer store: %v", err)
Expand Down Expand Up @@ -1624,7 +1617,7 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo
return
}
app := origApp.DeepCopy()
logCtx := getAppLog(app).WithFields(log.Fields{
logCtx := log.WithFields(applog.GetAppLog(app)).WithFields(log.Fields{
"comparison-level": comparisonLevel,
"dest-server": origApp.Spec.Destination.Server,
"dest-name": origApp.Spec.Destination.Name,
Expand Down Expand Up @@ -1826,7 +1819,7 @@ func (ctrl *ApplicationController) processAppHydrateQueueItem() (processNext boo

ctrl.hydrator.ProcessAppHydrateQueueItem(origApp)

getAppLog(origApp).Debug("Successfully processed app hydrate queue item")
log.WithFields(applog.GetAppLog(origApp)).Debug("Successfully processed app hydrate queue item")
return
}

Expand Down Expand Up @@ -1875,7 +1868,7 @@ func currentSourceEqualsSyncedSource(app *appv1.Application) bool {
// Additionally, it returns whether full refresh was requested or not.
// If full refresh is requested then target and live state should be reconciled, else only live state tree should be updated.
func (ctrl *ApplicationController) needRefreshAppStatus(app *appv1.Application, statusRefreshTimeout, statusHardRefreshTimeout time.Duration) (bool, appv1.RefreshType, CompareWith) {
logCtx := getAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))
var reason string
compareWith := CompareWithLatest
refreshType := appv1.RefreshTypeNormal
Expand Down Expand Up @@ -1953,7 +1946,7 @@ func (ctrl *ApplicationController) refreshAppConditions(app *appv1.Application)
// normalizeApplication normalizes an application.spec and additionally persists updates if it changed
func (ctrl *ApplicationController) normalizeApplication(orig, app *appv1.Application) {
app.Spec = *argo.NormalizeApplicationSpec(&app.Spec)
logCtx := getAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))

patch, modified, err := diff.CreateTwoWayMergePatch(orig, app, appv1.Application{})

Expand Down Expand Up @@ -1987,7 +1980,7 @@ func createMergePatch(orig, new any) ([]byte, bool, error) {

// persistAppStatus persists updates to application status. If no changes were made, it is a no-op
func (ctrl *ApplicationController) persistAppStatus(orig *appv1.Application, newStatus *appv1.ApplicationStatus) (patchMs time.Duration) {
logCtx := getAppLog(orig)
logCtx := log.WithFields(applog.GetAppLog(orig))
if orig.Status.Sync.Status != newStatus.Sync.Status {
message := fmt.Sprintf("Updated sync status: %s -> %s", orig.Status.Sync.Status, newStatus.Sync.Status)
ctrl.logAppEvent(context.TODO(), orig, argo.EventInfo{Reason: argo.EventReasonResourceUpdated, Type: corev1.EventTypeNormal}, message)
Expand Down Expand Up @@ -2032,7 +2025,7 @@ func (ctrl *ApplicationController) persistAppStatus(orig *appv1.Application, new

// autoSync will initiate a sync operation for an application configured with automated sync
func (ctrl *ApplicationController) autoSync(app *appv1.Application, syncStatus *appv1.SyncStatus, resources []appv1.ResourceStatus, revisionUpdated bool) (*appv1.ApplicationCondition, time.Duration) {
logCtx := getAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))
ts := stats.NewTimingStats()
defer func() {
for k, v := range ts.Timings() {
Expand Down Expand Up @@ -2193,16 +2186,16 @@ func alreadyAttemptedSync(app *appv1.Application, commitSHA string, commitSHAsMS
return false, ""
}
} else {
log.WithField("application", app.Name).Debugf("Skipping auto-sync: commitSHA %s has no changes", commitSHA)
log.WithFields(applog.GetAppLog(app)).Debugf("Skipping auto-sync: commitSHA %s has no changes", commitSHA)
}
} else {
if revisionUpdated {
log.WithField("application", app.Name).Infof("Executing compare of syncResult.Revision and commitSha because manifest changed: %v", commitSHA)
log.WithFields(applog.GetAppLog(app)).Infof("Executing compare of syncResult.Revision and commitSha because manifest changed: %v", commitSHA)
if app.Status.OperationState.SyncResult.Revision != commitSHA {
return false, ""
}
} else {
log.WithField("application", app.Name).Debugf("Skipping auto-sync: commitSHA %s has no changes", commitSHA)
log.WithFields(applog.GetAppLog(app)).Debugf("Skipping auto-sync: commitSHA %s has no changes", commitSHA)
}
}

Expand Down Expand Up @@ -2261,7 +2254,7 @@ func (ctrl *ApplicationController) canProcessApp(obj any) bool {

if annotations := app.GetAnnotations(); annotations != nil {
if skipVal, ok := annotations[common.AnnotationKeyAppSkipReconcile]; ok {
logCtx := getAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))
if skipReconcile, err := strconv.ParseBool(skipVal); err == nil {
if skipReconcile {
logCtx.Debugf("Skipping Application reconcile based on annotation %s", common.AnnotationKeyAppSkipReconcile)
Expand Down Expand Up @@ -2396,7 +2389,7 @@ func (ctrl *ApplicationController) newApplicationInformerAndLister() (cache.Shar
newApp, newOK := new.(*appv1.Application)
if oldOK && newOK {
if automatedSyncEnabled(oldApp, newApp) {
getAppLog(newApp).Info("Enabled automated sync")
log.WithFields(applog.GetAppLog(newApp)).Info("Enabled automated sync")
compareWith = CompareWithLatest.Pointer()
}
if ctrl.statusRefreshJitter != 0 && oldApp.ResourceVersion == newApp.ResourceVersion {
Expand Down
4 changes: 3 additions & 1 deletion controller/health.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"

applog "github.com/argoproj/argo-cd/v2/util/app/log"

"github.com/argoproj/argo-cd/v2/common"
"github.com/argoproj/argo-cd/v2/pkg/apis/application"
appv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
Expand Down Expand Up @@ -51,7 +53,7 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource
errCount++
savedErr = fmt.Errorf("failed to get resource health for %q with name %q in namespace %q: %w", res.Live.GetKind(), res.Live.GetName(), res.Live.GetNamespace(), err)
// also log so we don't lose the message
log.WithField("application", app.QualifiedName()).Warn(savedErr)
log.WithFields(applog.GetAppLog(app)).Warn(savedErr)
}
}

Expand Down
7 changes: 4 additions & 3 deletions controller/hydrator/hydrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"

applog "github.com/argoproj/argo-cd/v2/util/app/log"

commitclient "github.com/argoproj/argo-cd/v2/commitserver/apiclient"
"github.com/argoproj/argo-cd/v2/controller/utils"
appv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/argoproj/argo-cd/v2/reposerver/apiclient"
argoio "github.com/argoproj/argo-cd/v2/util/io"
Expand Down Expand Up @@ -55,7 +56,7 @@ func (h *Hydrator) ProcessAppHydrateQueueItem(origApp *appv1.Application) {
return
}

logCtx := utils.GetAppLog(app)
logCtx := log.WithFields(applog.GetAppLog(app))

logCtx.Debug("Processing app hydrate queue item")

Expand Down Expand Up @@ -127,7 +128,7 @@ func (h *Hydrator) ProcessHydrationQueueItem(hydrationKey HydrationQueueKey) (pr
app.Status.SourceHydrator.CurrentOperation.FinishedAt = &failedAt
app.Status.SourceHydrator.CurrentOperation.Message = fmt.Sprintf("Failed to hydrated revision %s: %v", drySHA, err.Error())
h.dependencies.PersistAppHydratorStatus(origApp, &app.Status.SourceHydrator)
logCtx = logCtx.WithField("app", app.QualifiedName())
logCtx = logCtx.WithFields(applog.GetAppLog(app))
logCtx.Errorf("Failed to hydrate app: %v", err)
}
return
Expand Down
6 changes: 4 additions & 2 deletions controller/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import (
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/cache"

applog "github.com/argoproj/argo-cd/v2/util/app/log"

"github.com/argoproj/argo-cd/v2/common"
statecache "github.com/argoproj/argo-cd/v2/controller/cache"
"github.com/argoproj/argo-cd/v2/controller/metrics"
Expand Down Expand Up @@ -301,7 +303,7 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp
}

ts.AddCheckpoint("manifests_ms")
logCtx := log.WithField("application", app.QualifiedName())
logCtx := log.WithFields(applog.GetAppLog(app))
for k, v := range ts.Timings() {
logCtx = logCtx.WithField(k, v.Milliseconds())
}
Expand Down Expand Up @@ -509,7 +511,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
failedToLoadObjs := false
conditions := make([]v1alpha1.ApplicationCondition, 0)

logCtx := log.WithField("application", app.QualifiedName())
logCtx := log.WithFields(applog.GetAppLog(app))
logCtx.Infof("Comparing app state (cluster: %s, namespace: %s)", app.Spec.Destination.Server, app.Spec.Destination.Namespace)

var targetObjs []*unstructured.Unstructured
Expand Down
Loading

0 comments on commit d4f077a

Please sign in to comment.