Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Add metrics to monitor reconcile status #346

Merged
merged 26 commits into from Sep 9, 2021
Merged

Add metrics to monitor reconcile status #346

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
375d66b
Simple version of metrics add
mhoshi-vm Aug 23, 2021
4d3cc33
Simple version
mhoshi-vm Aug 23, 2021
746e6a4
Refactor
mhoshi-vm Aug 23, 2021
1b09d38
Refactor
mhoshi-vm Aug 23, 2021
0bccb6b
Add Go mod
mhoshi-vm Aug 23, 2021
7d7a508
Fix linting
mhoshi-vm Aug 24, 2021
6aece10
Fix linting
mhoshi-vm Aug 24, 2021
3b786c2
Fix unneeded export.
mhoshi-vm Aug 24, 2021
b8beadc
Change to object creation
mhoshi-vm Aug 25, 2021
4d33eb4
Stop casting
mhoshi-vm Aug 27, 2021
e9cf87c
Add namespace to dummy app object
mhoshi-vm Aug 27, 2021
ee74e0f
Update test code
mhoshi-vm Aug 27, 2021
fe6d1a7
Retest
mhoshi-vm Aug 27, 2021
fedb24c
Fix linting
mhoshi-vm Aug 27, 2021
719ba20
Only register metrics once
mhoshi-vm Aug 27, 2021
3e6a524
Fix linting
mhoshi-vm Aug 27, 2021
5cd11b3
* Fix app factory to construct metrics
mhoshi-vm Sep 2, 2021
6492ce2
Merge remote-tracking branch 'upstream/develop' into sbp-metrics-2
mhoshi-vm Sep 2, 2021
ba4c625
Update test code
mhoshi-vm Sep 2, 2021
aec223a
Update test code
mhoshi-vm Sep 2, 2021
351b1f0
Create noop metrics
mhoshi-vm Sep 7, 2021
36bf99d
Merge remote-tracking branch 'upstream/develop' into sbp-metrics-2
mhoshi-vm Sep 7, 2021
c9ff2a7
Fix export
mhoshi-vm Sep 7, 2021
5a1d404
Fix Go fmt
mhoshi-vm Sep 7, 2021
8dd8800
Update test code
mhoshi-vm Sep 7, 2021
ca093f8
Fix uneeded export and test code
mhoshi-vm Sep 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion cmd/controller/app_factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/config"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/deploy"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/fetch"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics"
ctlpkgr "github.com/vmware-tanzu/carvel-kapp-controller/pkg/pkgrepository"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/template"
"k8s.io/client-go/kubernetes"
Expand All @@ -21,13 +22,16 @@ type AppFactory struct {
coreClient kubernetes.Interface
appClient kcclient.Interface
kcConfig *config.Config
appMetrics *metrics.AppMetrics
}

// NewCRDApp creates new CRD app
func (f *AppFactory) NewCRDApp(app *kcv1alpha1.App, log logr.Logger) *ctlapp.CRDApp {
fetchFactory := fetch.NewFactory(f.coreClient, f.kcConfig)
templateFactory := template.NewFactory(f.coreClient, fetchFactory)
deployFactory := deploy.NewFactory(f.coreClient)
return ctlapp.NewCRDApp(app, log, f.appClient, fetchFactory, templateFactory, deployFactory)

return ctlapp.NewCRDApp(app, log, f.appMetrics, f.appClient, fetchFactory, templateFactory, deployFactory)
}

// TODO: Create a PackageRepo factory for this func
Expand Down
6 changes: 6 additions & 0 deletions cmd/controller/run.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/go-logr/logr"
"github.com/vmware-tanzu/carvel-kapp-controller/cmd/controller/handlers"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/apiserver"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/reftracker"
v1 "k8s.io/api/core/v1"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -92,6 +93,10 @@ func Run(opts Options, runLog logr.Logger) {
os.Exit(1)
}

runLog.Info("setting up metrics")
appMetrics := metrics.NewServerMetrics()
appMetrics.RegisterAllMetrics()

// assign bindPort to env var KAPPCTRL_API_PORT if available
cppforlife marked this conversation as resolved.
Show resolved Hide resolved
var bindPort int
if apiPort, ok := os.LookupEnv(kappctrlAPIPORTEnvKey); ok {
Expand Down Expand Up @@ -123,6 +128,7 @@ func Run(opts Options, runLog logr.Logger) {
coreClient: coreClient,
kcConfig: kcConfig,
appClient: kcClient,
appMetrics: appMetrics,
}

{ // add controller for apps
Expand Down
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ require (
github.com/jonboulle/clockwork v0.2.0 // indirect
github.com/onsi/ginkgo v1.16.1 // indirect
github.com/onsi/gomega v1.11.0 // indirect
github.com/prometheus/client_golang v1.7.1
github.com/sirupsen/logrus v1.8.1 // indirect
github.com/stretchr/testify v1.7.0
github.com/tmc/grpc-websocket-proxy v0.0.0-20200427203606-3cfed13b9966 // indirect
Expand Down
8 changes: 5 additions & 3 deletions pkg/app/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/apis/kappctrl/v1alpha1"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/deploy"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/fetch"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/reftracker"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/template"
)
Expand All @@ -31,19 +32,20 @@ type App struct {
templateFactory template.Factory
deployFactory deploy.Factory

log logr.Logger
log logr.Logger
appMetrics *metrics.AppMetrics

pendingStatusUpdate bool
flushAllStatusUpdates bool
}

func NewApp(app v1alpha1.App, hooks Hooks,
fetchFactory fetch.Factory, templateFactory template.Factory,
deployFactory deploy.Factory, log logr.Logger) *App {
deployFactory deploy.Factory, log logr.Logger, appMetrics *metrics.AppMetrics) *App {

return &App{app: app, appPrev: *(app.DeepCopy()), hooks: hooks,
fetchFactory: fetchFactory, templateFactory: templateFactory,
deployFactory: deployFactory, log: log}
deployFactory: deployFactory, log: log, appMetrics: appMetrics}
}

func (a *App) Name() string { return a.app.Name }
Expand Down
7 changes: 6 additions & 1 deletion pkg/app/app_reconcile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ func (a *App) setReconciling() {
Status: corev1.ConditionTrue,
})

a.appMetrics.RegisterReconcileAttempt(a.app.Name, a.app.Namespace)
a.app.Status.FriendlyDescription = "Reconciling"
}

Expand All @@ -226,6 +227,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.setUsefulErrorMessage(result)
} else {
a.app.Status.Conditions = append(a.app.Status.Conditions, v1alpha1.AppCondition{
Expand All @@ -236,6 +238,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.app.Status.UsefulErrorMessage = ""
}
}
Expand All @@ -248,6 +251,7 @@ func (a *App) setDeleting() {
Status: corev1.ConditionTrue,
})

a.appMetrics.RegisterReconcileDeleteAttempt(a.app.Name, a.app.Namespace)
a.app.Status.FriendlyDescription = "Deleting"
}

Expand All @@ -261,9 +265,10 @@ func (a *App) setDeleteCompleted(result exec.CmdRunResult) {
Message: result.ErrorStr(),
})
a.app.Status.FriendlyDescription = fmt.Sprintf("Delete failed: %s", result.ErrorStr())
a.appMetrics.RegisterReconcileDeleteFailed(a.app.Name, a.app.Namespace)
a.setUsefulErrorMessage(result)
} else {
// assume resource will be deleted, hence nothing to update
a.appMetrics.DeleteMetrics(a.app.Name, a.app.Namespace)
}
}

Expand Down
13 changes: 9 additions & 4 deletions pkg/app/app_reconcile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package app

import (
"fmt"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics"
"reflect"
"testing"

Expand All @@ -22,12 +23,14 @@ import (

func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) {
log := logf.Log.WithName("kc")
appMetrics := &metrics.AppMetrics{}

// The url under fetch is invalid, which will cause this
// app to fail before deploy.
app := v1alpha1.App{
ObjectMeta: metav1.ObjectMeta{
Name: "simple-app",
Name: "simple-app",
Namespace: "pkg-standalone",
},
Spec: v1alpha1.AppSpec{
Fetch: []v1alpha1.AppFetch{
Expand All @@ -42,7 +45,7 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) {
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

crdApp := NewCRDApp(&app, log, kappcs, fetchFac, tmpFac, deployFac)
crdApp := NewCRDApp(&app, log, appMetrics, kappcs, fetchFac, tmpFac, deployFac)
_, err := crdApp.Reconcile(false)
assert.Nil(t, err, "unexpected error with reconciling", err)

Expand Down Expand Up @@ -78,13 +81,15 @@ func Test_NoInspectReconcile_IfNoDeployAttempted(t *testing.T) {

func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.T) {
log := logf.Log.WithName("kc")
appMetrics := &metrics.AppMetrics{}

fetchInline := map[string]string{
"file.yml": `foo: #@ data.values.nothere`,
}
app := v1alpha1.App{
ObjectMeta: metav1.ObjectMeta{
Name: "simple-app",
Name: "simple-app",
Namespace: "pkg-standalone",
},
Spec: v1alpha1.AppSpec{
Fetch: []v1alpha1.AppFetch{
Expand All @@ -102,7 +107,7 @@ func Test_TemplateError_DisplayedInStatus_UsefulErrorMessageProperty(t *testing.
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

crdApp := NewCRDApp(&app, log, kappcs, fetchFac, tmpFac, deployFac)
crdApp := NewCRDApp(&app, log, appMetrics, kappcs, fetchFac, tmpFac, deployFac)
_, err := crdApp.Reconcile(false)
assert.Nil(t, err, "Unexpected error with reconciling", err)

Expand Down
8 changes: 4 additions & 4 deletions pkg/app/app_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func Test_SecretRefs_RetrievesAllSecretRefs(t *testing.T) {
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log)
app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, nil)

out := app.SecretRefs()
if !reflect.DeepEqual(out, expected) {
Expand All @@ -85,7 +85,7 @@ func Test_SecretRefs_RetrievesNoSecretRefs_WhenNonePresent(t *testing.T) {
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log)
app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, nil)

out := app.SecretRefs()
if len(out) != 0 {
Expand Down Expand Up @@ -125,7 +125,7 @@ func Test_ConfigMapRefs_RetrievesAllConfigMapRefs(t *testing.T) {
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log)
app := apppkg.NewApp(appWithRefs, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, nil)

out := app.ConfigMapRefs()
if !reflect.DeepEqual(out, expected) {
Expand All @@ -151,7 +151,7 @@ func Test_ConfigMapRefs_RetrievesNoConfigMapRefs_WhenNonePresent(t *testing.T) {
tmpFac := template.NewFactory(k8scs, fetchFac)
deployFac := deploy.NewFactory(k8scs)

app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log)
app := apppkg.NewApp(appEmpty, apppkg.Hooks{}, fetchFac, tmpFac, deployFac, log, nil)

out := app.ConfigMapRefs()
if len(out) != 0 {
Expand Down
18 changes: 11 additions & 7 deletions pkg/app/crd_app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package app
import (
"context"
"fmt"
"github.com/vmware-tanzu/carvel-kapp-controller/pkg/metrics"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: move into next import subsection

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

"reflect"

"github.com/go-logr/logr"
Expand All @@ -20,24 +21,26 @@ import (
)

type CRDApp struct {
app *App
appModel *kcv1alpha1.App
log logr.Logger
appClient kcclient.Interface
app *App
appModel *kcv1alpha1.App
log logr.Logger
appMetrics *metrics.AppMetrics
appClient kcclient.Interface
}

func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger,
// NewCRDApp creates new CRD app
func NewCRDApp(appModel *kcv1alpha1.App, log logr.Logger, appMetrics *metrics.AppMetrics,
appClient kcclient.Interface, fetchFactory fetch.Factory,
templateFactory template.Factory, deployFactory deploy.Factory) *CRDApp {

crdApp := &CRDApp{appModel: appModel, log: log, appClient: appClient}
crdApp := &CRDApp{appModel: appModel, log: log, appMetrics: appMetrics, appClient: appClient}

crdApp.app = NewApp(*appModel, Hooks{
BlockDeletion: crdApp.blockDeletion,
UnblockDeletion: crdApp.unblockDeletion,
UpdateStatus: crdApp.updateStatus,
WatchChanges: crdApp.watchChanges,
}, fetchFactory, templateFactory, deployFactory, log)
}, fetchFactory, templateFactory, deployFactory, log, appMetrics)

return crdApp
}
Expand Down Expand Up @@ -114,6 +117,7 @@ func (a *CRDApp) updateApp(updateFunc func(*kcv1alpha1.App)) error {
}

func (a *CRDApp) Reconcile(force bool) (reconcile.Result, error) {
a.appMetrics.InitMetrics(a.app.Name(), a.app.Namespace())
cppforlife marked this conversation as resolved.
Show resolved Hide resolved
return a.app.Reconcile(force)
}

Expand Down
Loading