final refactoring

Signed-off-by: sethiyash <[email protected]>

Author: sethiyash

cmd/controller/run.go

@@ -96,8 +96,8 @@ func Run(opts Options, runLog logr.Logger) error {
 	}
 
 	runLog.Info("setting up metrics")
-	appMetrics := metrics.NewAppMetrics()
-	appMetrics.RegisterAllMetrics()
+	countMetrics := metrics.NewCountMetrics()
+	countMetrics.RegisterAllMetrics()
 
 	reconcileTimeMetrics := metrics.NewReconcileTimeMetrics()
 	reconcileTimeMetrics.RegisterAllMetrics()
@@ -197,15 +197,15 @@ func Run(opts Options, runLog logr.Logger) error {
 	}
 	{ // add controller for apps
 		appFactory := app.CRDAppFactory{
-			CoreClient:  coreClient,
-			AppClient:   kcClient,
-			KcConfig:    kcConfig,
-			AppMetrics:  appMetrics,
-			TimeMetrics: reconcileTimeMetrics,
-			CmdRunner:   sidecarCmdExec,
-			Kubeconf:    kubeconf,
-			CompInfo:    compInfo,
-			CacheFolder: cacheFolderApps,
+			CoreClient:   coreClient,
+			AppClient:    kcClient,
+			KcConfig:     kcConfig,
+			CountMetrics: countMetrics,
+			TimeMetrics:  reconcileTimeMetrics,
+			CmdRunner:    sidecarCmdExec,
+			Kubeconf:     kubeconf,
+			CompInfo:     compInfo,
+			CacheFolder:  cacheFolderApps,
 		}
 		reconciler := app.NewReconciler(kcClient, runLog.WithName("app"),
 			appFactory, refTracker, updateStatusTracker, compInfo)
@@ -232,7 +232,7 @@ func Run(opts Options, runLog logr.Logger) error {
 			kcClient, opts.PackagingGlobalNS, runLog.WithName("handler"))
 
 		reconciler := pkginstall.NewReconciler(kcClient, pkgClient, coreClient, pkgToPkgInstallHandler,
-			runLog.WithName("pkgi"), compInfo, kcConfig, reconcileTimeMetrics)
+			runLog.WithName("pkgi"), compInfo, kcConfig, countMetrics, reconcileTimeMetrics)
 
 		ctrl, err := controller.New("pkgi", mgr, controller.Options{
 			Reconciler:              reconciler,
@@ -256,14 +256,14 @@ func Run(opts Options, runLog logr.Logger) error {
 
 	{ // add controller for pkgrepositories
 		appFactory := pkgrepository.AppFactory{
-			CoreClient:  coreClient,
-			AppClient:   kcClient,
-			KcConfig:    kcConfig,
-			AppMetrics:  appMetrics,
-			TimeMetrics: reconcileTimeMetrics,
-			CmdRunner:   sidecarCmdExec,
-			Kubeconf:    kubeconf,
-			CacheFolder: cacheFolderPkgRepoApps,
+			CoreClient:   coreClient,
+			AppClient:    kcClient,
+			KcConfig:     kcConfig,
+			CountMetrics: countMetrics,
+			TimeMetrics:  reconcileTimeMetrics,
+			CmdRunner:    sidecarCmdExec,
+			Kubeconf:     kubeconf,
+			CacheFolder:  cacheFolderPkgRepoApps,
 		}
 
 		reconciler := pkgrepository.NewReconciler(kcClient, coreClient,

pkg/app/app.go

@@ -55,24 +55,24 @@ type App struct {
 	memoizedKubernetesVersion string
 	memoizedKubernetesAPIs    []string
 
-	log         logr.Logger
-	opts        Opts
-	appMetrics  *metrics.AppMetrics
-	timeMetrics *metrics.ReconcileTimeMetrics
+	log          logr.Logger
+	opts         Opts
+	countMetrics *metrics.ReconcileCountMetrics
+	timeMetrics  *metrics.ReconcileTimeMetrics
 
-	isFirstReconcile      string
+	isFirstReconcile      bool
 	pendingStatusUpdate   bool
 	flushAllStatusUpdates bool
 	metadata              *deploy.Meta
 }
 
 func NewApp(app v1alpha1.App, hooks Hooks,
 	fetchFactory fetch.Factory, templateFactory template.Factory,
-	deployFactory deploy.Factory, log logr.Logger, opts Opts, appMetrics *metrics.AppMetrics, timeMetrics *metrics.ReconcileTimeMetrics, compInfo ComponentInfo) *App {
+	deployFactory deploy.Factory, log logr.Logger, opts Opts, appMetrics *metrics.ReconcileCountMetrics, timeMetrics *metrics.ReconcileTimeMetrics, compInfo ComponentInfo) *App {
 
 	return &App{app: app, appPrev: *(app.DeepCopy()), hooks: hooks,
 		fetchFactory: fetchFactory, templateFactory: templateFactory,
-		deployFactory: deployFactory, log: log, opts: opts, appMetrics: appMetrics, timeMetrics: timeMetrics, compInfo: compInfo}
+		deployFactory: deployFactory, log: log, opts: opts, countMetrics: appMetrics, timeMetrics: timeMetrics, compInfo: compInfo}
 }
 
 func (a *App) Name() string      { return a.app.Name }

pkg/app/app_factory.go

@@ -26,7 +26,7 @@ type CRDAppFactory struct {
 	CoreClient       kubernetes.Interface
 	AppClient        kcclient.Interface
 	KcConfig         *config.Config
-	AppMetrics       *metrics.AppMetrics
+	CountMetrics     *metrics.ReconcileCountMetrics
 	TimeMetrics      *metrics.ReconcileTimeMetrics
 	VendirConfigHook func(vendirconf.Config) vendirconf.Config
 	KbldAllowBuild   bool
@@ -49,7 +49,7 @@ func (f *CRDAppFactory) NewCRDApp(app *kcv1alpha1.App, log logr.Logger) *CRDApp
 	templateFactory := template.NewFactory(f.CoreClient, fetchFactory, f.KbldAllowBuild, f.CmdRunner)
 	deployFactory := deploy.NewFactory(f.CoreClient, f.Kubeconf, f.KcConfig, f.CmdRunner, log)
 
-	return NewCRDApp(app, log, f.AppMetrics, f.TimeMetrics, f.AppClient, fetchFactory, templateFactory, deployFactory, f.CompInfo, Opts{
+	return NewCRDApp(app, log, f.CountMetrics, f.TimeMetrics, f.AppClient, fetchFactory, templateFactory, deployFactory, f.CompInfo, Opts{
 		DefaultSyncPeriod: f.KcConfig.AppDefaultSyncPeriod(),
 		MinimumSyncPeriod: f.KcConfig.AppMinimumSyncPeriod(),
 	})

pkg/app/app_fetch.go

@@ -20,12 +20,6 @@ const (
 )
 
 func (a *App) fetch(dstPath string) (string, exec.CmdRunResult) {
-	// update metrics with the total fetch time
-	reconcileStartTS := time.Now()
-	defer func() {
-		a.timeMetrics.RegisterFetchTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS))
-	}()
-
 	// fetch init stage
 	if len(a.app.Spec.Fetch) == 0 {
 		return "", exec.NewCmdRunResultWithErr(fmt.Errorf("Expected at least one fetch option"))

pkg/app/app_reconcile.go

@@ -15,13 +15,15 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
+const appResourceType = "app"
+
 // Reconcile is not expected to be called concurrently
 func (a *App) Reconcile(force bool) (reconcile.Result, error) {
 	defer a.flushUpdateStatus("app reconciled")
 
 	var err error
 
-	a.appMetrics.InitMetrics(a.Name(), a.Namespace())
+	a.countMetrics.InitMetrics(appResourceType, a.Name(), a.Namespace())
 
 	timerOpts := ReconcileTimerOpts{
 		DefaultSyncPeriod: a.opts.DefaultSyncPeriod,
@@ -104,13 +106,9 @@ func (a *App) reconcileDeploy() error {
 
 func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult {
 	reconcileStartTS := time.Now()
-	a.isFirstReconcile = "false"
-	if a.appMetrics.GetReconcileAttemptCounterValue(a.app.Name, a.app.Namespace) == 1 {
-		a.isFirstReconcile = "true"
-	}
-
+	a.isFirstReconcile = a.countMetrics.GetReconcileAttemptCounterValue("app", a.app.Name, a.app.Namespace) == 1
 	defer func() {
-		a.timeMetrics.RegisterOverallTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile,
+		a.timeMetrics.RegisterOverallTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile,
 			time.Since(reconcileStartTS))
 	}()
 
@@ -140,7 +138,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult {
 			UpdatedAt: metav1.NewTime(time.Now().UTC()),
 		}
 
-		a.timeMetrics.RegisterFetchTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile,
+		a.timeMetrics.RegisterFetchTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile,
 			a.app.Status.Fetch.UpdatedAt.Sub(a.app.Status.Fetch.StartedAt.Time))
 
 		err := a.updateStatus("marking fetch completed")
@@ -164,7 +162,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult {
 		UpdatedAt: metav1.NewTime(time.Now().UTC()),
 	}
 
-	a.timeMetrics.RegisterTemplateTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile,
+	a.timeMetrics.RegisterTemplateTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile,
 		a.app.Status.Template.UpdatedAt.Sub(templateStartTime))
 
 	err = a.updateStatus("marking template completed")
@@ -215,7 +213,7 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult {
 		},
 	}
 
-	a.timeMetrics.RegisterDeployTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile,
+	a.timeMetrics.RegisterDeployTime(appResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile,
 		a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time))
 
 	return result
@@ -269,7 +267,7 @@ func (a *App) setReconciling() {
 		Status: corev1.ConditionTrue,
 	})
 
-	a.appMetrics.RegisterReconcileAttempt(a.app.Name, a.app.Namespace)
+	a.countMetrics.RegisterReconcileAttempt(appResourceType, a.app.Name, a.app.Namespace)
 	a.app.Status.FriendlyDescription = "Reconciling"
 }
 
@@ -285,7 +283,7 @@ func (a *App) setReconcileCompleted(result exec.CmdRunResult) {
 		a.app.Status.ConsecutiveReconcileFailures++
 		a.app.Status.ConsecutiveReconcileSuccesses = 0
 		a.app.Status.FriendlyDescription = fmt.Sprintf("Reconcile failed: %s", result.ErrorStr())
-		a.appMetrics.RegisterReconcileFailure(a.app.Name, a.app.Namespace)
+		a.countMetrics.RegisterReconcileFailure(appResourceType, a.app.Name, a.app.Namespace)
 		a.setUsefulErrorMessage(result)
 	} else {
 		a.app.Status.Conditions = append(a.app.Status.Conditions, v1alpha1.Condition{
@@ -296,7 +294,7 @@ func (a *App) setReconcileCompleted(result exec.CmdRunResult) {
 		a.app.Status.ConsecutiveReconcileSuccesses++
 		a.app.Status.ConsecutiveReconcileFailures = 0
 		a.app.Status.FriendlyDescription = "Reconcile succeeded"
-		a.appMetrics.RegisterReconcileSuccess(a.app.Name, a.app.Namespace)
+		a.countMetrics.RegisterReconcileSuccess(appResourceType, a.app.Name, a.app.Namespace)
 		a.app.Status.UsefulErrorMessage = ""
 	}
 }
@@ -309,7 +307,7 @@ func (a *App) setDeleting() {
 		Status: corev1.ConditionTrue,
 	})
 
-	a.appMetrics.RegisterReconcileDeleteAttempt(a.app.Name, a.app.Namespace)
+	a.countMetrics.RegisterReconcileDeleteAttempt(appResourceType, a.app.Name, a.app.Namespace)
 	a.app.Status.FriendlyDescription = "Deleting"
 }
 
@@ -325,10 +323,10 @@ func (a *App) setDeleteCompleted(result exec.CmdRunResult) {
 		a.app.Status.ConsecutiveReconcileFailures++
 		a.app.Status.ConsecutiveReconcileSuccesses = 0
 		a.app.Status.FriendlyDescription = fmt.Sprintf("Delete failed: %s", result.ErrorStr())
-		a.appMetrics.RegisterReconcileDeleteFailed(a.app.Name, a.app.Namespace)
+		a.countMetrics.RegisterReconcileDeleteFailed(appResourceType, a.app.Name, a.app.Namespace)
 		a.setUsefulErrorMessage(result)
 	} else {
-		a.appMetrics.DeleteMetrics(a.app.Name, a.app.Namespace)
+		a.countMetrics.DeleteMetrics(appResourceType, a.app.Name, a.app.Namespace)
 	}
 }
 

pkg/app/app_reconcile_test.go

@@ -30,7 +30,7 @@ import (
 func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) {
 	log := logf.Log.WithName("kc")
 	var (
-		appMetrics  = metrics.NewAppMetrics()
+		appMetrics  = metrics.NewCountMetrics()
 		timeMetrics = metrics.NewReconcileTimeMetrics()
 	)
 
@@ -90,7 +90,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) {
 func Test_NoInspectReconcile_IfInspectNotEnabled(t *testing.T) {
 	log := logf.Log.WithName("kc")
 	var (
-		appMetrics  = metrics.NewAppMetrics()
+		appMetrics  = metrics.NewCountMetrics()
 		timeMetrics = metrics.NewReconcileTimeMetrics()
 	)
 
@@ -171,7 +171,7 @@ func Test_NoInspectReconcile_IfInspectNotEnabled(t *testing.T) {
 func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.T) {
 	log := logf.Log.WithName("kc")
 	var (
-		appMetrics  = metrics.NewAppMetrics()
+		appMetrics  = metrics.NewCountMetrics()
 		timeMetrics = metrics.NewReconcileTimeMetrics()
 	)
 

pkg/app/app_template_test.go

@@ -61,7 +61,7 @@ func Test_BuildAdditionalDownwardAPIValues_MemoizedCallCount(t *testing.T) {
 		K8sAPIsCount:    &k8sAPIsCallCount,
 		KCVersionCount:  &kcVersionCallCount,
 	}
-	app := NewApp(appEmpty, Hooks{}, fetchFac, tmpFac, deployFac, log, Opts{}, metrics.NewAppMetrics(), metrics.NewReconcileTimeMetrics(), fakeInfo)
+	app := NewApp(appEmpty, Hooks{}, fetchFac, tmpFac, deployFac, log, Opts{}, metrics.NewCountMetrics(), metrics.NewReconcileTimeMetrics(), fakeInfo)
 
 	dir, err := os.MkdirTemp("", "temp")
 	assert.NoError(t, err)

pkg/app/crd_app.go

@@ -21,20 +21,20 @@ import (
 )
 
 type CRDApp struct {
-	app        *App
-	appModel   *kcv1alpha1.App
-	log        logr.Logger
-	appMetrics *metrics.AppMetrics
-	appClient  kcclient.Interface
+	app          *App
+	appModel     *kcv1alpha1.App
+	log          logr.Logger
+	countMetrics *metrics.ReconcileCountMetrics
+	appClient    kcclient.Interface
 }
 
 // NewCRDApp creates new CRD app
-func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger, appMetrics *metrics.AppMetrics,
+func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger, appMetrics *metrics.ReconcileCountMetrics,
 	timeMetrics *metrics.ReconcileTimeMetrics, appClient kcclient.Interface, fetchFactory fetch.Factory,
 	templateFactory template.Factory, deployFactory deploy.Factory,
 	compInfo ComponentInfo, opts Opts) *CRDApp {
 
-	crdApp := &CRDApp{appModel: appModel, log: log, appMetrics: appMetrics, appClient: appClient}
+	crdApp := &CRDApp{appModel: appModel, log: log, countMetrics: appMetrics, appClient: appClient}
 
 	crdApp.app = NewApp(*appModel, Hooks{
 		BlockDeletion:   crdApp.blockDeletion,