From 7f4a9d5280e742cadf509afcfe418285bdb5118c Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Thu, 24 Oct 2024 14:24:09 +0300 Subject: [PATCH 1/5] chore (event-reporter): replaced redundant variables with dedicated MetricTimer structure --- event_reporter/metrics/utils/utils.go | 19 +++++++++++++++++++ .../reporter/application_event_reporter.go | 11 +++++------ 2 files changed, 24 insertions(+), 6 deletions(-) create mode 100644 event_reporter/metrics/utils/utils.go diff --git a/event_reporter/metrics/utils/utils.go b/event_reporter/metrics/utils/utils.go new file mode 100644 index 0000000000000..bf9155a852b70 --- /dev/null +++ b/event_reporter/metrics/utils/utils.go @@ -0,0 +1,19 @@ +package metrics_utils + +import ( + "time" +) + +type MetricTimer struct { + startAt time.Time +} + +func NewMetricTimer() *MetricTimer { + return &MetricTimer{ + startAt: time.Now(), + } +} + +func (m *MetricTimer) Duration() time.Duration { + return time.Since(m.startAt) +} diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index 35c3821f165d6..b5a01b13446e5 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + metrics_utils "github.com/argoproj/argo-cd/v2/event_reporter/metrics/utils" "math" "reflect" "strings" @@ -115,9 +116,9 @@ func (s *applicationEventReporter) StreamApplicationEvents( appInstanceLabelKey string, trackingMethod appv1.TrackingMethod, ) error { - startTime := time.Now() - logCtx := log.WithField("app", a.Name) + metricTimer := metrics_utils.NewMetricTimer() + logCtx := log.WithField("app", a.Name) logCtx.WithField("ignoreResourceCache", ignoreResourceCache).Info("streaming application events") project := a.Spec.GetProject() @@ -180,8 +181,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( s.metricsServer.IncErroredEventsCounter(metrics.MetricChildAppEventType, metrics.MetricEventUnknownErrorType, a.Name) return err } - reconcileDuration := time.Since(startTime) - s.metricsServer.ObserveEventProcessingDurationHistogramDuration(a.Name, metrics.MetricChildAppEventType, reconcileDuration) + s.metricsServer.ObserveEventProcessingDurationHistogramDuration(a.Name, metrics.MetricChildAppEventType, metricTimer.Duration()) } else { logCtx.Info("processing as root application") // will get here only for root applications (not managed as a resource by another application) @@ -201,8 +201,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( s.metricsServer.IncErroredEventsCounter(metrics.MetricParentAppEventType, metrics.MetricEventDeliveryErrorType, a.Name) return fmt.Errorf("failed to send event for root application %s/%s: %w", a.Namespace, a.Name, err) } - reconcileDuration := time.Since(startTime) - s.metricsServer.ObserveEventProcessingDurationHistogramDuration(a.Name, metrics.MetricParentAppEventType, reconcileDuration) + s.metricsServer.ObserveEventProcessingDurationHistogramDuration(a.Name, metrics.MetricParentAppEventType, metricTimer.Duration()) } revisionsMetadata, _ := s.getApplicationRevisionsMetadata(ctx, logCtx, a) From 967ef1583ca5ba005f0f17c0add21583db38a89f Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Thu, 24 Oct 2024 14:05:28 +0300 Subject: [PATCH 2/5] chore (event-reporter): renamed variable from ts -> eventProcessingStartedAt --- event_reporter/controller/controller.go | 8 ++++---- .../reporter/application_event_reporter.go | 18 +++++++++--------- event_reporter/reporter/event_payload.go | 8 ++++---- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/event_reporter/controller/controller.go b/event_reporter/controller/controller.go index 10a7b92fa5dab..7ce4f100b4411 100644 --- a/event_reporter/controller/controller.go +++ b/event_reporter/controller/controller.go @@ -65,7 +65,7 @@ func (c *eventReporterController) Run(ctx context.Context) { // sendIfPermitted is a helper to send the application to the client's streaming channel if the // caller has RBAC privileges permissions to view it - sendIfPermitted := func(ctx context.Context, a appv1.Application, eventType watch.EventType, ts string, ignoreResourceCache bool) error { + sendIfPermitted := func(ctx context.Context, a appv1.Application, eventType watch.EventType, eventProcessingStartedAt string, ignoreResourceCache bool) error { if eventType == watch.Bookmark { return nil // ignore this event } @@ -76,7 +76,7 @@ func (c *eventReporterController) Run(ctx context.Context) { } trackingMethod := argoutil.GetTrackingMethod(c.settingsMgr) - err = c.applicationEventReporter.StreamApplicationEvents(ctx, &a, ts, ignoreResourceCache, appInstanceLabelKey, trackingMethod) + err = c.applicationEventReporter.StreamApplicationEvents(ctx, &a, eventProcessingStartedAt, ignoreResourceCache, appInstanceLabelKey, trackingMethod) if err != nil { return err } @@ -105,9 +105,9 @@ func (c *eventReporterController) Run(ctx context.Context) { c.metricsServer.IncCachedIgnoredEventsCounter(metrics.MetricAppEventType, event.Application.Name) continue } - ts := time.Now().Format("2006-01-02T15:04:05.000Z") + eventProcessingStartedAt := time.Now().Format("2006-01-02T15:04:05.000Z") ctx, cancel := context.WithTimeout(ctx, 2*time.Minute) - err := sendIfPermitted(ctx, event.Application, event.Type, ts, ignoreResourceCache) + err := sendIfPermitted(ctx, event.Application, event.Type, eventProcessingStartedAt, ignoreResourceCache) if err != nil { logCtx.WithError(err).Error("failed to stream application events") if strings.Contains(err.Error(), "context deadline exceeded") { diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index b5a01b13446e5..490f14e5dbe67 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -46,7 +46,7 @@ type ApplicationEventReporter interface { StreamApplicationEvents( ctx context.Context, a *appv1.Application, - ts string, + eventProcessingStartedAt string, ignoreResourceCache bool, appInstanceLabelKey string, trackingMethod appv1.TrackingMethod, @@ -111,7 +111,7 @@ func (r *applicationEventReporter) getDesiredManifests(ctx context.Context, a *a func (s *applicationEventReporter) StreamApplicationEvents( ctx context.Context, a *appv1.Application, - ts string, + eventProcessingStartedAt string, ignoreResourceCache bool, appInstanceLabelKey string, trackingMethod appv1.TrackingMethod, @@ -176,7 +176,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( } utils.SetHealthStatusIfMissing(rs) - err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, ts, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) + err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, eventProcessingStartedAt, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricChildAppEventType, metrics.MetricEventUnknownErrorType, a.Name) return err @@ -185,7 +185,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( } else { logCtx.Info("processing as root application") // will get here only for root applications (not managed as a resource by another application) - appEvent, err := s.getApplicationEventPayload(ctx, a, appTree, ts, appInstanceLabelKey, trackingMethod, applicationVersions) + appEvent, err := s.getApplicationEventPayload(ctx, a, appTree, eventProcessingStartedAt, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricParentAppEventType, metrics.MetricEventGetPayloadErrorType, a.Name) return fmt.Errorf("failed to get application event: %w", err) @@ -196,7 +196,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( return nil } - utils.LogWithAppStatus(a, logCtx, ts).Info("sending root application event") + utils.LogWithAppStatus(a, logCtx, eventProcessingStartedAt).Info("sending root application event") if err := s.codefreshClient.SendEvent(ctx, a.Name, appEvent); err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricParentAppEventType, metrics.MetricEventDeliveryErrorType, a.Name) return fmt.Errorf("failed to send event for root application %s/%s: %w", a.Namespace, a.Name, err) @@ -216,7 +216,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( s.metricsServer.IncCachedIgnoredEventsCounter(metrics.MetricResourceEventType, a.Name) continue } - err := s.processResource(ctx, rs, a, logCtx, ts, desiredManifests, appTree, manifestGenErr, nil, revisionsMetadata, appInstanceLabelKey, trackingMethod, nil) + err := s.processResource(ctx, rs, a, logCtx, eventProcessingStartedAt, desiredManifests, appTree, manifestGenErr, nil, revisionsMetadata, appInstanceLabelKey, trackingMethod, nil) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricResourceEventType, metrics.MetricEventUnknownErrorType, a.Name) return err @@ -254,7 +254,7 @@ func (s *applicationEventReporter) processResource( rs appv1.ResourceStatus, parentApplication *appv1.Application, logCtx *log.Entry, - ts string, + appEventProcessingStartedAt string, desiredManifests *apiclient.ManifestResponse, appTree *appv1.ApplicationTree, manifestGenErr bool, @@ -293,7 +293,7 @@ func (s *applicationEventReporter) processResource( originalAppRevisionMetadata, _ = s.getApplicationRevisionsMetadata(ctx, logCtx, originalApplication) } - ev, err := getResourceEventPayload(parentApplicationToReport, &rs, actualState, desiredState, appTree, manifestGenErr, ts, originalApplication, revisionMetadataToReport, originalAppRevisionMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) + ev, err := getResourceEventPayload(parentApplicationToReport, &rs, actualState, desiredState, appTree, manifestGenErr, appEventProcessingStartedAt, originalApplication, revisionMetadataToReport, originalAppRevisionMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metricsEventType, metrics.MetricEventGetPayloadErrorType, parentApplication.Name) logCtx.WithError(err).Warn("failed to get event payload, resuming") @@ -303,7 +303,7 @@ func (s *applicationEventReporter) processResource( appRes := appv1.Application{} appName := "" if utils.IsApp(rs) && actualState.Manifest != nil && json.Unmarshal([]byte(*actualState.Manifest), &appRes) == nil { - utils.LogWithAppStatus(&appRes, logCtx, ts).Info("streaming resource event") + utils.LogWithAppStatus(&appRes, logCtx, appEventProcessingStartedAt).Info("streaming resource event") appName = appRes.Name } else { utils.LogWithResourceStatus(logCtx, rs).Info("streaming resource event") diff --git a/event_reporter/reporter/event_payload.go b/event_reporter/reporter/event_payload.go index cbac9a9031d32..9aeda6fdbc517 100644 --- a/event_reporter/reporter/event_payload.go +++ b/event_reporter/reporter/event_payload.go @@ -26,7 +26,7 @@ func getResourceEventPayload( desiredState *apiclient.Manifest, apptree *appv1.ApplicationTree, manifestGenErr bool, - ts string, + appEventProcessingStartedAt string, originalApplication *appv1.Application, // passed when rs is application revisionsMetadata *utils.AppSyncRevisionsMetadata, originalAppRevisionsMetadata *utils.AppSyncRevisionsMetadata, // passed when rs is application @@ -194,7 +194,7 @@ func getResourceEventPayload( } payload := events.EventPayload{ - Timestamp: ts, + Timestamp: appEventProcessingStartedAt, Object: object, Source: &source, Errors: errors, @@ -215,7 +215,7 @@ func (s *applicationEventReporter) getApplicationEventPayload( ctx context.Context, a *appv1.Application, appTree *appv1.ApplicationTree, - ts string, + eventProcessingStartedAt string, appInstanceLabelKey string, trackingMethod appv1.TrackingMethod, applicationVersions *apiclient.ApplicationVersions, @@ -295,7 +295,7 @@ func (s *applicationEventReporter) getApplicationEventPayload( errors = append(errors, parseAggregativeHealthErrorsOfApplication(a, appTree)...) payload := events.EventPayload{ - Timestamp: ts, + Timestamp: eventProcessingStartedAt, Object: object, Source: source, Errors: errors, From 35bdffadb493ad5d710af7da8bc2daa45232a543 Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Thu, 24 Oct 2024 21:25:58 +0300 Subject: [PATCH 3/5] chore (event-reporter): renamed logCtx to more appropriately named variables --- event_reporter/reporter/app_revision.go | 6 +-- .../reporter/application_event_reporter.go | 54 +++++++++---------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/event_reporter/reporter/app_revision.go b/event_reporter/reporter/app_revision.go index b4c68f346325f..e8724d9b3a810 100644 --- a/event_reporter/reporter/app_revision.go +++ b/event_reporter/reporter/app_revision.go @@ -63,14 +63,14 @@ func (s *applicationEventReporter) getRevisionsDetails(ctx context.Context, a *v return rms, nil } -func (s *applicationEventReporter) getApplicationRevisionsMetadata(ctx context.Context, logCtx *log.Entry, a *v1alpha1.Application) (*utils.AppSyncRevisionsMetadata, error) { +func (s *applicationEventReporter) getApplicationRevisionsMetadata(ctx context.Context, logWithAppName *log.Entry, a *v1alpha1.Application) (*utils.AppSyncRevisionsMetadata, error) { result := &utils.AppSyncRevisionsMetadata{} if a.Status.Sync.Revision != "" || a.Status.Sync.Revisions != nil || (a.Status.History != nil && len(a.Status.History) > 0) { // can be the latest revision of repository operationSyncRevisionsMetadata, err := s.getRevisionsDetails(ctx, a, utils.GetOperationSyncRevisions(a)) if err != nil { - logCtx.WithError(err).Warnf("failed to get application(%s) sync revisions metadata, resuming", a.GetName()) + logWithAppName.WithError(err).Warnf("failed to get application(%s) sync revisions metadata, resuming", a.GetName()) } if err == nil && operationSyncRevisionsMetadata != nil { @@ -79,7 +79,7 @@ func (s *applicationEventReporter) getApplicationRevisionsMetadata(ctx context.C // latest revision of repository where changes to app resource were actually made; empty if no changeRevision(-s) present operationChangeRevisionsMetadata, err := s.getRevisionsDetails(ctx, a, utils.GetOperationChangeRevisions(a)) if err != nil { - logCtx.WithError(err).Warnf("failed to get application(%s) change revisions metadata, resuming", a.GetName()) + logWithAppName.WithError(err).Warnf("failed to get application(%s) change revisions metadata, resuming", a.GetName()) } if err == nil && operationChangeRevisionsMetadata != nil && len(operationChangeRevisionsMetadata) > 0 { diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index 490f14e5dbe67..cc6f1da6c2cb4 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -118,8 +118,8 @@ func (s *applicationEventReporter) StreamApplicationEvents( ) error { metricTimer := metrics_utils.NewMetricTimer() - logCtx := log.WithField("app", a.Name) - logCtx.WithField("ignoreResourceCache", ignoreResourceCache).Info("streaming application events") + logWithAppName := log.WithField("app", a.Name) + logWithAppName.WithField("ignoreResourceCache", ignoreResourceCache).Info("streaming application events") project := a.Spec.GetProject() appTree, err := s.applicationServiceClient.ResourceTree(ctx, &application.ResourcesQuery{ @@ -134,28 +134,28 @@ func (s *applicationEventReporter) StreamApplicationEvents( // we still need process app even without tree, it is in case of app yaml originally contain error, // we still want to show it the errors that related to it on codefresh ui - logCtx.WithError(err).Warn("failed to get application tree, resuming") + logWithAppName.WithError(err).Warn("failed to get application tree, resuming") } - logCtx.Info("getting desired manifests") + logWithAppName.Info("getting desired manifests") - desiredManifests, manifestGenErr := s.getDesiredManifests(ctx, a, nil, logCtx) + desiredManifests, manifestGenErr := s.getDesiredManifests(ctx, a, nil, logWithAppName) syncRevision := utils.GetOperationStateRevision(a) var applicationVersions *apiclient.ApplicationVersions if syncRevision != nil { - syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logCtx) + syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logWithAppName) applicationVersions = syncManifests.GetApplicationVersions() } else { applicationVersions = nil } - logCtx.Info("getting parent application name") + logWithAppName.Info("getting parent application name") parentAppIdentity := utils.GetParentAppIdentity(a, appInstanceLabelKey, trackingMethod) if utils.IsChildApp(parentAppIdentity) { - logCtx.Info("processing as child application") + logWithAppName.Info("processing as child application") parentApplicationEntity, err := s.applicationServiceClient.Get(ctx, &application.ApplicationQuery{ Name: &parentAppIdentity.Name, AppNamespace: &parentAppIdentity.Namespace, @@ -166,24 +166,24 @@ func (s *applicationEventReporter) StreamApplicationEvents( rs := utils.GetAppAsResource(a) - parentDesiredManifests, manifestGenErr := s.getDesiredManifests(ctx, parentApplicationEntity, nil, logCtx) + parentDesiredManifests, manifestGenErr := s.getDesiredManifests(ctx, parentApplicationEntity, nil, logWithAppName) // helm app hasnt revision // TODO: add check if it helm application - parentAppSyncRevisionsMetadata, err := s.getApplicationRevisionsMetadata(ctx, logCtx, parentApplicationEntity) + parentAppSyncRevisionsMetadata, err := s.getApplicationRevisionsMetadata(ctx, logWithAppName, parentApplicationEntity) if err != nil { - logCtx.WithError(err).Warn("failed to get parent application's revision metadata, resuming") + logWithAppName.WithError(err).Warn("failed to get parent application's revision metadata, resuming") } utils.SetHealthStatusIfMissing(rs) - err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, eventProcessingStartedAt, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) + err = s.processResource(ctx, *rs, parentApplicationEntity, logWithAppName, eventProcessingStartedAt, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricChildAppEventType, metrics.MetricEventUnknownErrorType, a.Name) return err } s.metricsServer.ObserveEventProcessingDurationHistogramDuration(a.Name, metrics.MetricChildAppEventType, metricTimer.Duration()) } else { - logCtx.Info("processing as root application") + logWithAppName.Info("processing as root application") // will get here only for root applications (not managed as a resource by another application) appEvent, err := s.getApplicationEventPayload(ctx, a, appTree, eventProcessingStartedAt, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { @@ -196,7 +196,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( return nil } - utils.LogWithAppStatus(a, logCtx, eventProcessingStartedAt).Info("sending root application event") + utils.LogWithAppStatus(a, logWithAppName, eventProcessingStartedAt).Info("sending root application event") if err := s.codefreshClient.SendEvent(ctx, a.Name, appEvent); err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricParentAppEventType, metrics.MetricEventDeliveryErrorType, a.Name) return fmt.Errorf("failed to send event for root application %s/%s: %w", a.Namespace, a.Name, err) @@ -204,7 +204,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( s.metricsServer.ObserveEventProcessingDurationHistogramDuration(a.Name, metrics.MetricParentAppEventType, metricTimer.Duration()) } - revisionsMetadata, _ := s.getApplicationRevisionsMetadata(ctx, logCtx, a) + revisionsMetadata, _ := s.getApplicationRevisionsMetadata(ctx, logWithAppName, a) // for each resource in the application get desired and actual state, // then stream the event for _, rs := range a.Status.Resources { @@ -216,7 +216,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( s.metricsServer.IncCachedIgnoredEventsCounter(metrics.MetricResourceEventType, a.Name) continue } - err := s.processResource(ctx, rs, a, logCtx, eventProcessingStartedAt, desiredManifests, appTree, manifestGenErr, nil, revisionsMetadata, appInstanceLabelKey, trackingMethod, nil) + err := s.processResource(ctx, rs, a, logWithAppName, eventProcessingStartedAt, desiredManifests, appTree, manifestGenErr, nil, revisionsMetadata, appInstanceLabelKey, trackingMethod, nil) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricResourceEventType, metrics.MetricEventUnknownErrorType, a.Name) return err @@ -253,7 +253,7 @@ func (s *applicationEventReporter) processResource( ctx context.Context, rs appv1.ResourceStatus, parentApplication *appv1.Application, - logCtx *log.Entry, + logWithAppName *log.Entry, appEventProcessingStartedAt string, desiredManifests *apiclient.ManifestResponse, appTree *appv1.ApplicationTree, @@ -269,15 +269,15 @@ func (s *applicationEventReporter) processResource( metricsEventType = metrics.MetricChildAppEventType } - logCtx = logCtx.WithFields(log.Fields{ + logWithAppAndResource := logWithAppName.WithFields(log.Fields{ "gvk": fmt.Sprintf("%s/%s/%s", rs.Group, rs.Version, rs.Kind), "resource": fmt.Sprintf("%s/%s", rs.Namespace, rs.Name), }) // get resource desired state - desiredState := getResourceDesiredState(&rs, desiredManifests, logCtx) + desiredState := getResourceDesiredState(&rs, desiredManifests, logWithAppAndResource) - actualState, err := s.getResourceActualState(ctx, logCtx, metricsEventType, rs, parentApplication, originalApplication) + actualState, err := s.getResourceActualState(ctx, logWithAppAndResource, metricsEventType, rs, parentApplication, originalApplication) if err != nil { return err } @@ -285,28 +285,28 @@ func (s *applicationEventReporter) processResource( return nil } - parentApplicationToReport, revisionMetadataToReport := s.getAppForResourceReporting(rs, ctx, logCtx, parentApplication, revisionsMetadata) + parentApplicationToReport, revisionMetadataToReport := s.getAppForResourceReporting(rs, ctx, logWithAppAndResource, parentApplication, revisionsMetadata) var originalAppRevisionMetadata *utils.AppSyncRevisionsMetadata = nil if originalApplication != nil { - originalAppRevisionMetadata, _ = s.getApplicationRevisionsMetadata(ctx, logCtx, originalApplication) + originalAppRevisionMetadata, _ = s.getApplicationRevisionsMetadata(ctx, logWithAppAndResource, originalApplication) } ev, err := getResourceEventPayload(parentApplicationToReport, &rs, actualState, desiredState, appTree, manifestGenErr, appEventProcessingStartedAt, originalApplication, revisionMetadataToReport, originalAppRevisionMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metricsEventType, metrics.MetricEventGetPayloadErrorType, parentApplication.Name) - logCtx.WithError(err).Warn("failed to get event payload, resuming") + logWithAppAndResource.WithError(err).Warn("failed to get event payload, resuming") return nil } appRes := appv1.Application{} appName := "" if utils.IsApp(rs) && actualState.Manifest != nil && json.Unmarshal([]byte(*actualState.Manifest), &appRes) == nil { - utils.LogWithAppStatus(&appRes, logCtx, appEventProcessingStartedAt).Info("streaming resource event") + utils.LogWithAppStatus(&appRes, logWithAppAndResource, appEventProcessingStartedAt).Info("streaming resource event") appName = appRes.Name } else { - utils.LogWithResourceStatus(logCtx, rs).Info("streaming resource event") + utils.LogWithResourceStatus(logWithAppAndResource, rs).Info("streaming resource event") appName = parentApplication.Name } @@ -316,12 +316,12 @@ func (s *applicationEventReporter) processResource( } s.metricsServer.IncErroredEventsCounter(metricsEventType, metrics.MetricEventDeliveryErrorType, appName) - logCtx.WithError(err).Warn("failed to send resource event, resuming") + logWithAppAndResource.WithError(err).Warn("failed to send resource event, resuming") return nil } if err := s.cache.SetLastResourceEvent(parentApplicationToReport, rs, resourceEventCacheExpiration, utils.GetApplicationLatestRevision(parentApplicationToReport)); err != nil { - logCtx.WithError(err).Warn("failed to cache resource event") + logWithAppAndResource.WithError(err).Warn("failed to cache resource event") } return nil From ca203a2305ab011b8d4b51a586868b22d2760032 Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Fri, 25 Oct 2024 12:01:38 +0300 Subject: [PATCH 4/5] Revert "chore (event-reporter): renamed logCtx to more appropriately named variables" This reverts commit 35bdffadb493ad5d710af7da8bc2daa45232a543. --- event_reporter/reporter/app_revision.go | 6 +-- .../reporter/application_event_reporter.go | 54 +++++++++---------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/event_reporter/reporter/app_revision.go b/event_reporter/reporter/app_revision.go index e8724d9b3a810..b4c68f346325f 100644 --- a/event_reporter/reporter/app_revision.go +++ b/event_reporter/reporter/app_revision.go @@ -63,14 +63,14 @@ func (s *applicationEventReporter) getRevisionsDetails(ctx context.Context, a *v return rms, nil } -func (s *applicationEventReporter) getApplicationRevisionsMetadata(ctx context.Context, logWithAppName *log.Entry, a *v1alpha1.Application) (*utils.AppSyncRevisionsMetadata, error) { +func (s *applicationEventReporter) getApplicationRevisionsMetadata(ctx context.Context, logCtx *log.Entry, a *v1alpha1.Application) (*utils.AppSyncRevisionsMetadata, error) { result := &utils.AppSyncRevisionsMetadata{} if a.Status.Sync.Revision != "" || a.Status.Sync.Revisions != nil || (a.Status.History != nil && len(a.Status.History) > 0) { // can be the latest revision of repository operationSyncRevisionsMetadata, err := s.getRevisionsDetails(ctx, a, utils.GetOperationSyncRevisions(a)) if err != nil { - logWithAppName.WithError(err).Warnf("failed to get application(%s) sync revisions metadata, resuming", a.GetName()) + logCtx.WithError(err).Warnf("failed to get application(%s) sync revisions metadata, resuming", a.GetName()) } if err == nil && operationSyncRevisionsMetadata != nil { @@ -79,7 +79,7 @@ func (s *applicationEventReporter) getApplicationRevisionsMetadata(ctx context.C // latest revision of repository where changes to app resource were actually made; empty if no changeRevision(-s) present operationChangeRevisionsMetadata, err := s.getRevisionsDetails(ctx, a, utils.GetOperationChangeRevisions(a)) if err != nil { - logWithAppName.WithError(err).Warnf("failed to get application(%s) change revisions metadata, resuming", a.GetName()) + logCtx.WithError(err).Warnf("failed to get application(%s) change revisions metadata, resuming", a.GetName()) } if err == nil && operationChangeRevisionsMetadata != nil && len(operationChangeRevisionsMetadata) > 0 { diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index cc6f1da6c2cb4..490f14e5dbe67 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -118,8 +118,8 @@ func (s *applicationEventReporter) StreamApplicationEvents( ) error { metricTimer := metrics_utils.NewMetricTimer() - logWithAppName := log.WithField("app", a.Name) - logWithAppName.WithField("ignoreResourceCache", ignoreResourceCache).Info("streaming application events") + logCtx := log.WithField("app", a.Name) + logCtx.WithField("ignoreResourceCache", ignoreResourceCache).Info("streaming application events") project := a.Spec.GetProject() appTree, err := s.applicationServiceClient.ResourceTree(ctx, &application.ResourcesQuery{ @@ -134,28 +134,28 @@ func (s *applicationEventReporter) StreamApplicationEvents( // we still need process app even without tree, it is in case of app yaml originally contain error, // we still want to show it the errors that related to it on codefresh ui - logWithAppName.WithError(err).Warn("failed to get application tree, resuming") + logCtx.WithError(err).Warn("failed to get application tree, resuming") } - logWithAppName.Info("getting desired manifests") + logCtx.Info("getting desired manifests") - desiredManifests, manifestGenErr := s.getDesiredManifests(ctx, a, nil, logWithAppName) + desiredManifests, manifestGenErr := s.getDesiredManifests(ctx, a, nil, logCtx) syncRevision := utils.GetOperationStateRevision(a) var applicationVersions *apiclient.ApplicationVersions if syncRevision != nil { - syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logWithAppName) + syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logCtx) applicationVersions = syncManifests.GetApplicationVersions() } else { applicationVersions = nil } - logWithAppName.Info("getting parent application name") + logCtx.Info("getting parent application name") parentAppIdentity := utils.GetParentAppIdentity(a, appInstanceLabelKey, trackingMethod) if utils.IsChildApp(parentAppIdentity) { - logWithAppName.Info("processing as child application") + logCtx.Info("processing as child application") parentApplicationEntity, err := s.applicationServiceClient.Get(ctx, &application.ApplicationQuery{ Name: &parentAppIdentity.Name, AppNamespace: &parentAppIdentity.Namespace, @@ -166,24 +166,24 @@ func (s *applicationEventReporter) StreamApplicationEvents( rs := utils.GetAppAsResource(a) - parentDesiredManifests, manifestGenErr := s.getDesiredManifests(ctx, parentApplicationEntity, nil, logWithAppName) + parentDesiredManifests, manifestGenErr := s.getDesiredManifests(ctx, parentApplicationEntity, nil, logCtx) // helm app hasnt revision // TODO: add check if it helm application - parentAppSyncRevisionsMetadata, err := s.getApplicationRevisionsMetadata(ctx, logWithAppName, parentApplicationEntity) + parentAppSyncRevisionsMetadata, err := s.getApplicationRevisionsMetadata(ctx, logCtx, parentApplicationEntity) if err != nil { - logWithAppName.WithError(err).Warn("failed to get parent application's revision metadata, resuming") + logCtx.WithError(err).Warn("failed to get parent application's revision metadata, resuming") } utils.SetHealthStatusIfMissing(rs) - err = s.processResource(ctx, *rs, parentApplicationEntity, logWithAppName, eventProcessingStartedAt, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) + err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, eventProcessingStartedAt, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricChildAppEventType, metrics.MetricEventUnknownErrorType, a.Name) return err } s.metricsServer.ObserveEventProcessingDurationHistogramDuration(a.Name, metrics.MetricChildAppEventType, metricTimer.Duration()) } else { - logWithAppName.Info("processing as root application") + logCtx.Info("processing as root application") // will get here only for root applications (not managed as a resource by another application) appEvent, err := s.getApplicationEventPayload(ctx, a, appTree, eventProcessingStartedAt, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { @@ -196,7 +196,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( return nil } - utils.LogWithAppStatus(a, logWithAppName, eventProcessingStartedAt).Info("sending root application event") + utils.LogWithAppStatus(a, logCtx, eventProcessingStartedAt).Info("sending root application event") if err := s.codefreshClient.SendEvent(ctx, a.Name, appEvent); err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricParentAppEventType, metrics.MetricEventDeliveryErrorType, a.Name) return fmt.Errorf("failed to send event for root application %s/%s: %w", a.Namespace, a.Name, err) @@ -204,7 +204,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( s.metricsServer.ObserveEventProcessingDurationHistogramDuration(a.Name, metrics.MetricParentAppEventType, metricTimer.Duration()) } - revisionsMetadata, _ := s.getApplicationRevisionsMetadata(ctx, logWithAppName, a) + revisionsMetadata, _ := s.getApplicationRevisionsMetadata(ctx, logCtx, a) // for each resource in the application get desired and actual state, // then stream the event for _, rs := range a.Status.Resources { @@ -216,7 +216,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( s.metricsServer.IncCachedIgnoredEventsCounter(metrics.MetricResourceEventType, a.Name) continue } - err := s.processResource(ctx, rs, a, logWithAppName, eventProcessingStartedAt, desiredManifests, appTree, manifestGenErr, nil, revisionsMetadata, appInstanceLabelKey, trackingMethod, nil) + err := s.processResource(ctx, rs, a, logCtx, eventProcessingStartedAt, desiredManifests, appTree, manifestGenErr, nil, revisionsMetadata, appInstanceLabelKey, trackingMethod, nil) if err != nil { s.metricsServer.IncErroredEventsCounter(metrics.MetricResourceEventType, metrics.MetricEventUnknownErrorType, a.Name) return err @@ -253,7 +253,7 @@ func (s *applicationEventReporter) processResource( ctx context.Context, rs appv1.ResourceStatus, parentApplication *appv1.Application, - logWithAppName *log.Entry, + logCtx *log.Entry, appEventProcessingStartedAt string, desiredManifests *apiclient.ManifestResponse, appTree *appv1.ApplicationTree, @@ -269,15 +269,15 @@ func (s *applicationEventReporter) processResource( metricsEventType = metrics.MetricChildAppEventType } - logWithAppAndResource := logWithAppName.WithFields(log.Fields{ + logCtx = logCtx.WithFields(log.Fields{ "gvk": fmt.Sprintf("%s/%s/%s", rs.Group, rs.Version, rs.Kind), "resource": fmt.Sprintf("%s/%s", rs.Namespace, rs.Name), }) // get resource desired state - desiredState := getResourceDesiredState(&rs, desiredManifests, logWithAppAndResource) + desiredState := getResourceDesiredState(&rs, desiredManifests, logCtx) - actualState, err := s.getResourceActualState(ctx, logWithAppAndResource, metricsEventType, rs, parentApplication, originalApplication) + actualState, err := s.getResourceActualState(ctx, logCtx, metricsEventType, rs, parentApplication, originalApplication) if err != nil { return err } @@ -285,28 +285,28 @@ func (s *applicationEventReporter) processResource( return nil } - parentApplicationToReport, revisionMetadataToReport := s.getAppForResourceReporting(rs, ctx, logWithAppAndResource, parentApplication, revisionsMetadata) + parentApplicationToReport, revisionMetadataToReport := s.getAppForResourceReporting(rs, ctx, logCtx, parentApplication, revisionsMetadata) var originalAppRevisionMetadata *utils.AppSyncRevisionsMetadata = nil if originalApplication != nil { - originalAppRevisionMetadata, _ = s.getApplicationRevisionsMetadata(ctx, logWithAppAndResource, originalApplication) + originalAppRevisionMetadata, _ = s.getApplicationRevisionsMetadata(ctx, logCtx, originalApplication) } ev, err := getResourceEventPayload(parentApplicationToReport, &rs, actualState, desiredState, appTree, manifestGenErr, appEventProcessingStartedAt, originalApplication, revisionMetadataToReport, originalAppRevisionMetadata, appInstanceLabelKey, trackingMethod, applicationVersions) if err != nil { s.metricsServer.IncErroredEventsCounter(metricsEventType, metrics.MetricEventGetPayloadErrorType, parentApplication.Name) - logWithAppAndResource.WithError(err).Warn("failed to get event payload, resuming") + logCtx.WithError(err).Warn("failed to get event payload, resuming") return nil } appRes := appv1.Application{} appName := "" if utils.IsApp(rs) && actualState.Manifest != nil && json.Unmarshal([]byte(*actualState.Manifest), &appRes) == nil { - utils.LogWithAppStatus(&appRes, logWithAppAndResource, appEventProcessingStartedAt).Info("streaming resource event") + utils.LogWithAppStatus(&appRes, logCtx, appEventProcessingStartedAt).Info("streaming resource event") appName = appRes.Name } else { - utils.LogWithResourceStatus(logWithAppAndResource, rs).Info("streaming resource event") + utils.LogWithResourceStatus(logCtx, rs).Info("streaming resource event") appName = parentApplication.Name } @@ -316,12 +316,12 @@ func (s *applicationEventReporter) processResource( } s.metricsServer.IncErroredEventsCounter(metricsEventType, metrics.MetricEventDeliveryErrorType, appName) - logWithAppAndResource.WithError(err).Warn("failed to send resource event, resuming") + logCtx.WithError(err).Warn("failed to send resource event, resuming") return nil } if err := s.cache.SetLastResourceEvent(parentApplicationToReport, rs, resourceEventCacheExpiration, utils.GetApplicationLatestRevision(parentApplicationToReport)); err != nil { - logWithAppAndResource.WithError(err).Warn("failed to cache resource event") + logCtx.WithError(err).Warn("failed to cache resource event") } return nil From 8f42a558e8db5d6b62388d9886e87f265df344ab Mon Sep 17 00:00:00 2001 From: oleksandr-codefresh Date: Fri, 25 Oct 2024 13:30:20 +0300 Subject: [PATCH 5/5] chore (event-reporter): fix lint issue --- event_reporter/reporter/application_event_reporter.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/event_reporter/reporter/application_event_reporter.go b/event_reporter/reporter/application_event_reporter.go index 490f14e5dbe67..a64cd1c0a1f82 100644 --- a/event_reporter/reporter/application_event_reporter.go +++ b/event_reporter/reporter/application_event_reporter.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - metrics_utils "github.com/argoproj/argo-cd/v2/event_reporter/metrics/utils" "math" "reflect" "strings" @@ -28,6 +27,7 @@ import ( "k8s.io/apimachinery/pkg/watch" appclient "github.com/argoproj/argo-cd/v2/event_reporter/application" + metricsUtils "github.com/argoproj/argo-cd/v2/event_reporter/metrics/utils" "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" appv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) @@ -116,7 +116,7 @@ func (s *applicationEventReporter) StreamApplicationEvents( appInstanceLabelKey string, trackingMethod appv1.TrackingMethod, ) error { - metricTimer := metrics_utils.NewMetricTimer() + metricTimer := metricsUtils.NewMetricTimer() logCtx := log.WithField("app", a.Name) logCtx.WithField("ignoreResourceCache", ignoreResourceCache).Info("streaming application events")