Skip to content

Commit

Permalink
Alerting: update rule versions on folder move (grafana#88376)
Browse files Browse the repository at this point in the history
* Alerting: update rule versions on folder move (grafana#88361)
* Add tracing to folder.Move and folder.Update
  • Loading branch information
alexander-akhmetov authored Aug 13, 2024
1 parent 8044cb5 commit b2eeb0d
Show file tree
Hide file tree
Showing 21 changed files with 196 additions and 57 deletions.
2 changes: 1 addition & 1 deletion pkg/api/dashboard_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,7 @@ func getDashboardShouldReturn200WithConfig(t *testing.T, sc *scenarioContext, pr
dashboardPermissions := accesscontrolmock.NewMockedPermissionsService()

folderSvc := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()),
dashboardStore, folderStore, db.InitTestDB(t), features, supportbundlestest.NewFakeBundleService(), nil)
dashboardStore, folderStore, db.InitTestDB(t), features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())

if dashboardService == nil {
dashboardService, err = service.ProvideDashboardServiceImpl(
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/folder_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ func setupServer(b testing.TB, sc benchScenario, features featuremgmt.FeatureTog
folderStore := folderimpl.ProvideDashboardFolderStore(sc.db.DB())

ac := acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient())
folderServiceWithFlagOn := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore, folderStore, sc.db.DB(), features, supportbundlestest.NewFakeBundleService(), nil)
folderServiceWithFlagOn := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore, folderStore, sc.db.DB(), features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())

cfg := setting.NewCfg()
actionSets := resourcepermissions.NewActionSetService(features)
Expand Down
9 changes: 6 additions & 3 deletions pkg/events/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,13 @@ type DataSourceCreated struct {
OrgID int64 `json:"org_id"`
}

type FolderTitleUpdated struct {
// FolderFullPathUpdated is emitted when the full path of the folder(s) is updated.
// For example, when the folder is renamed or moved to another folder.
// It does not contain the full path of the folders because calculating
// it requires more resources and not needed in the event at the moment.
type FolderFullPathUpdated struct {
Timestamp time.Time `json:"timestamp"`
Title string `json:"name"`
ID int64 `json:"id"`
UID string `json:"uid"`
UIDs []string `json:"uids"`
OrgID int64 `json:"org_id"`
}
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func TestIntegrationAnnotationListingWithInheritedRBAC(t *testing.T) {
})

ac := acimpl.ProvideAccessControl(features, zanzana.NewNoopClient())
folderSvc := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore, folderimpl.ProvideDashboardFolderStore(sql), sql, features, supportbundlestest.NewFakeBundleService(), nil)
folderSvc := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashStore, folderimpl.ProvideDashboardFolderStore(sql), sql, features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())

cfg.AnnotationMaximumTagsLength = 60

Expand Down
2 changes: 1 addition & 1 deletion pkg/services/dashboards/database/database_folder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ func TestIntegrationDashboardInheritedFolderRBAC(t *testing.T) {
guardian.New = origNewGuardian
})

folderSvc := folderimpl.ProvideService(mock.New(), bus.ProvideBus(tracer), dashboardWriteStore, folderimpl.ProvideDashboardFolderStore(sqlStore.DB()), sqlStore.DB(), features, supportbundlestest.NewFakeBundleService(), nil)
folderSvc := folderimpl.ProvideService(mock.New(), bus.ProvideBus(tracer), dashboardWriteStore, folderimpl.ProvideDashboardFolderStore(sqlStore.DB()), sqlStore.DB(), features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())

parentUID := ""
for i := 0; ; i++ {
Expand Down
4 changes: 2 additions & 2 deletions pkg/services/dashboards/database/database_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ func TestIntegrationFindDashboardsByTitle(t *testing.T) {

ac := acimpl.ProvideAccessControl(features, zanzana.NewNoopClient())
folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
folderServiceWithFlagOn := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), nil)
folderServiceWithFlagOn := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())

user := &user.SignedInUser{
OrgID: 1,
Expand Down Expand Up @@ -948,7 +948,7 @@ func TestIntegrationFindDashboardsByFolder(t *testing.T) {

ac := acimpl.ProvideAccessControl(features, zanzana.NewNoopClient())
folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
folderServiceWithFlagOn := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), nil)
folderServiceWithFlagOn := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())

user := &user.SignedInUser{
OrgID: 1,
Expand Down
59 changes: 50 additions & 9 deletions pkg/services/folder/folderimpl/folder.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ import (

"github.com/grafana/dskit/concurrency"
"github.com/prometheus/client_golang/prometheus"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
"golang.org/x/exp/slices"

"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/bus"
"github.com/grafana/grafana/pkg/events"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/infra/metrics"
"github.com/grafana/grafana/pkg/infra/tracing"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
Expand All @@ -44,12 +47,14 @@ type Service struct {
dashboardFolderStore folder.FolderStore
features featuremgmt.FeatureToggles
accessControl accesscontrol.AccessControl
// bus is currently used to publish event in case of title change
// bus is currently used to publish event in case of folder full path change.
// For example when a folder is moved to another folder or when a folder is renamed.
bus bus.Bus

mutex sync.RWMutex
registry map[string]folder.RegistryService
metrics *foldersMetrics
tracer tracing.Tracer
}

func ProvideService(
Expand All @@ -61,6 +66,7 @@ func ProvideService(
features featuremgmt.FeatureToggles,
supportBundles supportbundles.Service,
r prometheus.Registerer,
tracer tracing.Tracer,
) folder.Service {
store := ProvideStore(db)
srv := &Service{
Expand All @@ -74,6 +80,7 @@ func ProvideService(
db: db,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(r),
tracer: tracer,
}
srv.DBMigration(db)

Expand Down Expand Up @@ -655,6 +662,9 @@ func (s *Service) Create(ctx context.Context, cmd *folder.CreateFolderCommand) (
}

func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (*folder.Folder, error) {
ctx, span := s.tracer.Start(ctx, "folder.Update")
defer span.End()

if cmd.SignedInUser == nil {
return nil, folder.ErrBadRequest.Errorf("missing signed in user")
}
Expand All @@ -679,14 +689,8 @@ func (s *Service) Update(ctx context.Context, cmd *folder.UpdateFolderCommand) (

if cmd.NewTitle != nil {
metrics.MFolderIDsServiceCount.WithLabelValues(metrics.Folder).Inc()
if err := s.bus.Publish(ctx, &events.FolderTitleUpdated{
Timestamp: foldr.Updated,
Title: foldr.Title,
ID: dashFolder.ID, // nolint:staticcheck
UID: dashFolder.UID,
OrgID: cmd.OrgID,
}); err != nil {
s.log.ErrorContext(ctx, "failed to publish FolderTitleUpdated event", "folder", foldr.Title, "user", cmd.SignedInUser.GetID(), "error", err)

if err := s.publishFolderFullPathUpdatedEvent(ctx, foldr.Updated, cmd.OrgID, cmd.UID); err != nil {
return err
}
}
Expand Down Expand Up @@ -873,6 +877,9 @@ func (s *Service) legacyDelete(ctx context.Context, cmd *folder.DeleteFolderComm
}

func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*folder.Folder, error) {
ctx, span := s.tracer.Start(ctx, "folder.Move")
defer span.End()

if cmd.SignedInUser == nil {
return nil, folder.ErrBadRequest.Errorf("missing signed in user")
}
Expand Down Expand Up @@ -947,13 +954,47 @@ func (s *Service) Move(ctx context.Context, cmd *folder.MoveFolderCommand) (*fol
return folder.ErrInternal.Errorf("failed to move legacy folder: %w", err)
}

if err := s.publishFolderFullPathUpdatedEvent(ctx, f.Updated, cmd.OrgID, cmd.UID); err != nil {
return err
}

return nil
}); err != nil {
return nil, err
}
return f, nil
}

func (s *Service) publishFolderFullPathUpdatedEvent(ctx context.Context, timestamp time.Time, orgID int64, folderUID string) error {
ctx, span := s.tracer.Start(ctx, "folder.publishFolderFullPathUpdatedEvent")
defer span.End()

descFolders, err := s.store.GetDescendants(ctx, orgID, folderUID)
if err != nil {
s.log.ErrorContext(ctx, "Failed to get descendants of the folder", "folderUID", folderUID, "orgID", orgID, "error", err)
return err
}
uids := make([]string, 0, len(descFolders)+1)
uids = append(uids, folderUID)
for _, f := range descFolders {
uids = append(uids, f.UID)
}
span.AddEvent("found folder descendants", trace.WithAttributes(
attribute.Int64("folders", int64(len(uids))),
))

if err := s.bus.Publish(ctx, &events.FolderFullPathUpdated{
Timestamp: timestamp,
UIDs: uids,
OrgID: orgID,
}); err != nil {
s.log.ErrorContext(ctx, "Failed to publish FolderFullPathUpdated event", "folderUID", folderUID, "orgID", orgID, "descendantsUIDs", uids, "error", err)
return err
}

return nil
}

func (s *Service) canMove(ctx context.Context, cmd *folder.MoveFolderCommand) (bool, error) {
// Check that the user is allowed to move the folder to the destination folder
var evaluator accesscontrol.Evaluator
Expand Down
12 changes: 11 additions & 1 deletion pkg/services/folder/folderimpl/folder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestIntegrationProvideFolderService(t *testing.T) {
t.Run("should register scope resolvers", func(t *testing.T) {
ac := acmock.New()
db := db.InitTestDB(t)
ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), nil, nil, db, featuremgmt.WithFeatures(), supportbundlestest.NewFakeBundleService(), nil)
ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), nil, nil, db, featuremgmt.WithFeatures(), supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())

require.Len(t, ac.Calls.RegisterAttributeScopeResolver, 3)
})
Expand Down Expand Up @@ -100,6 +100,7 @@ func TestIntegrationFolderService(t *testing.T) {
accessControl: acimpl.ProvideAccessControl(features, zanzana.NewNoopClient()),
metrics: newFoldersMetrics(nil),
registry: make(map[string]folder.RegistryService),
tracer: tracing.InitializeTracerForTest(),
}

require.NoError(t, service.RegisterService(alertingStore))
Expand Down Expand Up @@ -440,6 +441,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
accessControl: ac,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
}

signedInUser := user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{
Expand Down Expand Up @@ -553,6 +555,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
db: db,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
}

origNewGuardian := guardian.New
Expand Down Expand Up @@ -630,6 +633,7 @@ func TestIntegrationNestedFolderService(t *testing.T) {
db: db,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
}

testCases := []struct {
Expand Down Expand Up @@ -805,6 +809,7 @@ func TestNestedFolderServiceFeatureToggle(t *testing.T) {
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
accessControl: acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
}
t.Run("create folder", func(t *testing.T) {
nestedFolderStore.ExpectedFolder = &folder.Folder{ParentUID: util.GenerateShortUID()}
Expand Down Expand Up @@ -841,6 +846,7 @@ func TestFolderServiceDualWrite(t *testing.T) {
features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
accessControl: acimpl.ProvideAccessControl(featuremgmt.WithFeatures(), zanzana.NewNoopClient()),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
bus: bus.ProvideBus(tracing.InitializeTracerForTest()),
}

Expand Down Expand Up @@ -1475,6 +1481,7 @@ func TestIntegrationNestedFolderSharedWithMe(t *testing.T) {
accessControl: ac,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
}

dashboardPermissions := acmock.NewMockedPermissionsService()
Expand Down Expand Up @@ -1977,6 +1984,7 @@ func TestFolderServiceGetFolders(t *testing.T) {
accessControl: ac,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
}

signedInAdminUser := user.SignedInUser{UserID: 1, OrgID: orgID, Permissions: map[int64]map[string][]string{
Expand Down Expand Up @@ -2063,6 +2071,7 @@ func TestGetChildrenFilterByPermission(t *testing.T) {
accessControl: ac,
registry: make(map[string]folder.RegistryService),
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
}

origGuardian := guardian.New
Expand Down Expand Up @@ -2523,6 +2532,7 @@ func setup(t *testing.T, dashStore dashboards.Store, dashboardFolderStore folder
accessControl: ac,
db: db,
metrics: newFoldersMetrics(nil),
tracer: tracing.InitializeTracerForTest(),
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/services/libraryelements/libraryelements_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ func createFolder(t *testing.T, sc scenarioContext, title string) *folder.Folder
require.NoError(t, err)

folderStore := folderimpl.ProvideDashboardFolderStore(sc.sqlStore)
s := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sc.sqlStore, features, supportbundlestest.NewFakeBundleService(), nil)
s := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sc.sqlStore, features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())
t.Logf("Creating folder with title and UID %q", title)
ctx := identity.WithRequester(context.Background(), &sc.user)
folder, err := s.Create(ctx, &folder.CreateFolderCommand{
Expand Down Expand Up @@ -463,7 +463,7 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo
Cfg: cfg,
features: featuremgmt.WithFeatures(),
SQLStore: sqlStore,
folderService: folderimpl.ProvideService(ac, bus.ProvideBus(tracer), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), nil),
folderService: folderimpl.ProvideService(ac, bus.ProvideBus(tracer), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest()),
}

// deliberate difference between signed in user and user in db to make it crystal clear
Expand Down
4 changes: 2 additions & 2 deletions pkg/services/librarypanels/librarypanels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,7 @@ func createFolder(t *testing.T, sc scenarioContext, title string) *folder.Folder
dashboardStore, err := database.ProvideDashboardStore(sc.replStore, cfg, features, tagimpl.ProvideService(sc.sqlStore), quotaService)
require.NoError(t, err)
folderStore := folderimpl.ProvideDashboardFolderStore(sc.sqlStore)
s := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sc.sqlStore, features, supportbundlestest.NewFakeBundleService(), nil)
s := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sc.sqlStore, features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())

t.Logf("Creating folder with title and UID %q", title)
ctx := identity.WithRequester(context.Background(), sc.user)
Expand Down Expand Up @@ -836,7 +836,7 @@ func testScenario(t *testing.T, desc string, fn func(t *testing.T, sc scenarioCo
dashboardStore, err := database.ProvideDashboardStore(replStore, cfg, featuremgmt.WithFeatures(), tagimpl.ProvideService(sqlStore), quotaService)
require.NoError(t, err)
features := featuremgmt.WithFeatures()
folderService := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), nil)
folderService := folderimpl.ProvideService(ac, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sqlStore, features, supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())

elementService := libraryelements.ProvideService(cfg, sqlStore, routing.NewRouteRegister(), folderService, featuremgmt.WithFeatures(), ac)
service := LibraryPanelService{
Expand Down
2 changes: 1 addition & 1 deletion pkg/services/ngalert/api/api_provisioning_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1819,7 +1819,7 @@ func createTestEnv(t *testing.T, testConfig string) testEnvironment {
require.NoError(t, err)

folderStore := folderimpl.ProvideDashboardFolderStore(sqlStore)
folderService := folderimpl.ProvideService(actest.FakeAccessControl{ExpectedEvaluate: true}, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sqlStore, featuremgmt.WithFeatures(), supportbundlestest.NewFakeBundleService(), nil)
folderService := folderimpl.ProvideService(actest.FakeAccessControl{ExpectedEvaluate: true}, bus.ProvideBus(tracing.InitializeTracerForTest()), dashboardStore, folderStore, sqlStore, featuremgmt.WithFeatures(), supportbundlestest.NewFakeBundleService(), nil, tracing.InitializeTracerForTest())
store := store.DBstore{
Logger: log,
SQLStore: sqlStore,
Expand Down
4 changes: 2 additions & 2 deletions pkg/services/ngalert/api/persist.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ type RuleStore interface {
UpdateAlertRules(ctx context.Context, rule []ngmodels.UpdateRule) error
DeleteAlertRulesByUID(ctx context.Context, orgID int64, ruleUID ...string) error

// IncreaseVersionForAllRulesInNamespace Increases version for all rules that have specified namespace. Returns all rules that belong to the namespace
IncreaseVersionForAllRulesInNamespace(ctx context.Context, orgID int64, namespaceUID string) ([]ngmodels.AlertRuleKeyWithVersion, error)
// IncreaseVersionForAllRulesInNamespaces Increases version for all rules that have specified namespace uids
IncreaseVersionForAllRulesInNamespaces(ctx context.Context, orgID int64, namespaceUIDs []string) ([]ngmodels.AlertRuleKeyWithVersion, error)

accesscontrol.RuleUIDToNamespaceStore
}
Loading

0 comments on commit b2eeb0d

Please sign in to comment.