Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(logs): standardize application logging (#20731) #21442

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
Loading