Juju 7288/add prom metric for jimm methods (#1491)

* add prometheus metric for jimm * fly-by rename of jimm methods from previous pr
canonical · Dec 12, 2024 · c91f849 · c91f849
1 parent 2219df3
commit c91f849
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 20 deletions.
diff --git a/internal/jimm/access.go b/internal/jimm/access.go
@@ -23,6 +23,7 @@ import (
 	"github.com/canonical/jimm/v3/internal/jimmjwx"
 	"github.com/canonical/jimm/v3/internal/openfga"
 	ofganames "github.com/canonical/jimm/v3/internal/openfga/names"
+	"github.com/canonical/jimm/v3/internal/servermon"
 	jimmnames "github.com/canonical/jimm/v3/pkg/names"
 )
 
@@ -723,10 +724,13 @@ func (j *JIMM) parseAndValidateTag(ctx context.Context, key string) (*ofganames.
 //
 // This approach to cleaning up tuples is intended to be temporary while we implement
 // a better approach to eventual consistency of JIMM's database objects and OpenFGA tuples.
-func (j *JIMM) OpenFGACleanup(ctx context.Context) error {
+func (j *JIMM) OpenFGACleanup(ctx context.Context) (err error) {
+	const op = errors.Op("jimm.CleanupDyingModels")
+	zapctx.Info(ctx, string(op))
+	durationObserver := servermon.DurationObserver(servermon.JimmMethodsDurationHistogram, string(op))
+	defer durationObserver()
 	var (
 		continuationToken string
-		err               error
 		tuples            []ofga.Tuple
 	)
 	for {

diff --git a/internal/jimm/model.go b/internal/jimm/model.go
@@ -759,10 +759,10 @@ func (m *modelSummariesMap) addModelSummary(summary jujuparams.ModelSummaryResul
 	m.modelSummaries[summary.Result.UUID] = summary
 }
 
-// ModelSummaries returns the list of modelsummary the user has access to.
+// ListModelSummaries returns the list of modelsummary the user has access to.
 // It queries the controllers and then merge the info from the JIMM db.
-func (j *JIMM) ModelSummaries(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) {
-	const op = errors.Op("jimm.ModelSummaries")
+func (j *JIMM) ListModelSummaries(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) {
+	const op = errors.Op("jimm.ListModelSummaries")
 
 	modelSummariesSafeMap := modelSummariesMap{}
 	modelSummaryResults := []jujuparams.ModelSummaryResult{}

diff --git a/internal/jimm/model_cleanup.go b/internal/jimm/model_cleanup.go
@@ -12,15 +12,18 @@ import (
 
 	"github.com/canonical/jimm/v3/internal/dbmodel"
 	"github.com/canonical/jimm/v3/internal/errors"
+	"github.com/canonical/jimm/v3/internal/servermon"
 )
 
 // CleanupDyingModels loops over dying models, contacting the respective controller.
 // And deleting the model from our database if the error is `NotFound` which means the model was successfully deleted.
-func (j *JIMM) CleanupDyingModels(ctx context.Context) error {
+func (j *JIMM) CleanupDyingModels(ctx context.Context) (err error) {
 	const op = errors.Op("jimm.CleanupDyingModels")
 	zapctx.Info(ctx, string(op))
+	durationObserver := servermon.DurationObserver(servermon.JimmMethodsDurationHistogram, string(op))
+	defer durationObserver()
 
-	err := j.DB().ForEachModel(ctx, func(m *dbmodel.Model) error {
+	err = j.DB().ForEachModel(ctx, func(m *dbmodel.Model) error {
 		if m.Life != state.Dying.String() {
 			return nil
 		}

diff --git a/internal/jimm/model_test.go b/internal/jimm/model_test.go
@@ -1972,7 +1972,7 @@ func TestModelSummaries(t *testing.T) {
 					},
 				},
 			}
-			summaries, err := j.ModelSummaries(ctx, alice, "")
+			summaries, err := j.ListModelSummaries(ctx, alice, "")
 			c.Check(err, qt.IsNil)
 			c.Check(summaries.Results, qt.HasLen, t.expectedSummariesSize)
 			c.Check(summaries.Results, qt.DeepEquals, t.expectedSummaries)

diff --git a/internal/jujuapi/modelmanager.go b/internal/jujuapi/modelmanager.go
@@ -71,7 +71,7 @@ type ModelManager interface {
 	ImportModel(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error
 	ModelDefaultsForCloud(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag) (jujuparams.ModelDefaultsResult, error)
 	ModelInfo(ctx context.Context, u *openfga.User, mt names.ModelTag) (*jujuparams.ModelInfo, error)
-	ModelSummaries(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error)
+	ListModelSummaries(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error)
 	ModelStatus(ctx context.Context, u *openfga.User, mt names.ModelTag) (*jujuparams.ModelStatus, error)
 	QueryModelsJq(ctx context.Context, models []string, jqQuery string) (params.CrossModelQueryResponse, error)
 	SetModelDefaults(ctx context.Context, user *dbmodel.Identity, cloudTag names.CloudTag, region string, configs map[string]interface{}) error
@@ -113,7 +113,7 @@ func (r *controllerRoot) ListModelSummaries(ctx context.Context, _ jujuparams.Mo
 	if r.controllerUUIDMasking {
 		maskingControllerUUID = r.params.ControllerUUID
 	}
-	res, err := r.jimm.ModelSummaries(ctx, r.user, maskingControllerUUID)
+	res, err := r.jimm.ListModelSummaries(ctx, r.user, maskingControllerUUID)
 	if err != nil {
 		return jujuparams.ModelSummaryResults{}, errors.E(op, err)
 	}

diff --git a/internal/servermon/monitoring.go b/internal/servermon/monitoring.go
@@ -31,6 +31,13 @@ var (
 		Help:      "Histogram of database query time in seconds",
 		Buckets:   []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10},
 	}, []string{"method"})
+	JimmMethodsDurationHistogram = promauto.NewHistogramVec(prometheus.HistogramOpts{
+		Namespace: "jimm",
+		Subsystem: "jimm",
+		Name:      "op_duration_seconds",
+		Help:      "Histogram of jimm operations time in seconds",
+		Buckets:   []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10},
+	}, []string{"method"})
 	DBQueryErrorCount = promauto.NewCounterVec(prometheus.CounterOpts{
 		Namespace: "jimm",
 		Subsystem: "db",
@@ -101,12 +108,6 @@ var (
 		Name:      "models_created_fail_total",
 		Help:      "The number of fails attempting to create models.",
 	})
-	MonitorDeltasReceivedCount = promauto.NewCounterVec(prometheus.CounterOpts{
-		Namespace: "jimm",
-		Subsystem: "monitor",
-		Name:      "deltas_received_total",
-		Help:      "The number of watcher deltas received.",
-	}, []string{"controller"})
 	MonitorErrorsCount = promauto.NewCounterVec(prometheus.CounterOpts{
 		Namespace: "jimm",
 		Subsystem: "monitor",

diff --git a/internal/testutils/jimmtest/mocks/model.go b/internal/testutils/jimmtest/mocks/model.go
@@ -25,7 +25,7 @@ type ModelManager struct {
 	ForEachModel_           func(ctx context.Context, u *openfga.User, f func(*dbmodel.Model, jujuparams.UserAccessPermission) error) error
 	ForEachUserModel_       func(ctx context.Context, u *openfga.User, f func(*dbmodel.Model, jujuparams.UserAccessPermission) error) error
 	FullModelStatus_        func(ctx context.Context, user *openfga.User, modelTag names.ModelTag, patterns []string) (*jujuparams.FullStatus, error)
-	ModelSummaries_         func(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error)
+	ListModelSummaries_     func(ctx context.Context, user *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error)
 	GetModel_               func(ctx context.Context, uuid string) (dbmodel.Model, error)
 	ImportModel_            func(ctx context.Context, user *openfga.User, controllerName string, modelTag names.ModelTag, newOwner string) error
 	IdentityModelDefaults_  func(ctx context.Context, user *dbmodel.Identity) (map[string]interface{}, error)
@@ -129,11 +129,11 @@ func (j *ModelManager) ModelStatus(ctx context.Context, u *openfga.User, mt name
 	return j.ModelStatus_(ctx, u, mt)
 }
 
-func (j *ModelManager) ModelSummaries(ctx context.Context, u *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) {
-	if j.ModelSummaries_ == nil {
+func (j *ModelManager) ListModelSummaries(ctx context.Context, u *openfga.User, maskingControllerUUID string) (jujuparams.ModelSummaryResults, error) {
+	if j.ListModelSummaries_ == nil {
 		return jujuparams.ModelSummaryResults{}, errors.E(errors.CodeNotImplemented)
 	}
-	return j.ModelSummaries_(ctx, u, maskingControllerUUID)
+	return j.ListModelSummaries_(ctx, u, maskingControllerUUID)
 }
 
 func (j *ModelManager) QueryModelsJq(ctx context.Context, models []string, jqQuery string) (params.CrossModelQueryResponse, error) {