From a59ec345c2d33cd463681f0f4560b2e7d7a2d9b5 Mon Sep 17 00:00:00 2001 From: Dana Axinte <53751979+dana-axinte@users.noreply.github.com> Date: Thu, 3 Jul 2025 17:32:18 +0100 Subject: [PATCH] SecretsManager: Introduce metrics and logs (#107582) Co-authored-by: Michael Mandrus --- .../secret/encryption/manager/manager_test.go | 6 +- .../secret/encryption/manager/test_helpers.go | 2 +- .../secret/secretkeeper/metrics/metrics.go | 71 ++++ .../apis/secret/secretkeeper/secretkeeper.go | 4 +- .../secret/secretkeeper/secretkeeper_test.go | 4 +- .../secret/secretkeeper/sqlkeeper/keeper.go | 23 +- .../secretkeeper/sqlkeeper/keeper_test.go | 4 +- pkg/setting/setting_secrets_manager.go | 7 - .../secret/encryption/data_key_store.go | 10 +- .../secret/encryption/data_key_store_test.go | 2 +- pkg/storage/secret/encryption/metrics.go | 98 +++++ pkg/storage/secret/metadata/keeper_store.go | 48 ++- .../secret/metadata/keeper_store_test.go | 4 +- .../secret/metadata/metrics/metrics.go | 356 ++++++++++++++++++ pkg/storage/secret/metadata/outbox_store.go | 24 ++ .../secret/metadata/outbox_store_test.go | 8 +- .../secret/metadata/secure_value_store.go | 41 +- .../metadata/secure_value_store_test.go | 4 +- pkg/storage/secret/migrator/migrator.go | 4 +- 19 files changed, 685 insertions(+), 35 deletions(-) create mode 100644 pkg/registry/apis/secret/secretkeeper/metrics/metrics.go create mode 100644 pkg/storage/secret/encryption/metrics.go create mode 100644 pkg/storage/secret/metadata/metrics/metrics.go diff --git a/pkg/registry/apis/secret/encryption/manager/manager_test.go b/pkg/registry/apis/secret/encryption/manager/manager_test.go index 15cbaa600c5..e6b5b05ca9b 100644 --- a/pkg/registry/apis/secret/encryption/manager/manager_test.go +++ b/pkg/registry/apis/secret/encryption/manager/manager_test.go @@ -77,7 +77,7 @@ func TestEncryptionService_DataKeys(t *testing.T) { testDB := sqlstore.NewTestStore(t, sqlstore.WithMigrator(migrator.New())) features := featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, featuremgmt.FlagSecretsManagementAppPlatform) tracer := noop.NewTracerProvider().Tracer("test") - store, err := encryptionstorage.ProvideDataKeyStorage(database.ProvideDatabase(testDB, tracer), tracer, features) + store, err := encryptionstorage.ProvideDataKeyStorage(database.ProvideDatabase(testDB, tracer), tracer, features, nil) require.NoError(t, err) ctx := context.Background() @@ -183,7 +183,7 @@ func TestEncryptionService_UseCurrentProvider(t *testing.T) { features := featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, featuremgmt.FlagSecretsManagementAppPlatform) testDB := sqlstore.NewTestStore(t, sqlstore.WithMigrator(migrator.New())) tracer := noop.NewTracerProvider().Tracer("test") - encryptionStore, err := encryptionstorage.ProvideDataKeyStorage(database.ProvideDatabase(testDB, tracer), tracer, features) + encryptionStore, err := encryptionstorage.ProvideDataKeyStorage(database.ProvideDatabase(testDB, tracer), tracer, features, nil) require.NoError(t, err) encMgr, err := ProvideEncryptionManager( @@ -374,7 +374,7 @@ func TestIntegration_SecretsService(t *testing.T) { EncryptionProvider: "secretKey.v1", }, } - store, err := encryptionstorage.ProvideDataKeyStorage(database.ProvideDatabase(testDB, tracer), tracer, features) + store, err := encryptionstorage.ProvideDataKeyStorage(database.ProvideDatabase(testDB, tracer), tracer, features, nil) require.NoError(t, err) usageStats := &usagestats.UsageStatsMock{T: t} diff --git a/pkg/registry/apis/secret/encryption/manager/test_helpers.go b/pkg/registry/apis/secret/encryption/manager/test_helpers.go index bfdd4cc7bea..a3958b5df7c 100644 --- a/pkg/registry/apis/secret/encryption/manager/test_helpers.go +++ b/pkg/registry/apis/secret/encryption/manager/test_helpers.go @@ -31,7 +31,7 @@ func setupTestService(tb testing.TB) *EncryptionManager { EncryptionProvider: "secretKey.v1", }, } - store, err := encryptionstorage.ProvideDataKeyStorage(database, tracer, features) + store, err := encryptionstorage.ProvideDataKeyStorage(database, tracer, features, nil) require.NoError(tb, err) usageStats := &usagestats.UsageStatsMock{T: tb} diff --git a/pkg/registry/apis/secret/secretkeeper/metrics/metrics.go b/pkg/registry/apis/secret/secretkeeper/metrics/metrics.go new file mode 100644 index 00000000000..0dc3727c721 --- /dev/null +++ b/pkg/registry/apis/secret/secretkeeper/metrics/metrics.go @@ -0,0 +1,71 @@ +package metrics + +import ( + "github.com/prometheus/client_golang/prometheus" +) + +const ( + namespace = "grafana_secrets_manager" + subsystem = "keeper" +) + +// KeeperMetrics is a struct that contains all the metrics for an implementation of all keepers. +type KeeperMetrics struct { + StoreDuration *prometheus.HistogramVec + UpdateDuration *prometheus.HistogramVec + ExposeDuration *prometheus.HistogramVec + DeleteDuration *prometheus.HistogramVec +} + +func newKeeperMetrics() *KeeperMetrics { + return &KeeperMetrics{ + StoreDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "store_duration_seconds", + Help: "Duration of keeper store operations", + Buckets: prometheus.DefBuckets, + }, []string{"keeper_type"}), + UpdateDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "update_duration_seconds", + Help: "Duration of keeper update operations", + Buckets: prometheus.DefBuckets, + }, []string{"keeper_type"}), + ExposeDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "expose_duration_seconds", + Help: "Duration of keeper expose operations", + Buckets: prometheus.DefBuckets, + }, []string{"keeper_type"}), + DeleteDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "delete_duration_seconds", + Help: "Duration of keeper delete operations", + Buckets: prometheus.DefBuckets, + }, []string{"keeper_type"}), + } +} + +// NewKeeperMetrics creates a new KeeperMetrics struct containing registered metrics +func NewKeeperMetrics(reg prometheus.Registerer) *KeeperMetrics { + m := newKeeperMetrics() + + if reg != nil { + reg.MustRegister( + m.StoreDuration, + m.UpdateDuration, + m.ExposeDuration, + m.DeleteDuration, + ) + } + + return m +} + +func NewTestMetrics() *KeeperMetrics { + return newKeeperMetrics() +} diff --git a/pkg/registry/apis/secret/secretkeeper/secretkeeper.go b/pkg/registry/apis/secret/secretkeeper/secretkeeper.go index 3a580728f50..9cb1a4ec495 100644 --- a/pkg/registry/apis/secret/secretkeeper/secretkeeper.go +++ b/pkg/registry/apis/secret/secretkeeper/secretkeeper.go @@ -6,6 +6,7 @@ import ( secretv0alpha1 "github.com/grafana/grafana/pkg/apis/secret/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/registry/apis/secret/secretkeeper/sqlkeeper" + "github.com/prometheus/client_golang/prometheus" ) // OSSKeeperService is the OSS implementation of the Service interface. @@ -19,10 +20,11 @@ func ProvideService( tracer trace.Tracer, store contracts.EncryptedValueStorage, encryptionManager contracts.EncryptionManager, + reg prometheus.Registerer, ) (*OSSKeeperService, error) { return &OSSKeeperService{ // TODO: rename to system keeper or something like that - systemKeeper: sqlkeeper.NewSQLKeeper(tracer, encryptionManager, store), + systemKeeper: sqlkeeper.NewSQLKeeper(tracer, encryptionManager, store, reg), }, nil } diff --git a/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go b/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go index d2c5aea98d4..f202babab8d 100644 --- a/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go +++ b/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go @@ -49,7 +49,7 @@ func setupTestService(t *testing.T, cfg *setting.Cfg) (*OSSKeeperService, error) database := database.ProvideDatabase(testDB, tracer) features := featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, featuremgmt.FlagSecretsManagementAppPlatform) - dataKeyStore, err := encryptionstorage.ProvideDataKeyStorage(database, tracer, features) + dataKeyStore, err := encryptionstorage.ProvideDataKeyStorage(database, tracer, features, nil) require.NoError(t, err) encValueStore, err := encryptionstorage.ProvideEncryptedValueStorage(database, tracer, features) @@ -59,7 +59,7 @@ func setupTestService(t *testing.T, cfg *setting.Cfg) (*OSSKeeperService, error) require.NoError(t, err) // Initialize the keeper service - keeperService, err := ProvideService(tracer, encValueStore, encryptionManager) + keeperService, err := ProvideService(tracer, encValueStore, encryptionManager, nil) return keeperService, err } diff --git a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go index 04caae1237a..30656bb8e65 100644 --- a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go +++ b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go @@ -3,9 +3,12 @@ package sqlkeeper import ( "context" "fmt" + "time" secretv0alpha1 "github.com/grafana/grafana/pkg/apis/secret/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" + "github.com/grafana/grafana/pkg/registry/apis/secret/secretkeeper/metrics" + "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" ) @@ -14,6 +17,7 @@ type SQLKeeper struct { tracer trace.Tracer encryptionManager contracts.EncryptionManager store contracts.EncryptedValueStorage + metrics *metrics.KeeperMetrics } var _ contracts.Keeper = (*SQLKeeper)(nil) @@ -22,19 +26,21 @@ func NewSQLKeeper( tracer trace.Tracer, encryptionManager contracts.EncryptionManager, store contracts.EncryptedValueStorage, + reg prometheus.Registerer, ) *SQLKeeper { return &SQLKeeper{ tracer: tracer, encryptionManager: encryptionManager, store: store, + metrics: metrics.NewKeeperMetrics(reg), } } -// TODO: parameter cfg is not being used -func (s *SQLKeeper) Store(ctx context.Context, _ secretv0alpha1.KeeperConfig, namespace string, exposedValueOrRef string) (contracts.ExternalID, error) { +func (s *SQLKeeper) Store(ctx context.Context, cfg secretv0alpha1.KeeperConfig, namespace string, exposedValueOrRef string) (contracts.ExternalID, error) { ctx, span := s.tracer.Start(ctx, "SQLKeeper.Store", trace.WithAttributes(attribute.String("namespace", namespace))) defer span.End() + start := time.Now() encryptedData, err := s.encryptionManager.Encrypt(ctx, namespace, []byte(exposedValueOrRef)) if err != nil { return "", fmt.Errorf("unable to encrypt value: %w", err) @@ -45,8 +51,8 @@ func (s *SQLKeeper) Store(ctx context.Context, _ secretv0alpha1.KeeperConfig, na return "", fmt.Errorf("unable to store encrypted value: %w", err) } + s.metrics.StoreDuration.WithLabelValues(string(cfg.Type())).Observe(time.Since(start).Seconds()) externalID := contracts.ExternalID(encryptedVal.UID) - span.SetAttributes(attribute.String("externalID", externalID.String())) return externalID, nil @@ -59,6 +65,7 @@ func (s *SQLKeeper) Expose(ctx context.Context, cfg secretv0alpha1.KeeperConfig, )) defer span.End() + start := time.Now() encryptedValue, err := s.store.Get(ctx, namespace, externalID.String()) if err != nil { return "", fmt.Errorf("unable to get encrypted value: %w", err) @@ -70,6 +77,8 @@ func (s *SQLKeeper) Expose(ctx context.Context, cfg secretv0alpha1.KeeperConfig, } exposedValue := secretv0alpha1.NewExposedSecureValue(string(exposedBytes)) + s.metrics.ExposeDuration.WithLabelValues(string(cfg.Type())).Observe(time.Since(start).Seconds()) + return exposedValue, nil } @@ -80,10 +89,14 @@ func (s *SQLKeeper) Delete(ctx context.Context, cfg secretv0alpha1.KeeperConfig, )) defer span.End() + start := time.Now() err := s.store.Delete(ctx, namespace, externalID.String()) if err != nil { return fmt.Errorf("failed to delete encrypted value: %w", err) } + + s.metrics.DeleteDuration.WithLabelValues(string(cfg.Type())).Observe(time.Since(start).Seconds()) + return nil } @@ -94,6 +107,7 @@ func (s *SQLKeeper) Update(ctx context.Context, cfg secretv0alpha1.KeeperConfig, )) defer span.End() + start := time.Now() encryptedData, err := s.encryptionManager.Encrypt(ctx, namespace, []byte(exposedValueOrRef)) if err != nil { return fmt.Errorf("unable to encrypt value: %w", err) @@ -103,5 +117,8 @@ func (s *SQLKeeper) Update(ctx context.Context, cfg secretv0alpha1.KeeperConfig, if err != nil { return fmt.Errorf("failed to update encrypted value: %w", err) } + + s.metrics.UpdateDuration.WithLabelValues(string(cfg.Type())).Observe(time.Since(start).Seconds()) + return nil } diff --git a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go index 500d04bdf0e..fa9aa7a6ba5 100644 --- a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go +++ b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go @@ -159,7 +159,7 @@ func setupTestService(t *testing.T, cfg *setting.Cfg) (*SQLKeeper, error) { features := featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, featuremgmt.FlagSecretsManagementAppPlatform) // Initialize the encryption manager - dataKeyStore, err := encryptionstorage.ProvideDataKeyStorage(database, tracer, features) + dataKeyStore, err := encryptionstorage.ProvideDataKeyStorage(database, tracer, features, nil) require.NoError(t, err) usageStats := &usagestats.UsageStatsMock{T: t} @@ -178,7 +178,7 @@ func setupTestService(t *testing.T, cfg *setting.Cfg) (*SQLKeeper, error) { require.NoError(t, err) // Initialize the SQLKeeper - sqlKeeper := NewSQLKeeper(tracer, encMgr, encValueStore) + sqlKeeper := NewSQLKeeper(tracer, encMgr, encValueStore, nil) return sqlKeeper, nil } diff --git a/pkg/setting/setting_secrets_manager.go b/pkg/setting/setting_secrets_manager.go index f52021a9ebf..f8a3b186e53 100644 --- a/pkg/setting/setting_secrets_manager.go +++ b/pkg/setting/setting_secrets_manager.go @@ -2,17 +2,10 @@ package setting import ( "regexp" - "time" "github.com/grafana/grafana/pkg/services/kmsproviders" ) -type EncryptionSettings struct { - DataKeysCacheTTL time.Duration - DataKeysCleanupInterval time.Duration - Algorithm string -} - type SecretsManagerSettings struct { SecretKey string EncryptionProvider string diff --git a/pkg/storage/secret/encryption/data_key_store.go b/pkg/storage/secret/encryption/data_key_store.go index c38c4ce1153..78c2bf63910 100644 --- a/pkg/storage/secret/encryption/data_key_store.go +++ b/pkg/storage/secret/encryption/data_key_store.go @@ -9,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" + "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" ) @@ -19,9 +20,15 @@ type encryptionStoreImpl struct { dialect sqltemplate.Dialect tracer trace.Tracer log log.Logger + metrics *DataKeyMetrics } -func ProvideDataKeyStorage(db contracts.Database, tracer trace.Tracer, features featuremgmt.FeatureToggles) (contracts.DataKeyStorage, error) { +func ProvideDataKeyStorage( + db contracts.Database, + tracer trace.Tracer, + features featuremgmt.FeatureToggles, + registerer prometheus.Registerer, +) (contracts.DataKeyStorage, error) { if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) || !features.IsEnabledGlobally(featuremgmt.FlagSecretsManagementAppPlatform) { return &encryptionStoreImpl{}, nil @@ -32,6 +39,7 @@ func ProvideDataKeyStorage(db contracts.Database, tracer trace.Tracer, features dialect: sqltemplate.DialectForDriver(db.DriverName()), tracer: tracer, log: log.New("encryption.store"), + metrics: NewDataKeyMetrics(registerer), } return store, nil diff --git a/pkg/storage/secret/encryption/data_key_store_test.go b/pkg/storage/secret/encryption/data_key_store_test.go index 450577f4584..4f2d52d3387 100644 --- a/pkg/storage/secret/encryption/data_key_store_test.go +++ b/pkg/storage/secret/encryption/data_key_store_test.go @@ -31,7 +31,7 @@ func TestEncryptionStoreImpl_DataKeyLifecycle(t *testing.T) { testDB := sqlstore.NewTestStore(t, sqlstore.WithMigrator(migrator.New())) tracer := noop.NewTracerProvider().Tracer("test") features := featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, featuremgmt.FlagSecretsManagementAppPlatform) - store, err := ProvideDataKeyStorage(database.ProvideDatabase(testDB, tracer), tracer, features) + store, err := ProvideDataKeyStorage(database.ProvideDatabase(testDB, tracer), tracer, features, nil) require.NoError(t, err) ctx := context.Background() diff --git a/pkg/storage/secret/encryption/metrics.go b/pkg/storage/secret/encryption/metrics.go new file mode 100644 index 00000000000..a7a116541ce --- /dev/null +++ b/pkg/storage/secret/encryption/metrics.go @@ -0,0 +1,98 @@ +package encryption + +import ( + "github.com/prometheus/client_golang/prometheus" +) + +const ( + namespace = "grafana_secrets_manager" + subsystem = "data_key_storage" +) + +// DataKeyMetrics is a struct that contains all the metrics for all operations of encryption storage. +type DataKeyMetrics struct { + CreateDataKeyDuration prometheus.Histogram + GetDataKeyDuration prometheus.Histogram + GetCurrentDataKeyDuration prometheus.Histogram + GetAllDataKeysDuration prometheus.Histogram + DisableDataKeysDuration prometheus.Histogram + DeleteDataKeyDuration prometheus.Histogram + ReEncryptDataKeysDuration prometheus.Histogram +} + +func newDataKeyMetrics() *DataKeyMetrics { + return &DataKeyMetrics{ + CreateDataKeyDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "create_data_key_duration_seconds", + Help: "Duration of create data key operations", + Buckets: prometheus.DefBuckets, + }), + GetDataKeyDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "get_data_key_duration_seconds", + Help: "Duration of get data key operations", + Buckets: prometheus.DefBuckets, + }), + GetCurrentDataKeyDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "get_current_data_key_duration_seconds", + Help: "Duration of get current data key operations", + Buckets: prometheus.DefBuckets, + }), + GetAllDataKeysDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "get_all_data_keys_duration_seconds", + Help: "Duration of get all data keys operations", + Buckets: prometheus.DefBuckets, + }), + DisableDataKeysDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "disable_data_keys_duration_seconds", + Help: "Duration of disable data keys operations", + Buckets: prometheus.DefBuckets, + }), + DeleteDataKeyDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "delete_data_key_duration_seconds", + Help: "Duration of delete data key operations", + Buckets: prometheus.DefBuckets, + }), + ReEncryptDataKeysDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "re_encrypt_data_keys_duration_seconds", + Help: "Duration of re-encrypt data keys operations", + Buckets: prometheus.DefBuckets, + }), + } +} + +// NewDataKeyMetrics returns a singleton instance of the SecretsMetrics struct containing registered metrics +func NewDataKeyMetrics(reg prometheus.Registerer) *DataKeyMetrics { + m := newDataKeyMetrics() + + if reg != nil { + reg.MustRegister( + m.CreateDataKeyDuration, + m.GetDataKeyDuration, + m.GetCurrentDataKeyDuration, + m.GetAllDataKeysDuration, + m.DisableDataKeysDuration, + m.DeleteDataKeyDuration, + m.ReEncryptDataKeysDuration, + ) + } + + return m +} + +func NewTestMetrics() *DataKeyMetrics { + return newDataKeyMetrics() +} diff --git a/pkg/storage/secret/metadata/keeper_store.go b/pkg/storage/secret/metadata/keeper_store.go index 89b115eef2c..defc9974125 100644 --- a/pkg/storage/secret/metadata/keeper_store.go +++ b/pkg/storage/secret/metadata/keeper_store.go @@ -3,12 +3,15 @@ package metadata import ( "context" "fmt" + "time" secretv0alpha1 "github.com/grafana/grafana/pkg/apis/secret/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/storage/secret/metadata/metrics" "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" + "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" @@ -19,11 +22,17 @@ type keeperMetadataStorage struct { db contracts.Database dialect sqltemplate.Dialect tracer trace.Tracer + metrics *metrics.StorageMetrics } var _ contracts.KeeperMetadataStorage = (*keeperMetadataStorage)(nil) -func ProvideKeeperMetadataStorage(db contracts.Database, tracer trace.Tracer, features featuremgmt.FeatureToggles) (contracts.KeeperMetadataStorage, error) { +func ProvideKeeperMetadataStorage( + db contracts.Database, + tracer trace.Tracer, + features featuremgmt.FeatureToggles, + reg prometheus.Registerer, +) (contracts.KeeperMetadataStorage, error) { if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) || !features.IsEnabledGlobally(featuremgmt.FlagSecretsManagementAppPlatform) { return &keeperMetadataStorage{}, nil @@ -33,10 +42,12 @@ func ProvideKeeperMetadataStorage(db contracts.Database, tracer trace.Tracer, fe db: db, dialect: sqltemplate.DialectForDriver(db.DriverName()), tracer: tracer, + metrics: metrics.NewStorageMetrics(reg), }, nil } func (s *keeperMetadataStorage) Create(ctx context.Context, keeper *secretv0alpha1.Keeper, actorUID string) (*secretv0alpha1.Keeper, error) { + start := time.Now() ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.Create", trace.WithAttributes( attribute.String("name", keeper.GetName()), attribute.String("namespace", keeper.GetNamespace()), @@ -53,7 +64,6 @@ func (s *keeperMetadataStorage) Create(ctx context.Context, keeper *secretv0alph SQLTemplate: sqltemplate.New(s.dialect), Row: row, } - query, err := sqltemplate.Execute(sqlKeeperCreate, req) if err != nil { return nil, fmt.Errorf("execute template %q: %w", sqlKeeperCreate.Name(), err) @@ -65,6 +75,11 @@ func (s *keeperMetadataStorage) Create(ctx context.Context, keeper *secretv0alph return err } + // Validate before inserting that any `secureValues` referenced exist and do not reference other third-party keepers. + if err := s.validateSecureValueReferences(ctx, keeper); err != nil { + return err + } + result, err := s.db.ExecContext(ctx, query, req.GetArgs()...) if err != nil { return fmt.Errorf("inserting row: %w", err) @@ -90,10 +105,14 @@ func (s *keeperMetadataStorage) Create(ctx context.Context, keeper *secretv0alph return nil, fmt.Errorf("failed to convert to kubernetes object: %w", err) } + s.metrics.KeeperMetadataCreateDuration.WithLabelValues(string(createdKeeper.Spec.GetType())).Observe(time.Since(start).Seconds()) + s.metrics.KeeperMetadataCreateCount.WithLabelValues(string(createdKeeper.Spec.GetType())).Inc() + return createdKeeper, nil } func (s *keeperMetadataStorage) Read(ctx context.Context, namespace xkube.Namespace, name string, opts contracts.ReadOpts) (*secretv0alpha1.Keeper, error) { + start := time.Now() ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.Read", trace.WithAttributes( attribute.String("name", name), attribute.String("namespace", namespace.String()), @@ -111,6 +130,9 @@ func (s *keeperMetadataStorage) Read(ctx context.Context, namespace xkube.Namesp return nil, fmt.Errorf("failed to convert to kubernetes object: %w", err) } + s.metrics.KeeperMetadataGetDuration.WithLabelValues(string(keeper.Spec.GetType())).Observe(time.Since(start).Seconds()) + s.metrics.KeeperMetadataGetCount.WithLabelValues(string(keeper.Spec.GetType())).Inc() + return keeper, nil } @@ -153,6 +175,7 @@ func (s *keeperMetadataStorage) read(ctx context.Context, namespace, name string } func (s *keeperMetadataStorage) Update(ctx context.Context, newKeeper *secretv0alpha1.Keeper, actorUID string) (*secretv0alpha1.Keeper, error) { + start := time.Now() ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.Update", trace.WithAttributes( attribute.String("name", newKeeper.GetName()), attribute.String("namespace", newKeeper.GetNamespace()), @@ -168,6 +191,11 @@ func (s *keeperMetadataStorage) Update(ctx context.Context, newKeeper *secretv0a return err } + // Validate before updating that any `secureValues` referenced exists and does not reference other third-party keepers. + if err := s.validateSecureValueReferences(ctx, newKeeper); err != nil { + return err + } + // Read old value first. oldKeeperRow, err := s.read(ctx, newKeeper.Namespace, newKeeper.Name, contracts.ReadOpts{ForUpdate: true}) if err != nil { @@ -217,10 +245,14 @@ func (s *keeperMetadataStorage) Update(ctx context.Context, newKeeper *secretv0a return nil, fmt.Errorf("failed to convert to kubernetes object: %w", err) } + s.metrics.KeeperMetadataUpdateDuration.WithLabelValues(string(keeper.Spec.GetType())).Observe(time.Since(start).Seconds()) + s.metrics.KeeperMetadataUpdateCount.WithLabelValues(string(keeper.Spec.GetType())).Inc() + return keeper, nil } func (s *keeperMetadataStorage) Delete(ctx context.Context, namespace xkube.Namespace, name string) error { + start := time.Now() ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.Delete", trace.WithAttributes( attribute.String("name", name), attribute.String("namespace", namespace.String()), @@ -253,10 +285,14 @@ func (s *keeperMetadataStorage) Delete(ctx context.Context, namespace xkube.Name return fmt.Errorf("expected 1 row affected, got %d for %s on %s", rowsAffected, name, namespace) } + s.metrics.KeeperMetadataDeleteDuration.Observe(time.Since(start).Seconds()) + s.metrics.KeeperMetadataDeleteCount.Inc() + return nil } func (s *keeperMetadataStorage) List(ctx context.Context, namespace xkube.Namespace) (keeperList []secretv0alpha1.Keeper, err error) { + start := time.Now() ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.List", trace.WithAttributes( attribute.String("namespace", namespace.String()), )) @@ -306,6 +342,9 @@ func (s *keeperMetadataStorage) List(ctx context.Context, namespace xkube.Namesp return nil, fmt.Errorf("read rows error: %w", err) } + s.metrics.KeeperMetadataListDuration.Observe(time.Since(start).Seconds()) + s.metrics.KeeperMetadataListCount.Inc() + return keepers, nil } @@ -467,9 +506,10 @@ func (s *keeperMetadataStorage) GetKeeperConfig(ctx context.Context, namespace s // Check if keeper is the systemwide one. if name == nil { - return nil, nil + return &secretv0alpha1.SystemKeeperConfig{}, nil } + start := time.Now() span.SetAttributes(attribute.String("name", *name)) // Load keeper config from metadata store, or TODO: keeper cache. @@ -480,6 +520,8 @@ func (s *keeperMetadataStorage) GetKeeperConfig(ctx context.Context, namespace s keeperConfig := toProvider(secretv0alpha1.KeeperType(kp.Type), kp.Payload) + s.metrics.KeeperMetadataGetKeeperConfigDuration.Observe(time.Since(start).Seconds()) + // TODO: this would be a good place to check if credentials are secure values and load them. return keeperConfig, nil } diff --git a/pkg/storage/secret/metadata/keeper_store_test.go b/pkg/storage/secret/metadata/keeper_store_test.go index a2165d5273f..644b3ccf714 100644 --- a/pkg/storage/secret/metadata/keeper_store_test.go +++ b/pkg/storage/secret/metadata/keeper_store_test.go @@ -40,7 +40,7 @@ func Test_KeeperMetadataStorage_GetKeeperConfig(t *testing.T) { // get system keeper config keeperConfig, err := keeperMetadataStorage.GetKeeperConfig(ctx, defaultKeeperNS, nil, contracts.ReadOpts{}) require.NoError(t, err) - require.Nil(t, keeperConfig) + require.IsType(t, &secretv0alpha1.SystemKeeperConfig{}, keeperConfig) }) t.Run("get test keeper config", func(t *testing.T) { @@ -340,7 +340,7 @@ func initStorage(t *testing.T) contracts.KeeperMetadataStorage { features := featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, featuremgmt.FlagSecretsManagementAppPlatform) // Initialize the keeper storage - keeperMetadataStorage, err := ProvideKeeperMetadataStorage(db, tracer, features) + keeperMetadataStorage, err := ProvideKeeperMetadataStorage(db, tracer, features, nil) require.NoError(t, err) return keeperMetadataStorage } diff --git a/pkg/storage/secret/metadata/metrics/metrics.go b/pkg/storage/secret/metadata/metrics/metrics.go new file mode 100644 index 00000000000..bffbaebc604 --- /dev/null +++ b/pkg/storage/secret/metadata/metrics/metrics.go @@ -0,0 +1,356 @@ +package metrics + +import ( + "sync" + + "github.com/prometheus/client_golang/prometheus" +) + +const ( + namespace = "grafana_secrets_manager" + subsystem = "storage" +) + +// StorageMetrics is a struct that contains all the metrics for all operations of secrets storage. +type StorageMetrics struct { + OutboxAppendDuration *prometheus.HistogramVec + OutboxReceiveDuration prometheus.Histogram + OutboxAppendCount *prometheus.CounterVec + OutboxReceiveCount prometheus.Counter + OutboxDeleteDuration prometheus.Histogram + OutboxDeleteCount prometheus.Counter + OutboxIncrementReceiveCountDuration prometheus.Histogram + OutboxIncrementReceiveCountCount prometheus.Counter + OutboxTotalMessageLifetimeDuration *prometheus.HistogramVec + + KeeperMetadataCreateDuration *prometheus.HistogramVec + KeeperMetadataCreateCount *prometheus.CounterVec + KeeperMetadataUpdateDuration *prometheus.HistogramVec + KeeperMetadataUpdateCount *prometheus.CounterVec + KeeperMetadataDeleteDuration prometheus.Histogram + KeeperMetadataDeleteCount prometheus.Counter + KeeperMetadataGetDuration *prometheus.HistogramVec + KeeperMetadataGetCount *prometheus.CounterVec + KeeperMetadataListDuration prometheus.Histogram + KeeperMetadataListCount prometheus.Counter + KeeperMetadataGetKeeperConfigDuration prometheus.Histogram + + SecureValueMetadataCreateDuration prometheus.Histogram + SecureValueMetadataCreateCount prometheus.Counter + SecureValueMetadataUpdateDuration prometheus.Histogram + SecureValueMetadataUpdateCount prometheus.Counter + SecureValueMetadataDeleteDuration prometheus.Histogram + SecureValueMetadataDeleteCount prometheus.Counter + SecureValueMetadataGetDuration prometheus.Histogram + SecureValueMetadataGetCount prometheus.Counter + SecureValueMetadataListDuration prometheus.Histogram + SecureValueMetadataListCount prometheus.Counter + SecureValueGetForDecryptDuration prometheus.Histogram + SecureValueSetExternalIDDuration prometheus.Histogram + SecureValueSetStatusDuration prometheus.Histogram + + DecryptDuration *prometheus.HistogramVec + DecryptRequestCount *prometheus.CounterVec +} + +func newStorageMetrics() *StorageMetrics { + return &StorageMetrics{ + // Outbox metrics + OutboxAppendDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "outbox_append_duration_seconds", + Help: "Duration of outbox message append operations", + Buckets: prometheus.DefBuckets, + }, []string{"message_type"}), + OutboxAppendCount: prometheus.NewCounterVec(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "outbox_append_count", + Help: "Count of outbox message append operations", + }, []string{"message_type"}), + OutboxReceiveDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "outbox_receive_duration_seconds", + Help: "Duration of outbox message receive operations", + Buckets: prometheus.DefBuckets, + }), + OutboxReceiveCount: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "outbox_receive_count", + Help: "Count of outbox message receive operations", + }), + OutboxDeleteDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "outbox_delete_duration_seconds", + Help: "Duration of outbox message delete operations", + Buckets: prometheus.DefBuckets, + }), + OutboxDeleteCount: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "outbox_delete_count", + Help: "Count of outbox message delete operations", + }), + OutboxIncrementReceiveCountDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "outbox_increment_receive_count_duration_seconds", + Help: "Duration of outbox message increment receive count operations", + Buckets: prometheus.DefBuckets, + }), + OutboxIncrementReceiveCountCount: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "outbox_increment_receive_count_count", + Help: "Count of outbox message increment receive count operations", + }), + OutboxTotalMessageLifetimeDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "outbox_total_message_lifetime_duration_seconds", + Help: "Total duration of outbox message lifetime", + Buckets: prometheus.DefBuckets, + }, []string{"message_type"}), + + // Keeper metrics + KeeperMetadataCreateDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "keeper_metadata_create_duration_seconds", + Help: "Duration of keeper metadata create operations", + Buckets: prometheus.DefBuckets, + }, []string{"keeper_type"}), + KeeperMetadataCreateCount: prometheus.NewCounterVec(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "keeper_metadata_create_count", + Help: "Count of keeper metadata create operations", + }, []string{"keeper_type"}), + KeeperMetadataUpdateDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "keeper_metadata_update_duration_seconds", + Help: "Duration of keeper metadata update operations", + Buckets: prometheus.DefBuckets, + }, []string{"keeper_type"}), + KeeperMetadataUpdateCount: prometheus.NewCounterVec(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "keeper_metadata_update_count", + Help: "Count of keeper metadata update operations", + }, []string{"keeper_type"}), + KeeperMetadataDeleteDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "keeper_metadata_delete_duration_seconds", + Help: "Duration of keeper metadata delete operations", + Buckets: prometheus.DefBuckets, + }), + KeeperMetadataDeleteCount: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "keeper_metadata_delete_count", + Help: "Count of keeper metadata delete operations", + }), + KeeperMetadataGetDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "keeper_metadata_get_duration_seconds", + Help: "Duration of keeper metadata get operations", + Buckets: prometheus.DefBuckets, + }, []string{"keeper_type"}), + KeeperMetadataGetCount: prometheus.NewCounterVec(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "keeper_metadata_get_count", + Help: "Count of keeper metadata get operations", + }, []string{"keeper_type"}), + KeeperMetadataListDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "keeper_metadata_list_duration_seconds", + Help: "Duration of keeper metadata list operations", + Buckets: prometheus.DefBuckets, + }), + KeeperMetadataListCount: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "keeper_metadata_list_count", + Help: "Count of keeper metadata list operations", + }), + KeeperMetadataGetKeeperConfigDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "keeper_metadata_get_keeper_config_duration_seconds", + Help: "Duration of keeper metadata get keeper config operations", + Buckets: prometheus.DefBuckets, + }), + + // Secure value metrics + SecureValueMetadataCreateDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_metadata_create_duration_seconds", + Help: "Duration of secure value metadata create operations", + Buckets: prometheus.DefBuckets, + }), + SecureValueMetadataCreateCount: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_metadata_create_count", + Help: "Count of secure value metadata create operations", + }), + SecureValueMetadataUpdateDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_metadata_update_duration_seconds", + Help: "Duration of secure value metadata update operations", + Buckets: prometheus.DefBuckets, + }), + SecureValueMetadataUpdateCount: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_metadata_update_count", + Help: "Count of secure value metadata update operations", + }), + SecureValueMetadataDeleteDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_metadata_delete_duration_seconds", + Help: "Duration of secure value metadata delete operations", + Buckets: prometheus.DefBuckets, + }), + SecureValueMetadataDeleteCount: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_metadata_delete_count", + Help: "Count of secure value metadata delete operations", + }), + SecureValueMetadataGetDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_metadata_get_duration_seconds", + Help: "Duration of secure value metadata get operations", + Buckets: prometheus.DefBuckets, + }), + SecureValueMetadataGetCount: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_metadata_get_count", + Help: "Count of secure value metadata get operations", + }), + SecureValueMetadataListDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_metadata_list_duration_seconds", + Help: "Duration of secure value metadata list operations", + Buckets: prometheus.DefBuckets, + }), + SecureValueMetadataListCount: prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_metadata_list_count", + Help: "Count of secure value metadata list operations", + }), + SecureValueGetForDecryptDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_get_for_decrypt_duration_seconds", + Help: "Duration of secure value get for decrypt operations", + Buckets: prometheus.DefBuckets, + }), + SecureValueSetExternalIDDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_set_external_id_duration_seconds", + Help: "Duration of secure value set external id operations", + Buckets: prometheus.DefBuckets, + }), + SecureValueSetStatusDuration: prometheus.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "secure_value_set_status_duration_seconds", + Help: "Duration of secure value set status operations", + Buckets: prometheus.DefBuckets, + }), + + // Decrypt metrics + DecryptDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "decrypt_duration_seconds", + Help: "Duration of decrypt operations", + Buckets: prometheus.DefBuckets, + }, []string{"successful"}), + DecryptRequestCount: prometheus.NewCounterVec(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: subsystem, + Name: "decrypt_request_count", + Help: "Count of decrypt operations", + }, []string{"successful"}), + } +} + +var ( + initOnce sync.Once + metricsInstance *StorageMetrics +) + +// NewStorageMetrics returns a singleton instance of the SecretsMetrics struct containing registered metrics +func NewStorageMetrics(reg prometheus.Registerer) *StorageMetrics { + initOnce.Do(func() { + m := newStorageMetrics() + + if reg != nil { + reg.MustRegister( + m.OutboxAppendDuration, + m.OutboxAppendCount, + m.OutboxReceiveDuration, + m.OutboxReceiveCount, + m.OutboxDeleteDuration, + m.OutboxDeleteCount, + m.OutboxIncrementReceiveCountDuration, + m.OutboxIncrementReceiveCountCount, + m.OutboxTotalMessageLifetimeDuration, + m.KeeperMetadataCreateDuration, + m.KeeperMetadataCreateCount, + m.KeeperMetadataUpdateDuration, + m.KeeperMetadataUpdateCount, + m.KeeperMetadataDeleteDuration, + m.KeeperMetadataDeleteCount, + m.KeeperMetadataGetDuration, + m.KeeperMetadataGetCount, + m.KeeperMetadataListDuration, + m.KeeperMetadataListCount, + m.KeeperMetadataGetKeeperConfigDuration, + m.SecureValueMetadataCreateDuration, + m.SecureValueMetadataCreateCount, + m.SecureValueMetadataUpdateDuration, + m.SecureValueMetadataUpdateCount, + m.SecureValueMetadataDeleteDuration, + m.SecureValueMetadataDeleteCount, + m.SecureValueMetadataGetDuration, + m.SecureValueMetadataGetCount, + m.SecureValueMetadataListDuration, + m.SecureValueMetadataListCount, + m.SecureValueGetForDecryptDuration, + m.SecureValueSetExternalIDDuration, + m.SecureValueSetStatusDuration, + m.DecryptDuration, + m.DecryptRequestCount, + ) + } + + metricsInstance = m + }) + + return metricsInstance +} + +func NewTestMetrics() *StorageMetrics { + return newStorageMetrics() +} diff --git a/pkg/storage/secret/metadata/outbox_store.go b/pkg/storage/secret/metadata/outbox_store.go index bfc01bb1444..1f183b7882b 100644 --- a/pkg/storage/secret/metadata/outbox_store.go +++ b/pkg/storage/secret/metadata/outbox_store.go @@ -6,7 +6,9 @@ import ( "fmt" "time" + "github.com/grafana/grafana/pkg/storage/secret/metadata/metrics" unifiedsql "github.com/grafana/grafana/pkg/storage/unified/sql" + "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" @@ -20,15 +22,18 @@ type outboxStore struct { db contracts.Database dialect sqltemplate.Dialect tracer trace.Tracer + metrics *metrics.StorageMetrics } func ProvideOutboxQueue( db contracts.Database, tracer trace.Tracer, + reg prometheus.Registerer, ) contracts.OutboxQueue { return &outboxStore{ db: db, dialect: sqltemplate.DialectForDriver(db.DriverName()), + metrics: metrics.NewStorageMetrics(reg), tracer: tracer, } } @@ -47,6 +52,7 @@ type outboxMessageDB struct { } func (s *outboxStore) Append(ctx context.Context, input contracts.AppendOutboxMessage) (messageID int64, err error) { + start := time.Now() ctx, span := s.tracer.Start(ctx, "outboxStore.Append", trace.WithAttributes( attribute.String("name", input.Name), attribute.String("namespace", input.Namespace), @@ -73,6 +79,9 @@ func (s *outboxStore) Append(ctx context.Context, input contracts.AppendOutboxMe return messageID, fmt.Errorf("inserting message into outbox table: %+w", err) } + s.metrics.OutboxAppendDuration.WithLabelValues(string(input.Type)).Observe(time.Since(start).Seconds()) + s.metrics.OutboxAppendCount.WithLabelValues(string(input.Type)).Inc() + return messageID, nil } @@ -147,6 +156,7 @@ func (s *outboxStore) insertMessage(ctx context.Context, input contracts.AppendO } func (s *outboxStore) ReceiveN(ctx context.Context, limit uint) ([]contracts.OutboxMessage, error) { + start := time.Now() messageIDs, err := s.fetchMessageIdsInQueue(ctx, limit) if err != nil { return nil, fmt.Errorf("fetching message ids from queue: %w", err) @@ -223,6 +233,9 @@ func (s *outboxStore) ReceiveN(ctx context.Context, limit uint) ([]contracts.Out return messages, fmt.Errorf("reading rows: %w", err) } + s.metrics.OutboxReceiveDuration.Observe(time.Since(start).Seconds()) + s.metrics.OutboxReceiveCount.Add(float64(len(messages))) + return messages, nil } @@ -275,10 +288,14 @@ func (s *outboxStore) Delete(ctx context.Context, messageID int64) (err error) { assert.True(messageID != 0, "outboxStore.Delete: messageID is required") + start := time.Now() if err := s.deleteMessage(ctx, messageID); err != nil { return fmt.Errorf("deleting message from outbox table %+w", err) } + s.metrics.OutboxDeleteDuration.Observe(time.Since(start).Seconds()) + s.metrics.OutboxDeleteCount.Inc() + return nil } @@ -320,6 +337,9 @@ func (s *outboxStore) deleteMessage(ctx context.Context, messageID int64) error return fmt.Errorf("rows error: %w", err) } + totalLifetime := time.Since(time.UnixMilli(timestamp)) + s.metrics.OutboxTotalMessageLifetimeDuration.WithLabelValues(messageType).Observe(totalLifetime.Seconds()) + // Then delete the object delReq := deleteSecureValueOutbox{ SQLTemplate: sqltemplate.New(s.dialect), @@ -359,6 +379,7 @@ func (s *outboxStore) IncrementReceiveCount(ctx context.Context, messageIDs []in MessageIDs: messageIDs, } + start := time.Now() query, err := sqltemplate.Execute(sqlSecureValueOutboxUpdateReceiveCount, req) if err != nil { return fmt.Errorf("execute template %q: %w", sqlSecureValueOutboxUpdateReceiveCount.Name(), err) @@ -369,5 +390,8 @@ func (s *outboxStore) IncrementReceiveCount(ctx context.Context, messageIDs []in return fmt.Errorf("updating outbox messages receive count: %w", err) } + s.metrics.OutboxIncrementReceiveCountDuration.Observe(time.Since(start).Seconds()) + s.metrics.OutboxIncrementReceiveCountCount.Add(float64(len(messageIDs))) + return nil } diff --git a/pkg/storage/secret/metadata/outbox_store_test.go b/pkg/storage/secret/metadata/outbox_store_test.go index 518618833cf..f757e8b6b38 100644 --- a/pkg/storage/secret/metadata/outbox_store_test.go +++ b/pkg/storage/secret/metadata/outbox_store_test.go @@ -39,7 +39,7 @@ func (model *outboxStoreModel) Append(messageID int64, message contracts.AppendO func (model *outboxStoreModel) ReceiveN(n uint) []contracts.OutboxMessage { maxMessages := min(len(model.rows), int(n)) if maxMessages == 0 { - return []contracts.OutboxMessage{} + return nil } return model.rows[:maxMessages] } @@ -115,7 +115,7 @@ func TestOutboxStoreSecureValueOperationInProgress(t *testing.T) { ctx := context.Background() - outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB, tracer), tracer) + outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB, tracer), tracer, nil) _, err := outbox.Append(ctx, contracts.AppendOutboxMessage{ RequestID: "1", @@ -148,7 +148,7 @@ func TestOutboxStore(t *testing.T) { ctx := context.Background() - outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB, tracer), tracer) + outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB, tracer), tracer, nil) m1 := contracts.AppendOutboxMessage{ Type: contracts.CreateSecretOutboxMessage, @@ -216,7 +216,7 @@ func TestOutboxStoreProperty(t *testing.T) { testDB := sqlstore.NewTestStore(t, sqlstore.WithMigrator(migrator.New())) tracer := noop.NewTracerProvider().Tracer("test") - outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB, tracer), tracer) + outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB, tracer), tracer, nil) model := newOutboxStoreModel() diff --git a/pkg/storage/secret/metadata/secure_value_store.go b/pkg/storage/secret/metadata/secure_value_store.go index 39f42dcbdbe..6f79dfbd353 100644 --- a/pkg/storage/secret/metadata/secure_value_store.go +++ b/pkg/storage/secret/metadata/secure_value_store.go @@ -3,20 +3,28 @@ package metadata import ( "context" "fmt" + "time" secretv0alpha1 "github.com/grafana/grafana/pkg/apis/secret/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/storage/secret/metadata/metrics" "github.com/grafana/grafana/pkg/storage/unified/sql" "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" + "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" ) var _ contracts.SecureValueMetadataStorage = (*secureValueMetadataStorage)(nil) -func ProvideSecureValueMetadataStorage(db contracts.Database, tracer trace.Tracer, features featuremgmt.FeatureToggles) (contracts.SecureValueMetadataStorage, error) { +func ProvideSecureValueMetadataStorage( + db contracts.Database, + tracer trace.Tracer, + features featuremgmt.FeatureToggles, + reg prometheus.Registerer, +) (contracts.SecureValueMetadataStorage, error) { if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) || !features.IsEnabledGlobally(featuremgmt.FlagSecretsManagementAppPlatform) { return &secureValueMetadataStorage{}, nil @@ -25,6 +33,7 @@ func ProvideSecureValueMetadataStorage(db contracts.Database, tracer trace.Trace return &secureValueMetadataStorage{ db: db, dialect: sqltemplate.DialectForDriver(db.DriverName()), + metrics: metrics.NewStorageMetrics(reg), tracer: tracer, }, nil } @@ -33,10 +42,12 @@ func ProvideSecureValueMetadataStorage(db contracts.Database, tracer trace.Trace type secureValueMetadataStorage struct { db contracts.Database dialect sqltemplate.Dialect + metrics *metrics.StorageMetrics tracer trace.Tracer } func (s *secureValueMetadataStorage) Create(ctx context.Context, sv *secretv0alpha1.SecureValue, actorUID string) (*secretv0alpha1.SecureValue, error) { + start := time.Now() ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.Create", trace.WithAttributes( attribute.String("name", sv.GetName()), attribute.String("namespace", sv.GetNamespace()), @@ -117,10 +128,14 @@ func (s *secureValueMetadataStorage) Create(ctx context.Context, sv *secretv0alp return nil, fmt.Errorf("convert to kubernetes object: %w", err) } + s.metrics.SecureValueMetadataCreateDuration.Observe(time.Since(start).Seconds()) + s.metrics.SecureValueMetadataCreateCount.Inc() + return createdSecureValue, nil } func (s *secureValueMetadataStorage) Read(ctx context.Context, namespace xkube.Namespace, name string, opts contracts.ReadOpts) (*secretv0alpha1.SecureValue, error) { + start := time.Now() ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.Read", trace.WithAttributes( attribute.String("name", name), attribute.String("namespace", namespace.String()), @@ -138,10 +153,14 @@ func (s *secureValueMetadataStorage) Read(ctx context.Context, namespace xkube.N return nil, fmt.Errorf("convert to kubernetes object: %w", err) } + s.metrics.SecureValueMetadataGetDuration.Observe(time.Since(start).Seconds()) + s.metrics.SecureValueMetadataGetCount.Inc() + return secureValueKub, nil } func (s *secureValueMetadataStorage) Update(ctx context.Context, newSecureValue *secretv0alpha1.SecureValue, actorUID string) (*secretv0alpha1.SecureValue, error) { + start := time.Now() ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.Update", trace.WithAttributes( attribute.String("name", newSecureValue.GetName()), attribute.String("namespace", newSecureValue.GetNamespace()), @@ -228,10 +247,14 @@ func (s *secureValueMetadataStorage) Update(ctx context.Context, newSecureValue return nil, fmt.Errorf("convert to kubernetes object: %w", err) } + s.metrics.SecureValueMetadataUpdateDuration.Observe(time.Since(start).Seconds()) + s.metrics.SecureValueMetadataUpdateCount.Inc() + return secureValue, nil } func (s *secureValueMetadataStorage) Delete(ctx context.Context, namespace xkube.Namespace, name string) error { + start := time.Now() ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.Delete", trace.WithAttributes( attribute.String("name", name), attribute.String("namespace", namespace.String()), @@ -258,10 +281,14 @@ func (s *secureValueMetadataStorage) Delete(ctx context.Context, namespace xkube return fmt.Errorf("deleting secure value rowsAffected=%d error=%w", rowsAffected, err) } + s.metrics.SecureValueMetadataDeleteDuration.Observe(time.Since(start).Seconds()) + s.metrics.SecureValueMetadataDeleteCount.Inc() + return nil } func (s *secureValueMetadataStorage) List(ctx context.Context, namespace xkube.Namespace) (svList []secretv0alpha1.SecureValue, error error) { + start := time.Now() ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.List", trace.WithAttributes( attribute.String("namespace", namespace.String()), )) @@ -316,10 +343,14 @@ func (s *secureValueMetadataStorage) List(ctx context.Context, namespace xkube.N return nil, fmt.Errorf("read rows error: %w", err) } + s.metrics.SecureValueMetadataListDuration.Observe(time.Since(start).Seconds()) + s.metrics.SecureValueMetadataListCount.Inc() + return secureValues, nil } func (s *secureValueMetadataStorage) SetExternalID(ctx context.Context, namespace xkube.Namespace, name string, externalID contracts.ExternalID) error { + start := time.Now() ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.SetExternalID", trace.WithAttributes( attribute.String("name", name), attribute.String("namespace", namespace.String()), @@ -352,10 +383,13 @@ func (s *secureValueMetadataStorage) SetExternalID(ctx context.Context, namespac if modifiedCount > 1 { return fmt.Errorf("secureValueMetadataStorage.SetExternalID: modified more than one secret, this is a bug, check the where condition: modifiedCount=%d", modifiedCount) } + s.metrics.SecureValueSetExternalIDDuration.Observe(time.Since(start).Seconds()) + return nil } func (s *secureValueMetadataStorage) SetStatus(ctx context.Context, namespace xkube.Namespace, name string, status secretv0alpha1.SecureValueStatus) error { + start := time.Now() ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.SetStatus", trace.WithAttributes( attribute.String("name", name), attribute.String("namespace", namespace.String()), @@ -391,10 +425,13 @@ func (s *secureValueMetadataStorage) SetStatus(ctx context.Context, namespace xk if modifiedCount > 1 { return fmt.Errorf("secureValueMetadataStorage.SetExternalID: modified more than one secret, this is a bug, check the where condition: modifiedCount=%d", modifiedCount) } + s.metrics.SecureValueSetStatusDuration.Observe(time.Since(start).Seconds()) + return nil } func (s *secureValueMetadataStorage) ReadForDecrypt(ctx context.Context, namespace xkube.Namespace, name string) (*contracts.DecryptSecureValue, error) { + start := time.Now() ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.ReadForDecrypt", trace.WithAttributes( attribute.String("name", name), attribute.String("namespace", namespace.String()), @@ -437,6 +474,8 @@ func (s *secureValueMetadataStorage) ReadForDecrypt(ctx context.Context, namespa return nil, fmt.Errorf("convert to kubernetes object: %w", err) } + s.metrics.SecureValueGetForDecryptDuration.Observe(time.Since(start).Seconds()) + return secureValue, nil } diff --git a/pkg/storage/secret/metadata/secure_value_store_test.go b/pkg/storage/secret/metadata/secure_value_store_test.go index 6c710f0877d..b9ec0023430 100644 --- a/pkg/storage/secret/metadata/secure_value_store_test.go +++ b/pkg/storage/secret/metadata/secure_value_store_test.go @@ -43,11 +43,11 @@ func Test_SecureValueMetadataStorage_CreateAndRead(t *testing.T) { features := featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, featuremgmt.FlagSecretsManagementAppPlatform) // Initialize the secure value storage - secureValueStorage, err := ProvideSecureValueMetadataStorage(db, tracer, features) + secureValueStorage, err := ProvideSecureValueMetadataStorage(db, tracer, features, nil) require.NoError(t, err) // Initialize the keeper storage - keeperStorage, err := ProvideKeeperMetadataStorage(db, tracer, features) + keeperStorage, err := ProvideKeeperMetadataStorage(db, tracer, features, nil) require.NoError(t, err) t.Run("create and read a secure value", func(t *testing.T) { diff --git a/pkg/storage/secret/migrator/migrator.go b/pkg/storage/secret/migrator/migrator.go index d7d0bd6560b..f453d6dd3af 100644 --- a/pkg/storage/secret/migrator/migrator.go +++ b/pkg/storage/secret/migrator/migrator.go @@ -133,8 +133,8 @@ func (*SecretDB) AddMigration(mg *migrator.Migrator) { tables = append(tables, migrator.Table{ Name: TableNameSecureValueOutbox, Columns: []*migrator.Column{ - {Name: "request_id", Type: migrator.DB_NVarchar, Length: 253, Nullable: false}, - {Name: "id", Type: migrator.DB_BigInt, Length: 36, IsPrimaryKey: true, IsAutoIncrement: true}, // Fixed size of a UUID. + {Name: "request_id", Type: migrator.DB_NVarchar, Length: 1024, Nullable: false}, // Safer upper limit because we hex-encode traceparent+tracestate to form the request_id. + {Name: "id", Type: migrator.DB_BigInt, Length: 36, IsPrimaryKey: true, IsAutoIncrement: true}, {Name: "message_type", Type: migrator.DB_NVarchar, Length: 16, Nullable: false}, {Name: "name", Type: migrator.DB_NVarchar, Length: 253, Nullable: false}, // Limit enforced by K8s. {Name: "namespace", Type: migrator.DB_NVarchar, Length: 253, Nullable: false}, // Limit enforced by K8s.