From f32d944b23c3292414f564cbd56aac34756d8b6c Mon Sep 17 00:00:00 2001 From: Matheus Macabu Date: Wed, 2 Jul 2025 14:43:36 +0200 Subject: [PATCH] Secrets: Add initial tracing instrumentation (#107513) --- .../apis/secret/decrypt/authorizer.go | 21 +++++- .../apis/secret/decrypt/authorizer_test.go | 37 +++++----- .../encryption/cipher/service/service.go | 10 +-- .../encryption/cipher/service/service_test.go | 4 +- .../apis/secret/secretkeeper/secretkeeper.go | 5 +- .../secret/secretkeeper/secretkeeper_test.go | 6 +- .../secret/secretkeeper/sqlkeeper/keeper.go | 30 +++++--- .../secretkeeper/sqlkeeper/keeper_test.go | 6 +- pkg/storage/secret/database/database.go | 44 ++++++++---- .../secret/encryption/data_key_store.go | 44 ++++++++++-- .../secret/encryption/data_key_store_test.go | 4 +- .../encryption/encrypted_value_store.go | 37 +++++++++- .../encryption/encrypted_value_store_test.go | 6 +- pkg/storage/secret/metadata/keeper_store.go | 68 ++++++++++++++++++- .../secret/metadata/keeper_store_test.go | 6 +- pkg/storage/secret/metadata/outbox_store.go | 44 ++++++++++-- .../secret/metadata/outbox_store_test.go | 10 ++- .../secret/metadata/secure_value_store.go | 66 +++++++++++++++++- .../metadata/secure_value_store_test.go | 8 ++- 19 files changed, 376 insertions(+), 80 deletions(-) diff --git a/pkg/registry/apis/secret/decrypt/authorizer.go b/pkg/registry/apis/secret/decrypt/authorizer.go index de76ca759f6..6350e5f80af 100644 --- a/pkg/registry/apis/secret/decrypt/authorizer.go +++ b/pkg/registry/apis/secret/decrypt/authorizer.go @@ -6,6 +6,8 @@ import ( "github.com/grafana/authlib/authn" claims "github.com/grafana/authlib/types" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" secretv0alpha1 "github.com/grafana/grafana/pkg/apis/secret/v0alpha1" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" @@ -13,17 +15,32 @@ import ( // decryptAuthorizer is the authorizer implementation for decrypt operations. type decryptAuthorizer struct { + tracer trace.Tracer allowList contracts.DecryptAllowList } -func ProvideDecryptAuthorizer(allowList contracts.DecryptAllowList) contracts.DecryptAuthorizer { +func ProvideDecryptAuthorizer(tracer trace.Tracer, allowList contracts.DecryptAllowList) contracts.DecryptAuthorizer { return &decryptAuthorizer{ + tracer: tracer, allowList: allowList, } } // authorize checks whether the auth info token has the right permissions to decrypt the secure value. -func (a *decryptAuthorizer) Authorize(ctx context.Context, secureValueName string, secureValueDecrypters []string) (string, bool) { +func (a *decryptAuthorizer) Authorize(ctx context.Context, secureValueName string, secureValueDecrypters []string) (id string, isAllowed bool) { + ctx, span := a.tracer.Start(ctx, "DecryptAuthorizer.Authorize", trace.WithAttributes( + attribute.String("name", secureValueName), + attribute.StringSlice("decrypters", secureValueDecrypters), + )) + defer span.End() + + defer func() { + if id != "" { + span.SetAttributes(attribute.String("serviceIdentity", id)) + } + span.SetAttributes(attribute.Bool("allowed", isAllowed)) + }() + authInfo, ok := claims.AuthInfoFrom(ctx) if !ok { return "", false diff --git a/pkg/registry/apis/secret/decrypt/authorizer_test.go b/pkg/registry/apis/secret/decrypt/authorizer_test.go index bd808a1740c..3e4cd59ef31 100644 --- a/pkg/registry/apis/secret/decrypt/authorizer_test.go +++ b/pkg/registry/apis/secret/decrypt/authorizer_test.go @@ -7,14 +7,17 @@ import ( "github.com/grafana/authlib/authn" "github.com/grafana/authlib/types" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" "github.com/grafana/grafana/pkg/apimachinery/identity" ) func TestDecryptAuthorizer(t *testing.T) { + tracer := noop.NewTracerProvider().Tracer("test") + t.Run("when no auth info is present, it returns false", func(t *testing.T) { ctx := context.Background() - authorizer := ProvideDecryptAuthorizer(nil) + authorizer := ProvideDecryptAuthorizer(tracer, nil) identity, allowed := authorizer.Authorize(ctx, "", nil) require.Empty(t, identity) @@ -23,7 +26,7 @@ func TestDecryptAuthorizer(t *testing.T) { t.Run("when token permissions are empty, it returns false", func(t *testing.T) { ctx := createAuthContext(context.Background(), "identity", []string{}) - authorizer := ProvideDecryptAuthorizer(nil) + authorizer := ProvideDecryptAuthorizer(tracer, nil) identity, allowed := authorizer.Authorize(ctx, "", nil) require.NotEmpty(t, identity) @@ -32,7 +35,7 @@ func TestDecryptAuthorizer(t *testing.T) { t.Run("when service identity is empty, it returns false", func(t *testing.T) { ctx := createAuthContext(context.Background(), "", []string{}) - authorizer := ProvideDecryptAuthorizer(nil) + authorizer := ProvideDecryptAuthorizer(tracer, nil) identity, allowed := authorizer.Authorize(ctx, "", nil) require.Empty(t, identity) @@ -40,7 +43,7 @@ func TestDecryptAuthorizer(t *testing.T) { }) t.Run("when permission format is malformed (missing verb), it returns false", func(t *testing.T) { - authorizer := ProvideDecryptAuthorizer(nil) + authorizer := ProvideDecryptAuthorizer(tracer, nil) // nameless ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues"}) @@ -56,7 +59,7 @@ func TestDecryptAuthorizer(t *testing.T) { }) t.Run("when permission verb is not exactly `decrypt`, it returns false", func(t *testing.T) { - authorizer := ProvideDecryptAuthorizer(nil) + authorizer := ProvideDecryptAuthorizer(tracer, nil) // nameless ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues:*"}) @@ -73,7 +76,7 @@ func TestDecryptAuthorizer(t *testing.T) { t.Run("when permission does not have 2 or 3 parts, it returns false", func(t *testing.T) { ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app:decrypt"}) - authorizer := ProvideDecryptAuthorizer(nil) + authorizer := ProvideDecryptAuthorizer(tracer, nil) identity, allowed := authorizer.Authorize(ctx, "", nil) require.NotEmpty(t, identity) @@ -82,7 +85,7 @@ func TestDecryptAuthorizer(t *testing.T) { t.Run("when permission has group that is not `secret.grafana.app`, it returns false", func(t *testing.T) { ctx := createAuthContext(context.Background(), "identity", []string{"wrong.group/securevalues/invalid:decrypt"}) - authorizer := ProvideDecryptAuthorizer(nil) + authorizer := ProvideDecryptAuthorizer(tracer, nil) identity, allowed := authorizer.Authorize(ctx, "", nil) require.NotEmpty(t, identity) @@ -90,7 +93,7 @@ func TestDecryptAuthorizer(t *testing.T) { }) t.Run("when permission has resource that is not `securevalues`, it returns false", func(t *testing.T) { - authorizer := ProvideDecryptAuthorizer(nil) + authorizer := ProvideDecryptAuthorizer(tracer, nil) // nameless ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/invalid-resource:decrypt"}) @@ -107,7 +110,7 @@ func TestDecryptAuthorizer(t *testing.T) { t.Run("when the identity is not in the allow list, it returns false", func(t *testing.T) { ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues:decrypt"}) - authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"allowed1": {}}) + authorizer := ProvideDecryptAuthorizer(tracer, map[string]struct{}{"allowed1": {}}) identity, allowed := authorizer.Authorize(ctx, "", nil) require.NotEmpty(t, identity) @@ -115,7 +118,7 @@ func TestDecryptAuthorizer(t *testing.T) { }) t.Run("when the identity doesn't match any allowed decrypters, it returns false", func(t *testing.T) { - authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}}) + authorizer := ProvideDecryptAuthorizer(tracer, map[string]struct{}{"identity": {}}) // nameless ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues:decrypt"}) @@ -131,7 +134,7 @@ func TestDecryptAuthorizer(t *testing.T) { }) t.Run("when the identity matches an allowed decrypter, it returns true", func(t *testing.T) { - authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}}) + authorizer := ProvideDecryptAuthorizer(tracer, map[string]struct{}{"identity": {}}) // nameless ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues:decrypt"}) @@ -154,7 +157,7 @@ func TestDecryptAuthorizer(t *testing.T) { "wrong.group/securevalues/group2:decrypt", "secret.grafana.app/securevalues/identity:decrypt", // old style of identity+permission }) - authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}}) + authorizer := ProvideDecryptAuthorizer(tracer, map[string]struct{}{"identity": {}}) identity, allowed := authorizer.Authorize(ctx, "name1", []string{"identity"}) require.True(t, allowed) @@ -167,7 +170,7 @@ func TestDecryptAuthorizer(t *testing.T) { t.Run("when empty secure value name with specific permission, it returns false", func(t *testing.T) { ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues/name:decrypt"}) - authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}}) + authorizer := ProvideDecryptAuthorizer(tracer, map[string]struct{}{"identity": {}}) identity, allowed := authorizer.Authorize(ctx, "", []string{"identity"}) require.Equal(t, "identity", identity) @@ -176,7 +179,7 @@ func TestDecryptAuthorizer(t *testing.T) { t.Run("when permission has an extra / but no name, it returns false", func(t *testing.T) { ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues/:decrypt"}) - authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}}) + authorizer := ProvideDecryptAuthorizer(tracer, map[string]struct{}{"identity": {}}) identity, allowed := authorizer.Authorize(ctx, "", []string{"identity"}) require.Equal(t, "identity", identity) @@ -185,7 +188,7 @@ func TestDecryptAuthorizer(t *testing.T) { t.Run("when the decrypters list is empty, meaning nothing can decrypt the secure value, it returns false", func(t *testing.T) { ctx := createAuthContext(context.Background(), "identity", []string{"secret.grafana.app/securevalues:decrypt"}) - authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}}) + authorizer := ProvideDecryptAuthorizer(tracer, map[string]struct{}{"identity": {}}) identity, allowed := authorizer.Authorize(ctx, "name", []string{}) require.Equal(t, "identity", identity) @@ -194,7 +197,7 @@ func TestDecryptAuthorizer(t *testing.T) { t.Run("when one of decrypters matches the identity, it returns true", func(t *testing.T) { ctx := createAuthContext(context.Background(), "identity1", []string{"secret.grafana.app/securevalues:decrypt"}) - authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity1": {}, "identity2": {}}) + authorizer := ProvideDecryptAuthorizer(tracer, map[string]struct{}{"identity1": {}, "identity2": {}}) identity, allowed := authorizer.Authorize(ctx, "", []string{"identity1", "identity2", "identity3"}) require.Equal(t, "identity1", identity) @@ -202,7 +205,7 @@ func TestDecryptAuthorizer(t *testing.T) { }) t.Run("permissions must be case-sensitive and return false", func(t *testing.T) { - authorizer := ProvideDecryptAuthorizer(map[string]struct{}{"identity": {}}) + authorizer := ProvideDecryptAuthorizer(tracer, map[string]struct{}{"identity": {}}) ctx := createAuthContext(context.Background(), "identity", []string{"SECRET.grafana.app/securevalues:decrypt"}) identity, allowed := authorizer.Authorize(ctx, "", []string{"identity"}) diff --git a/pkg/registry/apis/secret/encryption/cipher/service/service.go b/pkg/registry/apis/secret/encryption/cipher/service/service.go index 4c3c0103769..9bdf0cc714f 100644 --- a/pkg/registry/apis/secret/encryption/cipher/service/service.go +++ b/pkg/registry/apis/secret/encryption/cipher/service/service.go @@ -7,9 +7,9 @@ import ( "fmt" "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/registry/apis/secret/encryption" "github.com/grafana/grafana/pkg/registry/apis/secret/encryption/cipher" @@ -24,7 +24,7 @@ const ( // Service must not be used for cipher. // Use secrets.Service implementing envelope encryption instead. type Service struct { - tracer tracing.Tracer + tracer trace.Tracer log log.Logger cfg *setting.Cfg @@ -35,7 +35,7 @@ type Service struct { } func NewEncryptionService( - tracer tracing.Tracer, + tracer trace.Tracer, usageMetrics usagestats.Service, cfg *setting.Cfg, ) (*Service, error) { @@ -101,7 +101,7 @@ func (s *Service) registerUsageMetrics() { } func (s *Service) Decrypt(ctx context.Context, payload []byte, secret string) ([]byte, error) { - ctx, span := s.tracer.Start(ctx, "cipher.service.Decrypt") + ctx, span := s.tracer.Start(ctx, "CipherService.Decrypt") defer span.End() var err error @@ -163,7 +163,7 @@ func (s *Service) deriveEncryptionAlgorithm(payload []byte) (string, []byte, err } func (s *Service) Encrypt(ctx context.Context, payload []byte, secret string) ([]byte, error) { - ctx, span := s.tracer.Start(ctx, "cipher.service.Encrypt") + ctx, span := s.tracer.Start(ctx, "CipherService.Encrypt") defer span.End() var err error diff --git a/pkg/registry/apis/secret/encryption/cipher/service/service_test.go b/pkg/registry/apis/secret/encryption/cipher/service/service_test.go index 462b1011508..7a207797ce8 100644 --- a/pkg/registry/apis/secret/encryption/cipher/service/service_test.go +++ b/pkg/registry/apis/secret/encryption/cipher/service/service_test.go @@ -6,8 +6,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" - "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/registry/apis/secret/encryption/cipher" "github.com/grafana/grafana/pkg/setting" @@ -29,7 +29,7 @@ func newGcmService(t *testing.T) *Service { }, } - svc, err := NewEncryptionService(tracing.InitializeTracerForTest(), usageStats, settings) + svc, err := NewEncryptionService(noop.NewTracerProvider().Tracer("test"), usageStats, settings) require.NoError(t, err, "failed to set up encryption service") return svc } diff --git a/pkg/registry/apis/secret/secretkeeper/secretkeeper.go b/pkg/registry/apis/secret/secretkeeper/secretkeeper.go index 45bf6633106..3a580728f50 100644 --- a/pkg/registry/apis/secret/secretkeeper/secretkeeper.go +++ b/pkg/registry/apis/secret/secretkeeper/secretkeeper.go @@ -1,8 +1,9 @@ package secretkeeper import ( + "go.opentelemetry.io/otel/trace" + secretv0alpha1 "github.com/grafana/grafana/pkg/apis/secret/v0alpha1" - "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/registry/apis/secret/secretkeeper/sqlkeeper" ) @@ -15,7 +16,7 @@ type OSSKeeperService struct { var _ contracts.KeeperService = (*OSSKeeperService)(nil) func ProvideService( - tracer tracing.Tracer, + tracer trace.Tracer, store contracts.EncryptedValueStorage, encryptionManager contracts.EncryptionManager, ) (*OSSKeeperService, error) { diff --git a/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go b/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go index aaa02e154e5..e48f8b95a93 100644 --- a/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go +++ b/pkg/registry/apis/secret/secretkeeper/secretkeeper_test.go @@ -5,8 +5,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" - "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry/apis/secret/secretkeeper/sqlkeeper" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/tests/testsuite" @@ -31,8 +31,10 @@ func Test_OSSKeeperService_GetKeepers(t *testing.T) { } func setupTestService(t *testing.T, cfg *setting.Cfg) (*OSSKeeperService, error) { + tracer := noop.NewTracerProvider().Tracer("test") + // Initialize the keeper service - keeperService, err := ProvideService(tracing.InitializeTracerForTest(), nil, nil) + keeperService, err := ProvideService(tracer, nil, 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 700abab63a5..04caae1237a 100644 --- a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go +++ b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper.go @@ -5,12 +5,13 @@ import ( "fmt" secretv0alpha1 "github.com/grafana/grafana/pkg/apis/secret/v0alpha1" - "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" ) type SQLKeeper struct { - tracer tracing.Tracer + tracer trace.Tracer encryptionManager contracts.EncryptionManager store contracts.EncryptedValueStorage } @@ -18,7 +19,7 @@ type SQLKeeper struct { var _ contracts.Keeper = (*SQLKeeper)(nil) func NewSQLKeeper( - tracer tracing.Tracer, + tracer trace.Tracer, encryptionManager contracts.EncryptionManager, store contracts.EncryptedValueStorage, ) *SQLKeeper { @@ -31,7 +32,7 @@ func NewSQLKeeper( // TODO: parameter cfg is not being used func (s *SQLKeeper) Store(ctx context.Context, _ secretv0alpha1.KeeperConfig, namespace string, exposedValueOrRef string) (contracts.ExternalID, error) { - ctx, span := s.tracer.Start(ctx, "sqlKeeper.Store") + ctx, span := s.tracer.Start(ctx, "SQLKeeper.Store", trace.WithAttributes(attribute.String("namespace", namespace))) defer span.End() encryptedData, err := s.encryptionManager.Encrypt(ctx, namespace, []byte(exposedValueOrRef)) @@ -44,11 +45,18 @@ func (s *SQLKeeper) Store(ctx context.Context, _ secretv0alpha1.KeeperConfig, na return "", fmt.Errorf("unable to store encrypted value: %w", err) } - return contracts.ExternalID(encryptedVal.UID), nil + externalID := contracts.ExternalID(encryptedVal.UID) + + span.SetAttributes(attribute.String("externalID", externalID.String())) + + return externalID, nil } func (s *SQLKeeper) Expose(ctx context.Context, cfg secretv0alpha1.KeeperConfig, namespace string, externalID contracts.ExternalID) (secretv0alpha1.ExposedSecureValue, error) { - ctx, span := s.tracer.Start(ctx, "sqlKeeper.Expose") + ctx, span := s.tracer.Start(ctx, "SQLKeeper.Expose", trace.WithAttributes( + attribute.String("namespace", namespace), + attribute.String("externalID", externalID.String()), + )) defer span.End() encryptedValue, err := s.store.Get(ctx, namespace, externalID.String()) @@ -66,7 +74,10 @@ func (s *SQLKeeper) Expose(ctx context.Context, cfg secretv0alpha1.KeeperConfig, } func (s *SQLKeeper) Delete(ctx context.Context, cfg secretv0alpha1.KeeperConfig, namespace string, externalID contracts.ExternalID) error { - ctx, span := s.tracer.Start(ctx, "sqlKeeper.Delete") + ctx, span := s.tracer.Start(ctx, "SQLKeeper.Delete", trace.WithAttributes( + attribute.String("namespace", namespace), + attribute.String("externalID", externalID.String()), + )) defer span.End() err := s.store.Delete(ctx, namespace, externalID.String()) @@ -77,7 +88,10 @@ func (s *SQLKeeper) Delete(ctx context.Context, cfg secretv0alpha1.KeeperConfig, } func (s *SQLKeeper) Update(ctx context.Context, cfg secretv0alpha1.KeeperConfig, namespace string, externalID contracts.ExternalID, exposedValueOrRef string) error { - ctx, span := s.tracer.Start(ctx, "sqlKeeper.Update") + ctx, span := s.tracer.Start(ctx, "SQLKeeper.Update", trace.WithAttributes( + attribute.String("namespace", namespace), + attribute.String("externalID", externalID.String()), + )) defer span.End() encryptedData, err := s.encryptionManager.Encrypt(ctx, namespace, []byte(exposedValueOrRef)) diff --git a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go index b8a78e92dab..585b124941f 100644 --- a/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go +++ b/pkg/registry/apis/secret/secretkeeper/sqlkeeper/keeper_test.go @@ -9,8 +9,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" - "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/setting" ) @@ -136,6 +136,8 @@ func Test_SQLKeeperSetup(t *testing.T) { } func setupTestService(t *testing.T, cfg *setting.Cfg) (*SQLKeeper, error) { + tracer := noop.NewTracerProvider().Tracer("test") + // Initialize the encryption manager with in-memory implementation encMgr := &inMemoryEncryptionManager{} @@ -143,7 +145,7 @@ func setupTestService(t *testing.T, cfg *setting.Cfg) (*SQLKeeper, error) { encValueStore := newInMemoryEncryptedValueStorage() // Initialize the SQLKeeper - sqlKeeper := NewSQLKeeper(tracing.InitializeTracerForTest(), encMgr, encValueStore) + sqlKeeper := NewSQLKeeper(tracer, encMgr, encValueStore) return sqlKeeper, nil } diff --git a/pkg/storage/secret/database/database.go b/pkg/storage/secret/database/database.go index 8997a24ee9e..ff990817e6a 100644 --- a/pkg/storage/secret/database/database.go +++ b/pkg/storage/secret/database/database.go @@ -6,6 +6,8 @@ import ( "errors" "github.com/jmoiron/sqlx" + "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/trace" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" @@ -19,6 +21,7 @@ type contextSessionTxKey struct{} type Database struct { dbType string sqlx *sqlx.DB + tracer trace.Tracer // Keep the xorm.Engine instance and its references alive until the apiserver is shut down. // This is only needed because the xorm.Engine calls a runtime.SetFinalizer, in a RAII-like pattern to close the DB, @@ -34,13 +37,14 @@ type Database struct { engine *xorm.Engine } -func ProvideDatabase(db db.DB) *Database { +func ProvideDatabase(db db.DB, tracer trace.Tracer) *Database { engine := db.GetEngine() return &Database{ dbType: string(db.GetDBType()), sqlx: sqlx.NewDb(engine.DB().DB, db.GetDialect().DriverName()), engine: engine, + tracer: tracer, } } @@ -48,25 +52,31 @@ func (db *Database) DriverName() string { return db.dbType } -func (db *Database) Transaction(ctx context.Context, callback func(context.Context) error) error { - txCtx := ctx - +func (db *Database) Transaction(ctx context.Context, callback func(context.Context) error) (err error) { // If another transaction is already open, we just use that one instead of nesting. - sqlxTx, ok := txCtx.Value(contextSessionTxKey{}).(*sqlx.Tx) + sqlxTx, ok := ctx.Value(contextSessionTxKey{}).(*sqlx.Tx) if sqlxTx != nil && ok { // We are already in a transaction, so we don't commit or rollback, let the outermost transaction do it. - return callback(txCtx) + return callback(ctx) } - tx, err := db.sqlx.Beginx() + spanCtx, span := db.tracer.Start(ctx, "Database.Transaction") + defer span.End() + + defer func() { + if err != nil { + span.SetStatus(codes.Error, "Transaction failed") + span.RecordError(err) + } + }() + + sqlxTx, err = db.sqlx.BeginTxx(spanCtx, nil) if err != nil { return err } - sqlxTx = tx - // Save it in the context so the transaction can be reused in case it is nested. - txCtx = context.WithValue(ctx, contextSessionTxKey{}, sqlxTx) + txCtx := context.WithValue(spanCtx, contextSessionTxKey{}, sqlxTx) if err := callback(txCtx); err != nil { if rbErr := sqlxTx.Rollback(); rbErr != nil { @@ -80,19 +90,25 @@ func (db *Database) Transaction(ctx context.Context, callback func(context.Conte } func (db *Database) ExecContext(ctx context.Context, query string, args ...any) (sql.Result, error) { + spanCtx, span := db.tracer.Start(ctx, "Database.ExecContext") + defer span.End() + // If another transaction is already open, we just use that one instead of nesting. if tx, ok := ctx.Value(contextSessionTxKey{}).(*sqlx.Tx); tx != nil && ok { - return tx.ExecContext(ctx, db.sqlx.Rebind(query), args...) + return tx.ExecContext(spanCtx, db.sqlx.Rebind(query), args...) } - return db.sqlx.ExecContext(ctx, db.sqlx.Rebind(query), args...) + return db.sqlx.ExecContext(spanCtx, db.sqlx.Rebind(query), args...) } func (db *Database) QueryContext(ctx context.Context, query string, args ...any) (contracts.Rows, error) { + spanCtx, span := db.tracer.Start(ctx, "Database.QueryContext") + defer span.End() + // If another transaction is already open, we just use that one instead of nesting. if tx, ok := ctx.Value(contextSessionTxKey{}).(*sqlx.Tx); tx != nil && ok { - return tx.QueryContext(ctx, db.sqlx.Rebind(query), args...) + return tx.QueryContext(spanCtx, db.sqlx.Rebind(query), args...) } - return db.sqlx.QueryContext(ctx, db.sqlx.Rebind(query), args...) + return db.sqlx.QueryContext(spanCtx, db.sqlx.Rebind(query), args...) } diff --git a/pkg/storage/secret/encryption/data_key_store.go b/pkg/storage/secret/encryption/data_key_store.go index f66d2d9f9b0..c38c4ce1153 100644 --- a/pkg/storage/secret/encryption/data_key_store.go +++ b/pkg/storage/secret/encryption/data_key_store.go @@ -9,19 +9,19 @@ 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" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" ) // encryptionStoreImpl is the actual implementation of the data key storage. type encryptionStoreImpl struct { db contracts.Database dialect sqltemplate.Dialect + tracer trace.Tracer log log.Logger } -func ProvideDataKeyStorage( - db contracts.Database, - features featuremgmt.FeatureToggles, -) (contracts.DataKeyStorage, error) { +func ProvideDataKeyStorage(db contracts.Database, tracer trace.Tracer, features featuremgmt.FeatureToggles) (contracts.DataKeyStorage, error) { if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) || !features.IsEnabledGlobally(featuremgmt.FlagSecretsManagementAppPlatform) { return &encryptionStoreImpl{}, nil @@ -30,6 +30,7 @@ func ProvideDataKeyStorage( store := &encryptionStoreImpl{ db: db, dialect: sqltemplate.DialectForDriver(db.DriverName()), + tracer: tracer, log: log.New("encryption.store"), } @@ -37,6 +38,12 @@ func ProvideDataKeyStorage( } func (ss *encryptionStoreImpl) GetDataKey(ctx context.Context, namespace, uid string) (*contracts.SecretDataKey, error) { + ctx, span := ss.tracer.Start(ctx, "DataKeyStorage.GetDataKey", trace.WithAttributes( + attribute.String("namespace", namespace), + attribute.String("uid", uid), + )) + defer span.End() + req := readDataKey{ SQLTemplate: sqltemplate.New(ss.dialect), Namespace: namespace, @@ -89,6 +96,12 @@ func (ss *encryptionStoreImpl) GetDataKey(ctx context.Context, namespace, uid st } func (ss *encryptionStoreImpl) GetCurrentDataKey(ctx context.Context, namespace, label string) (*contracts.SecretDataKey, error) { + ctx, span := ss.tracer.Start(ctx, "DataKeyStorage.GetCurrentDataKey", trace.WithAttributes( + attribute.String("namespace", namespace), + attribute.String("label", label), + )) + defer span.End() + req := readCurrentDataKey{ SQLTemplate: sqltemplate.New(ss.dialect), Namespace: namespace, @@ -141,6 +154,11 @@ func (ss *encryptionStoreImpl) GetCurrentDataKey(ctx context.Context, namespace, } func (ss *encryptionStoreImpl) GetAllDataKeys(ctx context.Context, namespace string) ([]*contracts.SecretDataKey, error) { + ctx, span := ss.tracer.Start(ctx, "DataKeyStorage.GetAllDataKeys", trace.WithAttributes( + attribute.String("namespace", namespace), + )) + defer span.End() + req := listDataKeys{ SQLTemplate: sqltemplate.New(ss.dialect), Namespace: namespace, @@ -193,6 +211,13 @@ func (ss *encryptionStoreImpl) GetAllDataKeys(ctx context.Context, namespace str } func (ss *encryptionStoreImpl) CreateDataKey(ctx context.Context, dataKey *contracts.SecretDataKey) error { + ctx, span := ss.tracer.Start(ctx, "DataKeyStorage.CreateDataKey", trace.WithAttributes( + attribute.String("uid", dataKey.UID), + attribute.String("namespace", dataKey.Namespace), + attribute.Bool("active", dataKey.Active), + )) + defer span.End() + if !dataKey.Active { return fmt.Errorf("cannot insert deactivated data keys") } @@ -228,6 +253,11 @@ func (ss *encryptionStoreImpl) CreateDataKey(ctx context.Context, dataKey *contr } func (ss *encryptionStoreImpl) DisableDataKeys(ctx context.Context, namespace string) error { + ctx, span := ss.tracer.Start(ctx, "DataKeyStorage.DisableDataKeys", trace.WithAttributes( + attribute.String("namespace", namespace), + )) + defer span.End() + req := disableDataKeys{ SQLTemplate: sqltemplate.New(ss.dialect), Namespace: namespace, @@ -257,6 +287,12 @@ func (ss *encryptionStoreImpl) DisableDataKeys(ctx context.Context, namespace st } func (ss *encryptionStoreImpl) DeleteDataKey(ctx context.Context, namespace, uid string) error { + ctx, span := ss.tracer.Start(ctx, "DataKeyStorage.DeleteDataKey", trace.WithAttributes( + attribute.String("uid", uid), + attribute.String("namespace", namespace), + )) + defer span.End() + if len(uid) == 0 { return fmt.Errorf("data key id is missing") } diff --git a/pkg/storage/secret/encryption/data_key_store_test.go b/pkg/storage/secret/encryption/data_key_store_test.go index e186d22468d..450577f4584 100644 --- a/pkg/storage/secret/encryption/data_key_store_test.go +++ b/pkg/storage/secret/encryption/data_key_store_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/registry/apis/secret/encryption" @@ -28,8 +29,9 @@ const ( func TestEncryptionStoreImpl_DataKeyLifecycle(t *testing.T) { // Initialize data key storage with a fake db 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), features) + store, err := ProvideDataKeyStorage(database.ProvideDatabase(testDB, tracer), tracer, features) require.NoError(t, err) ctx := context.Background() diff --git a/pkg/storage/secret/encryption/encrypted_value_store.go b/pkg/storage/secret/encryption/encrypted_value_store.go index 55ab5eb54ab..58bdd4054a2 100644 --- a/pkg/storage/secret/encryption/encrypted_value_store.go +++ b/pkg/storage/secret/encryption/encrypted_value_store.go @@ -7,6 +7,8 @@ import ( "time" "github.com/google/uuid" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" "github.com/grafana/grafana/pkg/registry/apis/secret/contracts" "github.com/grafana/grafana/pkg/services/featuremgmt" @@ -17,7 +19,7 @@ var ( ErrEncryptedValueNotFound = errors.New("encrypted value not found") ) -func ProvideEncryptedValueStorage(db contracts.Database, features featuremgmt.FeatureToggles) (contracts.EncryptedValueStorage, error) { +func ProvideEncryptedValueStorage(db contracts.Database, tracer trace.Tracer, features featuremgmt.FeatureToggles) (contracts.EncryptedValueStorage, error) { if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) || !features.IsEnabledGlobally(featuremgmt.FlagSecretsManagementAppPlatform) { return &encryptedValStorage{}, nil @@ -26,15 +28,28 @@ func ProvideEncryptedValueStorage(db contracts.Database, features featuremgmt.Fe return &encryptedValStorage{ db: db, dialect: sqltemplate.DialectForDriver(db.DriverName()), + tracer: tracer, }, nil } type encryptedValStorage struct { db contracts.Database dialect sqltemplate.Dialect + tracer trace.Tracer } -func (s *encryptedValStorage) Create(ctx context.Context, namespace string, encryptedData []byte) (*contracts.EncryptedValue, error) { +func (s *encryptedValStorage) Create(ctx context.Context, namespace string, encryptedData []byte) (ev *contracts.EncryptedValue, err error) { + ctx, span := s.tracer.Start(ctx, "EncryptedValueStorage.Create", trace.WithAttributes( + attribute.String("namespace", namespace), + )) + defer span.End() + + defer func() { + if ev != nil { + span.SetAttributes(attribute.String("uid", ev.UID)) + } + }() + createdTime := time.Now().Unix() encryptedValue := &EncryptedValue{ UID: uuid.New().String(), @@ -74,6 +89,12 @@ func (s *encryptedValStorage) Create(ctx context.Context, namespace string, encr } func (s *encryptedValStorage) Update(ctx context.Context, namespace string, uid string, encryptedData []byte) error { + ctx, span := s.tracer.Start(ctx, "EncryptedValueStorage.Update", trace.WithAttributes( + attribute.String("uid", uid), + attribute.String("namespace", namespace), + )) + defer span.End() + req := updateEncryptedValue{ SQLTemplate: sqltemplate.New(s.dialect), Namespace: namespace, @@ -102,6 +123,12 @@ func (s *encryptedValStorage) Update(ctx context.Context, namespace string, uid } func (s *encryptedValStorage) Get(ctx context.Context, namespace string, uid string) (*contracts.EncryptedValue, error) { + ctx, span := s.tracer.Start(ctx, "EncryptedValueStorage.Get", trace.WithAttributes( + attribute.String("uid", uid), + attribute.String("namespace", namespace), + )) + defer span.End() + req := &readEncryptedValue{ SQLTemplate: sqltemplate.New(s.dialect), Namespace: namespace, @@ -141,6 +168,12 @@ func (s *encryptedValStorage) Get(ctx context.Context, namespace string, uid str } func (s *encryptedValStorage) Delete(ctx context.Context, namespace string, uid string) error { + ctx, span := s.tracer.Start(ctx, "EncryptedValueStorage.Delete", trace.WithAttributes( + attribute.String("uid", uid), + attribute.String("namespace", namespace), + )) + defer span.End() + req := deleteEncryptedValue{ SQLTemplate: sqltemplate.New(s.dialect), Namespace: namespace, diff --git a/pkg/storage/secret/encryption/encrypted_value_store_test.go b/pkg/storage/secret/encryption/encrypted_value_store_test.go index 71b5c4c5ae5..92afe58e7e2 100644 --- a/pkg/storage/secret/encryption/encrypted_value_store_test.go +++ b/pkg/storage/secret/encryption/encrypted_value_store_test.go @@ -9,16 +9,18 @@ import ( "github.com/grafana/grafana/pkg/storage/secret/database" "github.com/grafana/grafana/pkg/storage/secret/migrator" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" ) func TestEncryptedValueStoreImpl(t *testing.T) { // Initialize data key storage with a fake db testDB := sqlstore.NewTestStore(t, sqlstore.WithMigrator(migrator.New())) - database := database.ProvideDatabase(testDB) + tracer := noop.NewTracerProvider().Tracer("test") + database := database.ProvideDatabase(testDB, tracer) features := featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, featuremgmt.FlagSecretsManagementAppPlatform) ctx := context.Background() - store, err := ProvideEncryptedValueStorage(database, features) + store, err := ProvideEncryptedValueStorage(database, tracer, features) require.NoError(t, err) t.Run("creating an encrypted value returns it", func(t *testing.T) { diff --git a/pkg/storage/secret/metadata/keeper_store.go b/pkg/storage/secret/metadata/keeper_store.go index 78458201e89..89b115eef2c 100644 --- a/pkg/storage/secret/metadata/keeper_store.go +++ b/pkg/storage/secret/metadata/keeper_store.go @@ -9,17 +9,21 @@ import ( "github.com/grafana/grafana/pkg/registry/apis/secret/xkube" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/trace" ) // keeperMetadataStorage is the actual implementation of the keeper metadata storage. type keeperMetadataStorage struct { db contracts.Database dialect sqltemplate.Dialect + tracer trace.Tracer } var _ contracts.KeeperMetadataStorage = (*keeperMetadataStorage)(nil) -func ProvideKeeperMetadataStorage(db contracts.Database, features featuremgmt.FeatureToggles) (contracts.KeeperMetadataStorage, error) { +func ProvideKeeperMetadataStorage(db contracts.Database, tracer trace.Tracer, features featuremgmt.FeatureToggles) (contracts.KeeperMetadataStorage, error) { if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) || !features.IsEnabledGlobally(featuremgmt.FlagSecretsManagementAppPlatform) { return &keeperMetadataStorage{}, nil @@ -28,10 +32,18 @@ func ProvideKeeperMetadataStorage(db contracts.Database, features featuremgmt.Fe return &keeperMetadataStorage{ db: db, dialect: sqltemplate.DialectForDriver(db.DriverName()), + tracer: tracer, }, nil } func (s *keeperMetadataStorage) Create(ctx context.Context, keeper *secretv0alpha1.Keeper, actorUID string) (*secretv0alpha1.Keeper, error) { + ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.Create", trace.WithAttributes( + attribute.String("name", keeper.GetName()), + attribute.String("namespace", keeper.GetNamespace()), + attribute.String("actorUID", actorUID), + )) + defer span.End() + row, err := toKeeperCreateRow(keeper, actorUID) if err != nil { return nil, fmt.Errorf("failed to create row: %w", err) @@ -82,6 +94,13 @@ func (s *keeperMetadataStorage) Create(ctx context.Context, keeper *secretv0alph } func (s *keeperMetadataStorage) Read(ctx context.Context, namespace xkube.Namespace, name string, opts contracts.ReadOpts) (*secretv0alpha1.Keeper, error) { + ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.Read", trace.WithAttributes( + attribute.String("name", name), + attribute.String("namespace", namespace.String()), + attribute.Bool("isForUpdate", opts.ForUpdate), + )) + defer span.End() + keeperDB, err := s.read(ctx, namespace.String(), name, opts) if err != nil { return nil, err @@ -134,6 +153,13 @@ 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) { + ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.Update", trace.WithAttributes( + attribute.String("name", newKeeper.GetName()), + attribute.String("namespace", newKeeper.GetNamespace()), + attribute.String("actorUID", actorUID), + )) + defer span.End() + var newRow *keeperDB err := s.db.Transaction(ctx, func(ctx context.Context) error { @@ -195,6 +221,12 @@ func (s *keeperMetadataStorage) Update(ctx context.Context, newKeeper *secretv0a } func (s *keeperMetadataStorage) Delete(ctx context.Context, namespace xkube.Namespace, name string) error { + ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.Delete", trace.WithAttributes( + attribute.String("name", name), + attribute.String("namespace", namespace.String()), + )) + defer span.End() + req := deleteKeeper{ SQLTemplate: sqltemplate.New(s.dialect), Namespace: namespace.String(), @@ -224,7 +256,16 @@ func (s *keeperMetadataStorage) Delete(ctx context.Context, namespace xkube.Name return nil } -func (s *keeperMetadataStorage) List(ctx context.Context, namespace xkube.Namespace) ([]secretv0alpha1.Keeper, error) { +func (s *keeperMetadataStorage) List(ctx context.Context, namespace xkube.Namespace) (keeperList []secretv0alpha1.Keeper, err error) { + ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.List", trace.WithAttributes( + attribute.String("namespace", namespace.String()), + )) + defer span.End() + + defer func() { + span.SetAttributes(attribute.Int("returnedList.count", len(keeperList))) + }() + req := listKeeper{ SQLTemplate: sqltemplate.New(s.dialect), Namespace: namespace.String(), @@ -270,7 +311,20 @@ func (s *keeperMetadataStorage) List(ctx context.Context, namespace xkube.Namesp // validateSecureValueReferences checks that all secure values referenced by the keeper exist and are not referenced by other third-party keepers. // It is used by other methods inside a transaction. -func (s *keeperMetadataStorage) validateSecureValueReferences(ctx context.Context, keeper *secretv0alpha1.Keeper) error { +func (s *keeperMetadataStorage) validateSecureValueReferences(ctx context.Context, keeper *secretv0alpha1.Keeper) (err error) { + ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.ValidateSecureValueReferences", trace.WithAttributes( + attribute.String("name", keeper.GetName()), + attribute.String("namespace", keeper.GetNamespace()), + )) + defer span.End() + + defer func() { + if err != nil { + span.SetStatus(codes.Error, "failed to validate secure value references") + span.RecordError(err) + } + }() + usedSecureValues := extractSecureValues(keeper) // No secure values are referenced, return early. @@ -405,11 +459,19 @@ func (s *keeperMetadataStorage) validateSecureValueReferences(ctx context.Contex } func (s *keeperMetadataStorage) GetKeeperConfig(ctx context.Context, namespace string, name *string, opts contracts.ReadOpts) (secretv0alpha1.KeeperConfig, error) { + ctx, span := s.tracer.Start(ctx, "KeeperMetadataStorage.GetKeeperConfig", trace.WithAttributes( + attribute.String("namespace", namespace), + attribute.Bool("isForUpdate", opts.ForUpdate), + )) + defer span.End() + // Check if keeper is the systemwide one. if name == nil { return nil, nil } + span.SetAttributes(attribute.String("name", *name)) + // Load keeper config from metadata store, or TODO: keeper cache. kp, err := s.read(ctx, namespace, *name, opts) if err != nil { diff --git a/pkg/storage/secret/metadata/keeper_store_test.go b/pkg/storage/secret/metadata/keeper_store_test.go index 698697cdbe9..a2165d5273f 100644 --- a/pkg/storage/secret/metadata/keeper_store_test.go +++ b/pkg/storage/secret/metadata/keeper_store_test.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/storage/secret/database" "github.com/grafana/grafana/pkg/storage/secret/migrator" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" ) func Test_KeeperMetadataStorage_GetKeeperConfig(t *testing.T) { @@ -334,11 +335,12 @@ func Test_KeeperMetadataStorage_GetKeeperConfig(t *testing.T) { func initStorage(t *testing.T) contracts.KeeperMetadataStorage { testDB := sqlstore.NewTestStore(t, sqlstore.WithMigrator(migrator.New())) - db := database.ProvideDatabase(testDB) + tracer := noop.NewTracerProvider().Tracer("test") + db := database.ProvideDatabase(testDB, tracer) features := featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, featuremgmt.FlagSecretsManagementAppPlatform) // Initialize the keeper storage - keeperMetadataStorage, err := ProvideKeeperMetadataStorage(db, features) + keeperMetadataStorage, err := ProvideKeeperMetadataStorage(db, tracer, features) require.NoError(t, err) return keeperMetadataStorage } diff --git a/pkg/storage/secret/metadata/outbox_store.go b/pkg/storage/secret/metadata/outbox_store.go index 81acd6343bf..f7a67e74c68 100644 --- a/pkg/storage/secret/metadata/outbox_store.go +++ b/pkg/storage/secret/metadata/outbox_store.go @@ -7,6 +7,9 @@ import ( "time" unifiedsql "github.com/grafana/grafana/pkg/storage/unified/sql" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/trace" "github.com/google/uuid" "github.com/grafana/grafana/pkg/registry/apis/secret/assert" @@ -17,12 +20,14 @@ import ( type outboxStore struct { db contracts.Database dialect sqltemplate.Dialect + tracer trace.Tracer } -func ProvideOutboxQueue(db contracts.Database) contracts.OutboxQueue { +func ProvideOutboxQueue(db contracts.Database, tracer trace.Tracer) contracts.OutboxQueue { return &outboxStore{ db: db, dialect: sqltemplate.DialectForDriver(db.DriverName()), + tracer: tracer, } } @@ -39,10 +44,29 @@ type outboxMessageDB struct { Created int64 } -func (s *outboxStore) Append(ctx context.Context, input contracts.AppendOutboxMessage) (string, error) { +func (s *outboxStore) Append(ctx context.Context, input contracts.AppendOutboxMessage) (messageID string, err error) { + ctx, span := s.tracer.Start(ctx, "outboxStore.Append", trace.WithAttributes( + attribute.String("name", input.Name), + attribute.String("namespace", input.Namespace), + attribute.String("type", string(input.Type)), + attribute.String("requestID", input.RequestID), + )) + defer span.End() + + defer func() { + if err != nil { + span.SetStatus(codes.Error, "failed to append outbox message") + span.RecordError(err) + } + + if messageID != "" { + span.SetAttributes(attribute.String("messageID", messageID)) + } + }() + assert.True(input.Type != "", "outboxStore.Append: outbox message type is required") - messageID, err := s.insertMessage(ctx, input) + messageID, err = s.insertMessage(ctx, input) if err != nil { return messageID, fmt.Errorf("inserting message into outbox table: %+w", err) } @@ -189,7 +213,19 @@ func (s *outboxStore) ReceiveN(ctx context.Context, n uint) ([]contracts.OutboxM return messages, nil } -func (s *outboxStore) Delete(ctx context.Context, messageID string) error { +func (s *outboxStore) Delete(ctx context.Context, messageID string) (err error) { + ctx, span := s.tracer.Start(ctx, "outboxStore.Append", trace.WithAttributes( + attribute.String("messageID", messageID), + )) + defer span.End() + + defer func() { + if err != nil { + span.SetStatus(codes.Error, "failed to delete message from outbox") + span.RecordError(err) + } + }() + assert.True(messageID != "", "outboxStore.Delete: messageID is required") if err := s.deleteMessage(ctx, messageID); err != nil { diff --git a/pkg/storage/secret/metadata/outbox_store_test.go b/pkg/storage/secret/metadata/outbox_store_test.go index 5bbf2fcea70..0dd6c024d98 100644 --- a/pkg/storage/secret/metadata/outbox_store_test.go +++ b/pkg/storage/secret/metadata/outbox_store_test.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/storage/secret/database" "github.com/grafana/grafana/pkg/storage/secret/migrator" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" ) type outboxStoreModel struct { @@ -110,10 +111,11 @@ func TestOutboxStoreSecureValueOperationInProgress(t *testing.T) { t.Parallel() testDB := sqlstore.NewTestStore(t, sqlstore.WithMigrator(migrator.New())) + tracer := noop.NewTracerProvider().Tracer("test") ctx := context.Background() - outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB)) + outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB, tracer), tracer) _, err := outbox.Append(ctx, contracts.AppendOutboxMessage{ RequestID: "1", @@ -142,10 +144,11 @@ func TestOutboxStoreSecureValueOperationInProgress(t *testing.T) { func TestOutboxStore(t *testing.T) { testDB := sqlstore.NewTestStore(t, sqlstore.WithMigrator(migrator.New())) + tracer := noop.NewTracerProvider().Tracer("test") ctx := context.Background() - outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB)) + outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB, tracer), tracer) m1 := contracts.AppendOutboxMessage{ Type: contracts.CreateSecretOutboxMessage, @@ -212,8 +215,9 @@ func TestOutboxStoreProperty(t *testing.T) { // The number of iterations was decided arbitrarily based on the time the test takes to run for range 10 { testDB := sqlstore.NewTestStore(t, sqlstore.WithMigrator(migrator.New())) + tracer := noop.NewTracerProvider().Tracer("test") - outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB)) + outbox := ProvideOutboxQueue(database.ProvideDatabase(testDB, tracer), tracer) model := newOutboxStoreModel() diff --git a/pkg/storage/secret/metadata/secure_value_store.go b/pkg/storage/secret/metadata/secure_value_store.go index eaa50571cec..39f42dcbdbe 100644 --- a/pkg/storage/secret/metadata/secure_value_store.go +++ b/pkg/storage/secret/metadata/secure_value_store.go @@ -10,11 +10,13 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/storage/unified/sql" "github.com/grafana/grafana/pkg/storage/unified/sql/sqltemplate" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" ) var _ contracts.SecureValueMetadataStorage = (*secureValueMetadataStorage)(nil) -func ProvideSecureValueMetadataStorage(db contracts.Database, features featuremgmt.FeatureToggles) (contracts.SecureValueMetadataStorage, error) { +func ProvideSecureValueMetadataStorage(db contracts.Database, tracer trace.Tracer, features featuremgmt.FeatureToggles) (contracts.SecureValueMetadataStorage, error) { if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) || !features.IsEnabledGlobally(featuremgmt.FlagSecretsManagementAppPlatform) { return &secureValueMetadataStorage{}, nil @@ -23,6 +25,7 @@ func ProvideSecureValueMetadataStorage(db contracts.Database, features featuremg return &secureValueMetadataStorage{ db: db, dialect: sqltemplate.DialectForDriver(db.DriverName()), + tracer: tracer, }, nil } @@ -30,9 +33,17 @@ func ProvideSecureValueMetadataStorage(db contracts.Database, features featuremg type secureValueMetadataStorage struct { db contracts.Database dialect sqltemplate.Dialect + tracer trace.Tracer } func (s *secureValueMetadataStorage) Create(ctx context.Context, sv *secretv0alpha1.SecureValue, actorUID string) (*secretv0alpha1.SecureValue, error) { + ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.Create", trace.WithAttributes( + attribute.String("name", sv.GetName()), + attribute.String("namespace", sv.GetNamespace()), + attribute.String("actorUID", actorUID), + )) + defer span.End() + sv.Status.Phase = secretv0alpha1.SecureValuePhasePending sv.Status.Message = "Creating secure value" @@ -110,6 +121,13 @@ func (s *secureValueMetadataStorage) Create(ctx context.Context, sv *secretv0alp } func (s *secureValueMetadataStorage) Read(ctx context.Context, namespace xkube.Namespace, name string, opts contracts.ReadOpts) (*secretv0alpha1.SecureValue, error) { + ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.Read", trace.WithAttributes( + attribute.String("name", name), + attribute.String("namespace", namespace.String()), + attribute.Bool("isForUpdate", opts.ForUpdate), + )) + defer span.End() + secureValue, err := s.read(ctx, namespace, name, opts) if err != nil { return nil, err @@ -124,6 +142,13 @@ func (s *secureValueMetadataStorage) Read(ctx context.Context, namespace xkube.N } func (s *secureValueMetadataStorage) Update(ctx context.Context, newSecureValue *secretv0alpha1.SecureValue, actorUID string) (*secretv0alpha1.SecureValue, error) { + ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.Update", trace.WithAttributes( + attribute.String("name", newSecureValue.GetName()), + attribute.String("namespace", newSecureValue.GetNamespace()), + attribute.String("actorUID", actorUID), + )) + defer span.End() + var newRow *secureValueDB err := s.db.Transaction(ctx, func(ctx context.Context) error { @@ -207,6 +232,12 @@ func (s *secureValueMetadataStorage) Update(ctx context.Context, newSecureValue } func (s *secureValueMetadataStorage) Delete(ctx context.Context, namespace xkube.Namespace, name string) error { + ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.Delete", trace.WithAttributes( + attribute.String("name", name), + attribute.String("namespace", namespace.String()), + )) + defer span.End() + req := deleteSecureValue{ SQLTemplate: sqltemplate.New(s.dialect), Namespace: namespace.String(), @@ -230,7 +261,16 @@ func (s *secureValueMetadataStorage) Delete(ctx context.Context, namespace xkube return nil } -func (s *secureValueMetadataStorage) List(ctx context.Context, namespace xkube.Namespace) ([]secretv0alpha1.SecureValue, error) { +func (s *secureValueMetadataStorage) List(ctx context.Context, namespace xkube.Namespace) (svList []secretv0alpha1.SecureValue, error error) { + ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.List", trace.WithAttributes( + attribute.String("namespace", namespace.String()), + )) + defer span.End() + + defer func() { + span.SetAttributes(attribute.Int("returnedList.count", len(svList))) + }() + req := listSecureValue{ SQLTemplate: sqltemplate.New(s.dialect), Namespace: namespace.String(), @@ -280,6 +320,13 @@ func (s *secureValueMetadataStorage) List(ctx context.Context, namespace xkube.N } func (s *secureValueMetadataStorage) SetExternalID(ctx context.Context, namespace xkube.Namespace, name string, externalID contracts.ExternalID) error { + ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.SetExternalID", trace.WithAttributes( + attribute.String("name", name), + attribute.String("namespace", namespace.String()), + attribute.String("externalID", externalID.String()), + )) + defer span.End() + req := updateExternalIdSecureValue{ SQLTemplate: sqltemplate.New(s.dialect), Namespace: namespace.String(), @@ -309,6 +356,15 @@ func (s *secureValueMetadataStorage) SetExternalID(ctx context.Context, namespac } func (s *secureValueMetadataStorage) SetStatus(ctx context.Context, namespace xkube.Namespace, name string, status secretv0alpha1.SecureValueStatus) error { + ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.SetStatus", trace.WithAttributes( + attribute.String("name", name), + attribute.String("namespace", namespace.String()), + attribute.String("status.phase", string(status.Phase)), + attribute.String("status.message", status.Message), + attribute.String("status.externalID", status.ExternalID), + )) + defer span.End() + req := updateStatusSecureValue{ SQLTemplate: sqltemplate.New(s.dialect), Namespace: namespace.String(), @@ -339,6 +395,12 @@ func (s *secureValueMetadataStorage) SetStatus(ctx context.Context, namespace xk } func (s *secureValueMetadataStorage) ReadForDecrypt(ctx context.Context, namespace xkube.Namespace, name string) (*contracts.DecryptSecureValue, error) { + ctx, span := s.tracer.Start(ctx, "SecureValueMetadataStorage.ReadForDecrypt", trace.WithAttributes( + attribute.String("name", name), + attribute.String("namespace", namespace.String()), + )) + defer span.End() + req := readSecureValueForDecrypt{ SQLTemplate: sqltemplate.New(s.dialect), Namespace: namespace.String(), diff --git a/pkg/storage/secret/metadata/secure_value_store_test.go b/pkg/storage/secret/metadata/secure_value_store_test.go index 01334ae4bd3..6c710f0877d 100644 --- a/pkg/storage/secret/metadata/secure_value_store_test.go +++ b/pkg/storage/secret/metadata/secure_value_store_test.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana/pkg/storage/secret/database" "github.com/grafana/grafana/pkg/storage/secret/migrator" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/trace/noop" ) func createTestKeeper(t *testing.T, ctx context.Context, keeperStorage contracts.KeeperMetadataStorage, name, namespace string) string { @@ -36,16 +37,17 @@ func createTestKeeper(t *testing.T, ctx context.Context, keeperStorage contracts func Test_SecureValueMetadataStorage_CreateAndRead(t *testing.T) { ctx := context.Background() testDB := sqlstore.NewTestStore(t, sqlstore.WithMigrator(migrator.New())) - db := database.ProvideDatabase(testDB) + tracer := noop.NewTracerProvider().Tracer("test") + db := database.ProvideDatabase(testDB, tracer) features := featuremgmt.WithFeatures(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs, featuremgmt.FlagSecretsManagementAppPlatform) // Initialize the secure value storage - secureValueStorage, err := ProvideSecureValueMetadataStorage(db, features) + secureValueStorage, err := ProvideSecureValueMetadataStorage(db, tracer, features) require.NoError(t, err) // Initialize the keeper storage - keeperStorage, err := ProvideKeeperMetadataStorage(db, features) + keeperStorage, err := ProvideKeeperMetadataStorage(db, tracer, features) require.NoError(t, err) t.Run("create and read a secure value", func(t *testing.T) {