diff --git a/cmd/controller/run.go b/cmd/controller/run.go index 433f7ea8b1..f8f3350e63 100644 --- a/cmd/controller/run.go +++ b/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, diff --git a/pkg/app/app.go b/pkg/app/app.go index 8e8fb586a7..a13da4d63d 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -55,12 +55,12 @@ 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 @@ -68,11 +68,11 @@ type App struct { 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 } diff --git a/pkg/app/app_factory.go b/pkg/app/app_factory.go index a37ec40b80..504384a5bb 100644 --- a/pkg/app/app_factory.go +++ b/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(), }) diff --git a/pkg/app/app_fetch.go b/pkg/app/app_fetch.go index 67da673d35..07a12f0e12 100644 --- a/pkg/app/app_fetch.go +++ b/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")) diff --git a/pkg/app/app_reconcile.go b/pkg/app/app_reconcile.go index 030690ba83..5b00d45d61 100644 --- a/pkg/app/app_reconcile.go +++ b/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) } } diff --git a/pkg/app/app_reconcile_test.go b/pkg/app/app_reconcile_test.go index 25380b15b9..a9becd7c7c 100644 --- a/pkg/app/app_reconcile_test.go +++ b/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() ) diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index 465649547a..09bd161876 100644 --- a/pkg/app/app_template_test.go +++ b/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) diff --git a/pkg/app/crd_app.go b/pkg/app/crd_app.go index b5658690fc..2f33b8cf34 100644 --- a/pkg/app/crd_app.go +++ b/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, diff --git a/pkg/metrics/app_metrics.go b/pkg/metrics/app_metrics.go deleted file mode 100644 index 771e8b7efa..0000000000 --- a/pkg/metrics/app_metrics.go +++ /dev/null @@ -1,142 +0,0 @@ -// Copyright 2021 VMware, Inc. -// SPDX-License-Identifier: Apache-2.0 - -package metrics - -import ( - "sync" - - "github.com/prometheus/client_golang/prometheus" - dto "github.com/prometheus/client_model/go" - "sigs.k8s.io/controller-runtime/pkg/metrics" -) - -// AppMetrics holds server metrics -type AppMetrics struct { - reconcileAttemptTotal *prometheus.CounterVec - reconcileSuccessTotal *prometheus.CounterVec - reconcileFailureTotal *prometheus.CounterVec - reconcileDeleteAttemptTotal *prometheus.CounterVec - reconcileDeleteFailedTotal *prometheus.CounterVec -} - -var ( - once sync.Once -) - -// NewAppMetrics creates AppMetrics object -func NewAppMetrics() *AppMetrics { - const ( - metricNamespace = "kappctrl" - kappNameLabel = "app_name" - kappNamespaceLabel = "namespace" - ) - return &AppMetrics{ - reconcileAttemptTotal: prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: metricNamespace, - Name: "app_reconcile_attempt_total", - Help: "Total number of attempted reconciles", - }, - []string{kappNameLabel, kappNamespaceLabel}, - ), - reconcileSuccessTotal: prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: metricNamespace, - Name: "app_reconcile_success_total", - Help: "Total number of succeeded reconciles", - }, - []string{kappNameLabel, kappNamespaceLabel}, - ), - reconcileFailureTotal: prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: metricNamespace, - Name: "app_reconcile_failure_total", - Help: "Total number of failed reconciles", - }, - []string{kappNameLabel, kappNamespaceLabel}, - ), - reconcileDeleteAttemptTotal: prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: metricNamespace, - Name: "app_reconcile_delete_attempt_total", - Help: "Total number of attempted reconcile deletion", - }, - []string{kappNameLabel, kappNamespaceLabel}, - ), - reconcileDeleteFailedTotal: prometheus.NewCounterVec( - prometheus.CounterOpts{ - Namespace: metricNamespace, - Name: "app_reconcile_delete_failed_total", - Help: "Total number of failed reconcile deletion", - }, - []string{kappNameLabel, kappNamespaceLabel}, - ), - } -} - -// RegisterAllMetrics registers all prometheus metrics. -func (am *AppMetrics) RegisterAllMetrics() { - once.Do(func() { - metrics.Registry.MustRegister( - am.reconcileAttemptTotal, - am.reconcileSuccessTotal, - am.reconcileFailureTotal, - am.reconcileDeleteAttemptTotal, - am.reconcileDeleteFailedTotal, - ) - }) -} - -// InitMetrics initializes metrics -func (am *AppMetrics) InitMetrics(appName string, namespace string) { - // Initializes counter metrics - am.reconcileAttemptTotal.WithLabelValues(appName, namespace).Add(0) - am.reconcileSuccessTotal.WithLabelValues(appName, namespace).Add(0) - am.reconcileFailureTotal.WithLabelValues(appName, namespace).Add(0) - am.reconcileDeleteAttemptTotal.WithLabelValues(appName, namespace).Add(0) - am.reconcileDeleteFailedTotal.WithLabelValues(appName, namespace).Add(0) -} - -// DeleteMetrics deletes metrics -func (am *AppMetrics) DeleteMetrics(appName string, namespace string) { - // Delete counter metrics - am.reconcileAttemptTotal.DeleteLabelValues(appName, namespace) - am.reconcileSuccessTotal.DeleteLabelValues(appName, namespace) - am.reconcileFailureTotal.DeleteLabelValues(appName, namespace) - am.reconcileDeleteAttemptTotal.DeleteLabelValues(appName, namespace) - am.reconcileDeleteFailedTotal.DeleteLabelValues(appName, namespace) -} - -// RegisterReconcileAttempt increments reconcileAttemptTotal -func (am *AppMetrics) RegisterReconcileAttempt(appName string, namespace string) { - am.reconcileAttemptTotal.WithLabelValues(appName, namespace).Inc() -} - -// RegisterReconcileSuccess increments reconcileSuccessTotal -func (am *AppMetrics) RegisterReconcileSuccess(appName string, namespace string) { - am.reconcileSuccessTotal.WithLabelValues(appName, namespace).Inc() -} - -// RegisterReconcileFailure increments reconcileFailureTotal -func (am *AppMetrics) RegisterReconcileFailure(appName string, namespace string) { - am.reconcileFailureTotal.WithLabelValues(appName, namespace).Inc() -} - -// RegisterReconcileDeleteAttempt increments reconcileDeleteAttemptTotal -func (am *AppMetrics) RegisterReconcileDeleteAttempt(appName string, namespace string) { - am.reconcileDeleteAttemptTotal.WithLabelValues(appName, namespace).Inc() -} - -// RegisterReconcileDeleteFailed increments reconcileDeleteFailedTotal -func (am *AppMetrics) RegisterReconcileDeleteFailed(appName string, namespace string) { - am.reconcileDeleteFailedTotal.WithLabelValues(appName, namespace).Inc() -} - -func (am *AppMetrics) GetReconcileAttemptCounterValue(appName, namespace string) int64 { - var m = &dto.Metric{} - if err := am.reconcileAttemptTotal.WithLabelValues(appName, namespace).Write(m); err != nil { - return 0 - } - return int64(m.Counter.GetValue()) -} diff --git a/pkg/metrics/reconcile_count_metrics.go b/pkg/metrics/reconcile_count_metrics.go new file mode 100644 index 0000000000..c0e6c46350 --- /dev/null +++ b/pkg/metrics/reconcile_count_metrics.go @@ -0,0 +1,144 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package metrics + +import ( + "sync" + + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +// ReconcileCountMetrics holds server metrics +type ReconcileCountMetrics struct { + reconcileAttemptTotal *prometheus.CounterVec + reconcileSuccessTotal *prometheus.CounterVec + reconcileFailureTotal *prometheus.CounterVec + reconcileDeleteAttemptTotal *prometheus.CounterVec + reconcileDeleteFailedTotal *prometheus.CounterVec +} + +var ( + once sync.Once +) + +// NewCountMetrics creates ReconcileCountMetrics object +func NewCountMetrics() *ReconcileCountMetrics { + const ( + metricNamespace = "kappctrl" + kappNameLabel = "name" + kappNamespaceLabel = "namespace" + resourceTypeLabel = "controller" + ) + return &ReconcileCountMetrics{ + reconcileAttemptTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: "app_reconcile_attempt_total", + Help: "Total number of attempted reconciles", + }, + []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, + ), + reconcileSuccessTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: "app_reconcile_success_total", + Help: "Total number of succeeded reconciles", + }, + []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, + ), + reconcileFailureTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: "app_reconcile_failure_total", + Help: "Total number of failed reconciles", + }, + []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, + ), + reconcileDeleteAttemptTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: "app_reconcile_delete_attempt_total", + Help: "Total number of attempted reconcile deletion", + }, + []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, + ), + reconcileDeleteFailedTotal: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: metricNamespace, + Name: "app_reconcile_delete_failed_total", + Help: "Total number of failed reconcile deletion", + }, + []string{resourceTypeLabel, kappNameLabel, kappNamespaceLabel}, + ), + } +} + +// RegisterAllMetrics registers all prometheus metrics. +func (am *ReconcileCountMetrics) RegisterAllMetrics() { + once.Do(func() { + metrics.Registry.MustRegister( + am.reconcileAttemptTotal, + am.reconcileSuccessTotal, + am.reconcileFailureTotal, + am.reconcileDeleteAttemptTotal, + am.reconcileDeleteFailedTotal, + ) + }) +} + +// InitMetrics initializes metrics +func (am *ReconcileCountMetrics) InitMetrics(resourceType, name, namespace string) { + // Initializes counter metrics + am.reconcileAttemptTotal.WithLabelValues(resourceType, name, namespace).Add(0) + am.reconcileSuccessTotal.WithLabelValues(resourceType, name, namespace).Add(0) + am.reconcileFailureTotal.WithLabelValues(resourceType, name, namespace).Add(0) + am.reconcileDeleteAttemptTotal.WithLabelValues(resourceType, name, namespace).Add(0) + am.reconcileDeleteFailedTotal.WithLabelValues(resourceType, name, namespace).Add(0) +} + +// DeleteMetrics deletes metrics +func (am *ReconcileCountMetrics) DeleteMetrics(resourceType, name, namespace string) { + // Delete counter metrics + am.reconcileAttemptTotal.DeleteLabelValues(resourceType, name, namespace) + am.reconcileSuccessTotal.DeleteLabelValues(resourceType, name, namespace) + am.reconcileFailureTotal.DeleteLabelValues(resourceType, name, namespace) + am.reconcileDeleteAttemptTotal.DeleteLabelValues(resourceType, name, namespace) + am.reconcileDeleteFailedTotal.DeleteLabelValues(resourceType, name, namespace) +} + +// RegisterReconcileAttempt increments reconcileAttemptTotal +func (am *ReconcileCountMetrics) RegisterReconcileAttempt(resourceType, appName, namespace string) { + am.reconcileAttemptTotal.WithLabelValues(resourceType, appName, namespace).Inc() +} + +// RegisterReconcileSuccess increments reconcileSuccessTotal +func (am *ReconcileCountMetrics) RegisterReconcileSuccess(resourceType, appName, namespace string) { + am.reconcileSuccessTotal.WithLabelValues(resourceType, appName, namespace).Inc() +} + +// RegisterReconcileFailure increments reconcileFailureTotal +func (am *ReconcileCountMetrics) RegisterReconcileFailure(resourceType, appName, namespace string) { + am.reconcileFailureTotal.WithLabelValues(resourceType, appName, namespace).Inc() +} + +// RegisterReconcileDeleteAttempt increments reconcileDeleteAttemptTotal +func (am *ReconcileCountMetrics) RegisterReconcileDeleteAttempt(resourceType, appName, namespace string) { + am.reconcileDeleteAttemptTotal.WithLabelValues(resourceType, appName, namespace).Inc() +} + +// RegisterReconcileDeleteFailed increments reconcileDeleteFailedTotal +func (am *ReconcileCountMetrics) RegisterReconcileDeleteFailed(resourceType, appName, namespace string) { + am.reconcileDeleteFailedTotal.WithLabelValues(resourceType, appName, namespace).Inc() +} + +// GetReconcileAttemptCounterValue return reconcile count +func (am *ReconcileCountMetrics) GetReconcileAttemptCounterValue(resourceType, appName, namespace string) int64 { + var m = &dto.Metric{} + if err := am.reconcileAttemptTotal.WithLabelValues(resourceType, appName, namespace).Write(m); err != nil { + return 0 + } + return int64(m.Counter.GetValue()) +} diff --git a/pkg/metrics/reconcile_time_metrics.go b/pkg/metrics/reconcile_time_metrics.go index 5f57620300..eaae34230d 100644 --- a/pkg/metrics/reconcile_time_metrics.go +++ b/pkg/metrics/reconcile_time_metrics.go @@ -1,12 +1,18 @@ +// Copyright 2021 VMware, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Package metrics to define all prometheus metric methods package metrics import ( "github.com/prometheus/client_golang/prometheus" "sigs.k8s.io/controller-runtime/pkg/metrics" + "strconv" "sync" "time" ) +// ReconcileTimeMetrics holds reconcile time metrics type ReconcileTimeMetrics struct { reconcileTimeSeconds *prometheus.GaugeVec reconcileDeployTimeSeconds *prometheus.GaugeVec @@ -18,13 +24,14 @@ var ( timeMetricsOnce sync.Once ) +// NewReconcileTimeMetrics creates ReconcileTimeMetrics object func NewReconcileTimeMetrics() *ReconcileTimeMetrics { const ( - metricNamespace = "kappctrl_reconcile_time_seconds" + metricNamespace = "kappctrl" resourceTypeLabel = "controller" resourceNameLabel = "name" firstReconcileLabel = "firstReconcile" - namespace = "namespace" + namespaceLabel = "namespace" ) return &ReconcileTimeMetrics{ reconcileTimeSeconds: prometheus.NewGaugeVec( @@ -33,7 +40,7 @@ func NewReconcileTimeMetrics() *ReconcileTimeMetrics { Name: "reconcile_time_seconds", Help: "Overall time taken to reconcile a CR", }, - []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + []string{resourceTypeLabel, resourceNameLabel, namespaceLabel, firstReconcileLabel}, ), reconcileFetchTimeSeconds: prometheus.NewGaugeVec( prometheus.GaugeOpts{ @@ -41,7 +48,7 @@ func NewReconcileTimeMetrics() *ReconcileTimeMetrics { Name: "reconcile_fetch_time_seconds", Help: "Time taken to perform a fetch for a CR", }, - []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + []string{resourceTypeLabel, resourceNameLabel, namespaceLabel, firstReconcileLabel}, ), reconcileTemplateTimeSeconds: prometheus.NewGaugeVec( prometheus.GaugeOpts{ @@ -49,7 +56,7 @@ func NewReconcileTimeMetrics() *ReconcileTimeMetrics { Name: "reconcile_template_time_seconds", Help: "Time taken to perform a templating for a CR", }, - []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + []string{resourceTypeLabel, resourceNameLabel, namespaceLabel, firstReconcileLabel}, ), reconcileDeployTimeSeconds: prometheus.NewGaugeVec( prometheus.GaugeOpts{ @@ -57,11 +64,12 @@ func NewReconcileTimeMetrics() *ReconcileTimeMetrics { Name: "reconcile_deploy_time_seconds", Help: "Time taken to perform a deploy for a CR", }, - []string{resourceTypeLabel, resourceNameLabel, namespace, firstReconcileLabel}, + []string{resourceTypeLabel, resourceNameLabel, namespaceLabel, firstReconcileLabel}, ), } } +// RegisterAllMetrics registers reconcile time prometheus metrics. func (tm *ReconcileTimeMetrics) RegisterAllMetrics() { timeMetricsOnce.Do(func() { metrics.Registry.MustRegister( @@ -73,18 +81,22 @@ func (tm *ReconcileTimeMetrics) RegisterAllMetrics() { }) } -func (tm *ReconcileTimeMetrics) RegisterOverallTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { - tm.reconcileTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +// RegisterOverallTime sets overall time +func (tm *ReconcileTimeMetrics) RegisterOverallTime(resourceType, name, namespace string, firstReconcile bool, time time.Duration) { + tm.reconcileTimeSeconds.WithLabelValues(resourceType, name, namespace, strconv.FormatBool(firstReconcile)).Set(time.Seconds()) } -func (tm *ReconcileTimeMetrics) RegisterFetchTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { - tm.reconcileFetchTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +// RegisterFetchTime sets fetch time +func (tm *ReconcileTimeMetrics) RegisterFetchTime(resourceType, name, namespace string, firstReconcile bool, time time.Duration) { + tm.reconcileFetchTimeSeconds.WithLabelValues(resourceType, name, namespace, strconv.FormatBool(firstReconcile)).Set(time.Seconds()) } -func (tm *ReconcileTimeMetrics) RegisterTemplateTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { - tm.reconcileTemplateTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +// RegisterTemplateTime sets template time +func (tm *ReconcileTimeMetrics) RegisterTemplateTime(resourceType, name, namespace string, firstReconcile bool, time time.Duration) { + tm.reconcileTemplateTimeSeconds.WithLabelValues(resourceType, name, namespace, strconv.FormatBool(firstReconcile)).Set(time.Seconds()) } -func (tm *ReconcileTimeMetrics) RegisterDeployTime(resourceType, name, namespace, firstReconcile string, time time.Duration) { - tm.reconcileDeployTimeSeconds.WithLabelValues(resourceType, name, namespace, firstReconcile).Set(time.Seconds()) +// RegisterDeployTime sets deploy time +func (tm *ReconcileTimeMetrics) RegisterDeployTime(resourceType, name, namespace string, firstReconcile bool, time time.Duration) { + tm.reconcileDeployTimeSeconds.WithLabelValues(resourceType, name, namespace, strconv.FormatBool(firstReconcile)).Set(time.Seconds()) } diff --git a/pkg/packageinstall/packageinstall.go b/pkg/packageinstall/packageinstall.go index 5f0d894436..ee3331cd62 100644 --- a/pkg/packageinstall/packageinstall.go +++ b/pkg/packageinstall/packageinstall.go @@ -42,6 +42,7 @@ const ( // PackageInstall to indicate that lower version of the package // can be selected vs whats currently installed. DowngradableAnnKey = "packaging.carvel.dev/downgradable" + packageInstallType = "pkgi" ) // nolint: revive @@ -56,7 +57,10 @@ type PackageInstallCR struct { compInfo ComponentInfo opts Opts - timeMetrics *metrics.ReconcileTimeMetrics + countMetrics *metrics.ReconcileCountMetrics + timeMetrics *metrics.ReconcileTimeMetrics + + firstReconcile bool } // nolint: revive @@ -66,10 +70,11 @@ type Opts struct { func NewPackageInstallCR(model *pkgingv1alpha1.PackageInstall, log logr.Logger, kcclient kcclient.Interface, pkgclient pkgclient.Interface, coreClient kubernetes.Interface, - compInfo ComponentInfo, opts Opts, timeMetrics *metrics.ReconcileTimeMetrics) *PackageInstallCR { + compInfo ComponentInfo, opts Opts, countMetrics *metrics.ReconcileCountMetrics, timeMetrics *metrics.ReconcileTimeMetrics) *PackageInstallCR { return &PackageInstallCR{model: model, unmodifiedModel: model.DeepCopy(), log: log, - kcclient: kcclient, pkgclient: pkgclient, coreClient: coreClient, compInfo: compInfo, opts: opts, timeMetrics: timeMetrics} + kcclient: kcclient, pkgclient: pkgclient, coreClient: coreClient, compInfo: compInfo, opts: opts, countMetrics: countMetrics, + timeMetrics: timeMetrics} } func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { @@ -78,6 +83,8 @@ func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { func(st kcv1alpha1.GenericStatus) { pi.model.Status.GenericStatus = st }, } + pi.countMetrics.InitMetrics(packageInstallType, pi.model.Name, pi.model.Namespace) + var result reconcile.Result var err error @@ -104,10 +111,13 @@ func (pi *PackageInstallCR) Reconcile() (reconcile.Result, error) { func (pi *PackageInstallCR) reconcile(modelStatus *reconciler.Status) (reconcile.Result, error) { pi.log.Info("Reconciling") + pi.countMetrics.RegisterReconcileAttempt(packageInstallType, pi.model.Name, pi.model.Namespace) reconcileStartTS := time.Now() + pi.firstReconcile = pi.countMetrics.GetReconcileAttemptCounterValue("pkgi", pi.model.Name, pi.model.Namespace) == 1 defer func() { - pi.timeMetrics.RegisterOverallTime("pkgi", pi.model.Name, pi.model.Namespace, "", time.Since(reconcileStartTS)) + pi.timeMetrics.RegisterOverallTime(packageInstallType, pi.model.Name, pi.model.Namespace, + pi.firstReconcile, time.Since(reconcileStartTS)) }() err := pi.blockDeletion() diff --git a/pkg/packageinstall/packageinstall_deletion_test.go b/pkg/packageinstall/packageinstall_deletion_test.go index 0cdfd9b0cd..1117e596da 100644 --- a/pkg/packageinstall/packageinstall_deletion_test.go +++ b/pkg/packageinstall/packageinstall_deletion_test.go @@ -66,7 +66,7 @@ func Test_PackageInstallDeletion(t *testing.T) { appClient := fakekappctrl.NewSimpleClientset(pkgInstall, existingApp) coreClient := fake.NewSimpleClientset() - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, coreClient, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, &metrics.ReconcileTimeMetrics{}) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, coreClient, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, metrics.NewCountMetrics(), metrics.NewReconcileTimeMetrics()) _, err := ip.Reconcile() assert.Nil(t, err) diff --git a/pkg/packageinstall/packageinstall_downgrade_test.go b/pkg/packageinstall/packageinstall_downgrade_test.go index 776f106e20..6c79161abb 100644 --- a/pkg/packageinstall/packageinstall_downgrade_test.go +++ b/pkg/packageinstall/packageinstall_downgrade_test.go @@ -27,6 +27,7 @@ import ( func Test_PackageInstallVersionDowngrades(t *testing.T) { log := logf.Log.WithName("kc") var reconcileTimeMetrics = metrics.NewReconcileTimeMetrics() + var reconcileCountMetrics = metrics.NewCountMetrics() pkg1 := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ @@ -100,7 +101,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -149,7 +150,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -198,7 +199,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -253,7 +254,7 @@ func Test_PackageInstallVersionDowngrades(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileTimeMetrics) + ip := NewPackageInstallCR(pkgInstall, log, appClient, pkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, reconcileCountMetrics, reconcileTimeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) diff --git a/pkg/packageinstall/packageinstall_test.go b/pkg/packageinstall/packageinstall_test.go index 0fc71cb757..08e95a2d2b 100644 --- a/pkg/packageinstall/packageinstall_test.go +++ b/pkg/packageinstall/packageinstall_test.go @@ -355,6 +355,7 @@ func Test_PackageRefUsesName(t *testing.T) { func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { var timeMetrics = metrics.NewReconcileTimeMetrics() + var countMetrics = metrics.NewCountMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -404,7 +405,7 @@ func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -431,6 +432,7 @@ func Test_PlaceHolderSecretCreated_WhenPackageHasNoSecretRef(t *testing.T) { func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing.T) { var timeMetrics = metrics.NewReconcileTimeMetrics() + var countMetrics = metrics.NewCountMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -484,7 +486,7 @@ func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -521,6 +523,7 @@ func Test_PlaceHolderSecretsCreated_WhenPackageHasMultipleFetchStages(t *testing func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) { var timeMetrics = metrics.NewReconcileTimeMetrics() + var countMetrics = metrics.NewCountMetrics() pkg := datapkgingv1alpha1.Package{ ObjectMeta: metav1.ObjectMeta{ Name: "expected-pkg", @@ -575,7 +578,7 @@ func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) GitVersion: "v0.20.0", } - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) _, err := ip.Reconcile() assert.Nil(t, err) @@ -596,6 +599,7 @@ func Test_PlaceHolderSecretsNotCreated_WhenFetchStagesHaveSecrets(t *testing.T) func Test_PlaceHolderSecretCreated_WhenPackageInstallUpdated(t *testing.T) { var timeMetrics = metrics.NewReconcileTimeMetrics() + var countMetrics = metrics.NewCountMetrics() appSpec := v1alpha1.AppSpec{ Fetch: []v1alpha1.AppFetch{ { @@ -653,7 +657,7 @@ func Test_PlaceHolderSecretCreated_WhenPackageInstallUpdated(t *testing.T) { fakekctrl := fakekappctrl.NewSimpleClientset(model, existingApp) fakek8s := fake.NewSimpleClientset() - ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, timeMetrics) + ip := NewPackageInstallCR(model, log, fakekctrl, fakePkgClient, fakek8s, FakeComponentInfo{KCVersion: semver.MustParse("0.42.31337")}, Opts{}, countMetrics, timeMetrics) // mock the kubernetes server version fakeDiscovery, _ := fakek8s.Discovery().(*fakediscovery.FakeDiscovery) diff --git a/pkg/packageinstall/reconciler.go b/pkg/packageinstall/reconciler.go index f1ba01e3e5..bb00af117f 100644 --- a/pkg/packageinstall/reconciler.go +++ b/pkg/packageinstall/reconciler.go @@ -34,12 +34,14 @@ type Reconciler struct { log logr.Logger kcConfig *kcconfig.Config timeMetrics *metrics.ReconcileTimeMetrics + countMetrics *metrics.ReconcileCountMetrics } // NewReconciler is the constructor for the Reconciler struct func NewReconciler(kcClient kcclient.Interface, pkgClient pkgclient.Interface, coreClient kubernetes.Interface, pkgToPkgInstallHandler *PackageInstallVersionHandler, - log logr.Logger, compInfo ComponentInfo, kcConfig *kcconfig.Config, timeMetrics *metrics.ReconcileTimeMetrics) *Reconciler { + log logr.Logger, compInfo ComponentInfo, kcConfig *kcconfig.Config, countMetrics *metrics.ReconcileCountMetrics, + timeMetrics *metrics.ReconcileTimeMetrics) *Reconciler { return &Reconciler{kcClient: kcClient, pkgClient: pkgClient, @@ -48,6 +50,7 @@ func NewReconciler(kcClient kcclient.Interface, pkgClient pkgclient.Interface, compInfo: compInfo, log: log, kcConfig: kcConfig, + countMetrics: countMetrics, timeMetrics: timeMetrics, } } @@ -90,5 +93,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( return reconcile.Result{}, err } - return NewPackageInstallCR(existingPkgInstall, log, r.kcClient, r.pkgClient, r.coreClient, r.compInfo, Opts{DefaultSyncPeriod: r.kcConfig.PackageInstallDefaultSyncPeriod()}, r.timeMetrics).Reconcile() + return NewPackageInstallCR(existingPkgInstall, log, r.kcClient, r.pkgClient, r.coreClient, r.compInfo, + Opts{DefaultSyncPeriod: r.kcConfig.PackageInstallDefaultSyncPeriod()}, r.countMetrics, r.timeMetrics).Reconcile() } diff --git a/pkg/pkgrepository/app.go b/pkg/pkgrepository/app.go index 3a03920be7..69b2545cfa 100644 --- a/pkg/pkgrepository/app.go +++ b/pkg/pkgrepository/app.go @@ -30,22 +30,22 @@ type App struct { templateFactory template.Factory deployFactory deploy.Factory - timeMetrics *metrics.ReconcileTimeMetrics - appMetrics *metrics.AppMetrics + timeMetrics *metrics.ReconcileTimeMetrics + countMetrics *metrics.ReconcileCountMetrics log logr.Logger - isFirstReconcile string + isFirstReconcile bool pendingStatusUpdate bool flushAllStatusUpdates bool } // NewApp creates a new instance of an App based on v1alpha1.App func NewApp(app v1alpha1.App, hooks Hooks, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory, - log logr.Logger, appMetrics *metrics.AppMetrics, timeMetrics *metrics.ReconcileTimeMetrics, pkgRepoUID types.UID) *App { + log logr.Logger, countMetrics *metrics.ReconcileCountMetrics, timeMetrics *metrics.ReconcileTimeMetrics, pkgRepoUID types.UID) *App { return &App{app: app, appPrev: *(app.DeepCopy()), hooks: hooks, fetchFactory: fetchFactory, templateFactory: templateFactory, - deployFactory: deployFactory, log: log, appMetrics: appMetrics, timeMetrics: timeMetrics, pkgRepoUID: pkgRepoUID} + deployFactory: deployFactory, log: log, countMetrics: countMetrics, timeMetrics: timeMetrics, pkgRepoUID: pkgRepoUID} } func (a *App) Name() string { return a.app.Name } diff --git a/pkg/pkgrepository/app_deploy.go b/pkg/pkgrepository/app_deploy.go index 4e2b46ebfb..7b8b236e4c 100644 --- a/pkg/pkgrepository/app_deploy.go +++ b/pkg/pkgrepository/app_deploy.go @@ -5,8 +5,6 @@ package pkgrepository import ( "fmt" - "time" - "github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1" ctldep "github.com/vmware-tanzu/carvel-kapp-controller/pkg/deploy" "github.com/vmware-tanzu/carvel-kapp-controller/pkg/exec" @@ -14,11 +12,6 @@ import ( ) func (a *App) deploy(tplOutput string) exec.CmdRunResult { - reconcileStartTS := time.Now() - defer func() { - a.timeMetrics.RegisterDeployTime(a.app.Kind, a.app.Name, a.app.Namespace, "", time.Since(reconcileStartTS)) - }() - err := a.blockDeletion() if err != nil { return exec.NewCmdRunResultWithErr(fmt.Errorf("Blocking for deploy: %s", err)) diff --git a/pkg/pkgrepository/app_factory.go b/pkg/pkgrepository/app_factory.go index 379d1c849e..f2f194b215 100644 --- a/pkg/pkgrepository/app_factory.go +++ b/pkg/pkgrepository/app_factory.go @@ -23,14 +23,14 @@ import ( // AppFactory allows to create "hidden" Apps for reconciling PackageRepositories. type AppFactory struct { - CoreClient kubernetes.Interface - AppClient kcclient.Interface - TimeMetrics *metrics.ReconcileTimeMetrics - AppMetrics *metrics.AppMetrics - KcConfig *config.Config - CmdRunner exec.CmdRunner - Kubeconf *kubeconfig.Kubeconfig - CacheFolder *memdir.TmpDir + CoreClient kubernetes.Interface + AppClient kcclient.Interface + TimeMetrics *metrics.ReconcileTimeMetrics + CountMetrics *metrics.ReconcileCountMetrics + KcConfig *config.Config + CmdRunner exec.CmdRunner + Kubeconf *kubeconfig.Kubeconfig + CacheFolder *memdir.TmpDir } // NewCRDPackageRepo constructs "hidden" App to reconcile PackageRepository. @@ -42,5 +42,5 @@ func (f *AppFactory) NewCRDPackageRepo(app *kcv1alpha1.App, pkgr *pkgv1alpha1.Pa fetchFactory := fetch.NewFactory(f.CoreClient, vendirOpts, f.CmdRunner) templateFactory := template.NewFactory(f.CoreClient, fetchFactory, false, f.CmdRunner) deployFactory := deploy.NewFactory(f.CoreClient, f.Kubeconf, nil, f.CmdRunner, log) - return NewCRDApp(app, pkgr, log, f.AppClient, f.TimeMetrics, f.AppMetrics, fetchFactory, templateFactory, deployFactory) + return NewCRDApp(app, pkgr, log, f.AppClient, f.TimeMetrics, f.CountMetrics, fetchFactory, templateFactory, deployFactory) } diff --git a/pkg/pkgrepository/app_reconcile.go b/pkg/pkgrepository/app_reconcile.go index 66461214e7..194c52d807 100644 --- a/pkg/pkgrepository/app_reconcile.go +++ b/pkg/pkgrepository/app_reconcile.go @@ -15,13 +15,15 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" ) +const packageRepoResourceType = "pkgr" + // Reconcile is not expected to be called concurrently func (a *App) Reconcile(force bool) (reconcile.Result, error) { defer a.flushUpdateStatus("packagerepository reconciled") var err error - a.appMetrics.InitMetrics(a.Name(), a.Namespace()) + a.countMetrics.InitMetrics("pkgr", a.Name(), a.Namespace()) switch { case a.app.Spec.Paused: @@ -93,12 +95,10 @@ 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(packageRepoResourceType, a.app.Name, a.app.Namespace) == 1 defer func() { - a.timeMetrics.RegisterOverallTime(a.app.Kind, a.app.Name, a.app.Namespace, a.isFirstReconcile, time.Since(reconcileStartTS)) + a.timeMetrics.RegisterOverallTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, + time.Since(reconcileStartTS)) }() tmpDir := memdir.NewTmpDir("fetch-template-deploy") @@ -127,7 +127,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterFetchTime("pkgr", a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.timeMetrics.RegisterFetchTime(packageRepoResourceType, 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") @@ -151,7 +151,7 @@ func (a *App) reconcileFetchTemplateDeploy() exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterTemplateTime("pkgr", a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.timeMetrics.RegisterTemplateTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.app.Status.Template.UpdatedAt.Sub(templateStartTime)) err = a.updateStatus("marking template completed") @@ -181,7 +181,7 @@ func (a *App) updateLastDeploy(result exec.CmdRunResult) exec.CmdRunResult { UpdatedAt: metav1.NewTime(time.Now().UTC()), } - a.timeMetrics.RegisterDeployTime("pkgr", a.app.Name, a.app.Namespace, a.isFirstReconcile, + a.timeMetrics.RegisterDeployTime(packageRepoResourceType, a.app.Name, a.app.Namespace, a.isFirstReconcile, a.Status().Deploy.UpdatedAt.Sub(a.Status().Deploy.StartedAt.Time)) a.updateStatus("marking last deploy") @@ -215,7 +215,7 @@ func (a *App) setReconciling() { Status: corev1.ConditionTrue, }) - a.appMetrics.RegisterReconcileAttempt(a.app.Name, a.app.Namespace) + a.countMetrics.RegisterReconcileAttempt("pkgr", a.app.Name, a.app.Namespace) a.app.Status.FriendlyDescription = "Reconciling" } diff --git a/pkg/pkgrepository/app_reconcile_test.go b/pkg/pkgrepository/app_reconcile_test.go index 78ebf77267..7ae8ac5c3a 100644 --- a/pkg/pkgrepository/app_reconcile_test.go +++ b/pkg/pkgrepository/app_reconcile_test.go @@ -26,7 +26,7 @@ import ( func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { log := logf.Log.WithName("kc") var timeMetrics = metrics.NewReconcileTimeMetrics() - var appMetrics = metrics.NewAppMetrics() + var appMetrics = metrics.NewCountMetrics() // The url under fetch is invalid, which will cause this // app to fail before deploy. @@ -82,7 +82,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) { func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.T) { log := logf.Log.WithName("kc") var timeMetrics = metrics.NewReconcileTimeMetrics() - var appMetrics = metrics.NewAppMetrics() + var appMetrics = metrics.NewCountMetrics() fetchInline := map[string]string{ "packages/file.yml": `foo: #@ data.values.nothere`, @@ -141,7 +141,7 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing. func TestInvalidPackageRepositoryFormat(t *testing.T) { log := logf.Log.WithName("kc") var timeMetrics = metrics.NewReconcileTimeMetrics() - var appMetrics = metrics.NewAppMetrics() + var appMetrics = metrics.NewCountMetrics() fetchInline := map[string]string{ "file.yml": `foo: hi`, diff --git a/pkg/pkgrepository/crd_app.go b/pkg/pkgrepository/crd_app.go index a99c99d1dc..154be2fe38 100644 --- a/pkg/pkgrepository/crd_app.go +++ b/pkg/pkgrepository/crd_app.go @@ -24,13 +24,12 @@ type CRDApp struct { app *App appModel *kcv1alpha1.App pkgrModel *pkgingv1alpha1.PackageRepository - timeMetrics *metrics.ReconcileTimeMetrics log logr.Logger appClient kcclient.Interface } func NewCRDApp(appModel *kcv1alpha1.App, packageRepo *pkgingv1alpha1.PackageRepository, log logr.Logger, - appClient kcclient.Interface, timeMetrics *metrics.ReconcileTimeMetrics, appMetrics *metrics.AppMetrics, fetchFactory fetch.Factory, + appClient kcclient.Interface, timeMetrics *metrics.ReconcileTimeMetrics, countMetrics *metrics.ReconcileCountMetrics, fetchFactory fetch.Factory, templateFactory template.Factory, deployFactory deploy.Factory) *CRDApp { crdApp := &CRDApp{appModel: appModel, pkgrModel: packageRepo, log: log, appClient: appClient} @@ -39,7 +38,7 @@ func NewCRDApp(appModel *kcv1alpha1.App, packageRepo *pkgingv1alpha1.PackageRepo BlockDeletion: crdApp.blockDeletion, UnblockDeletion: crdApp.unblockDeletion, UpdateStatus: crdApp.updateStatus, - }, fetchFactory, templateFactory, deployFactory, log, appMetrics, timeMetrics, packageRepo.UID) + }, fetchFactory, templateFactory, deployFactory, log, countMetrics, timeMetrics, packageRepo.UID) return crdApp }