From c91f849d94df2a763238caaa9df9a9273fbe1606 Mon Sep 17 00:00:00 2001 From: SimoneDutto Date: Thu, 12 Dec 2024 10:40:49 +0100 Subject: [PATCH] Juju 7288/add prom metric for jimm methods (#1491) * add prometheus metric for jimm * fly-by rename of jimm methods from previous pr --- internal/jimm/access.go | 8 ++++++-- internal/jimm/model.go | 6 +++--- internal/jimm/model_cleanup.go | 7 +++++-- internal/jimm/model_test.go | 2 +- internal/jujuapi/modelmanager.go | 4 ++-- internal/servermon/monitoring.go | 13 +++++++------ internal/testutils/jimmtest/mocks/model.go | 8 ++++---- 7 files changed, 28 insertions(+), 20 deletions(-) diff --git a/internal/jimm/access.go b/internal/jimm/access.go index 21a91c407..98f809786 100644 --- 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 index d4b94aafc..a0bcf653b 100644 --- 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 index f4c040b07..8b5aafa87 100644 --- 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 index cd33831bd..f1f6ba6c0 100644 --- 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 index 65b0ea68e..910c4c1d3 100644 --- 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 index da6e663b4..f95b201a5 100644 --- 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 index 397d6c917..3b2d9c960 100644 --- 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) {