From decb6609925f153f216d0c7852585dfe2ab0cc86 Mon Sep 17 00:00:00 2001 From: Vikrant Gupta Date: Mon, 9 Jun 2025 15:46:22 +0530 Subject: [PATCH] chore(sqlmigration): drop the rule history and data migrations table (#8181) --- .../src/pages/AlertDetails/AlertDetails.tsx | 28 -------- pkg/query-service/app/http_handler.go | 52 +++++++++++---- pkg/query-service/rules/manager.go | 60 ++++------------- pkg/ruler/rulestore/sqlrulestore/rule.go | 14 ---- pkg/signoz/provider.go | 1 + .../040_drop_deprecated_tables.go | 66 +++++++++++++++++++ pkg/types/datamigration.go | 15 ----- pkg/types/ruletypes/rule.go | 7 -- 8 files changed, 120 insertions(+), 123 deletions(-) create mode 100644 pkg/sqlmigration/040_drop_deprecated_tables.go delete mode 100644 pkg/types/datamigration.go diff --git a/frontend/src/pages/AlertDetails/AlertDetails.tsx b/frontend/src/pages/AlertDetails/AlertDetails.tsx index 8b0c850dd495..29bb8df2f904 100644 --- a/frontend/src/pages/AlertDetails/AlertDetails.tsx +++ b/frontend/src/pages/AlertDetails/AlertDetails.tsx @@ -6,11 +6,7 @@ import { Filters } from 'components/AlertDetailsFilters/Filters'; import NotFound from 'components/NotFound'; import RouteTab from 'components/RouteTab'; import Spinner from 'components/Spinner'; -import { QueryParams } from 'constants/query'; import ROUTES from 'constants/routes'; -import { useNotifications } from 'hooks/useNotifications'; -import { useSafeNavigate } from 'hooks/useSafeNavigate'; -import useUrlQuery from 'hooks/useUrlQuery'; import history from 'lib/history'; import { useEffect, useMemo } from 'react'; import { useTranslation } from 'react-i18next'; @@ -74,9 +70,6 @@ BreadCrumbItem.defaultProps = { function AlertDetails(): JSX.Element { const { pathname } = useLocation(); const { routes } = useRouteTabUtils(); - const urlQuery = useUrlQuery(); - const { safeNavigate } = useSafeNavigate(); - const { notifications } = useNotifications(); const { isLoading, @@ -92,27 +85,6 @@ function AlertDetails(): JSX.Element { document.title = alertTitle || document.title; }, [alertDetailsResponse?.payload?.data.alert, isRefetching]); - useEffect(() => { - if (alertDetailsResponse?.payload?.data?.id) { - const ruleUUID = alertDetailsResponse.payload.data.id; - if (ruleId !== ruleUUID) { - urlQuery.set(QueryParams.ruleId, ruleUUID); - const generatedUrl = `${window.location.pathname}?${urlQuery}`; - notifications.info({ - message: - "We're transitioning alert rule IDs from integers to UUIDs.Both old and new alert links will continue to work after this change - existing notifications using integer IDs will remain functional while new alerts will use the UUID format. Please use the updated link in the URL for future references", - }); - safeNavigate(generatedUrl); - } - } - }, [ - alertDetailsResponse?.payload?.data.id, - notifications, - ruleId, - safeNavigate, - urlQuery, - ]); - if ( isError || !isValidRuleId || diff --git a/pkg/query-service/app/http_handler.go b/pkg/query-service/app/http_handler.go index f70d45c3c066..a3458f4a56f3 100644 --- a/pkg/query-service/app/http_handler.go +++ b/pkg/query-service/app/http_handler.go @@ -660,7 +660,13 @@ func Intersection(a, b []int) (c []int) { } func (aH *APIHandler) getRule(w http.ResponseWriter, r *http.Request) { - id := mux.Vars(r)["id"] + idStr := mux.Vars(r)["id"] + id, err := valuer.NewUUID(idStr) + if err != nil { + RespondError(w, &model.ApiError{Typ: model.ErrorBadData, Err: err}, nil) + return + } + ruleResponse, err := aH.ruleManager.GetRule(r.Context(), id) if err != nil { RespondError(w, &model.ApiError{Typ: model.ErrorInternal, Err: err}, nil) @@ -990,9 +996,15 @@ func (aH *APIHandler) metaForLinks(ctx context.Context, rule *ruletypes.Gettable } func (aH *APIHandler) getRuleStateHistory(w http.ResponseWriter, r *http.Request) { - ruleID := mux.Vars(r)["id"] + idStr := mux.Vars(r)["id"] + id, err := valuer.NewUUID(idStr) + if err != nil { + RespondError(w, &model.ApiError{Typ: model.ErrorBadData, Err: err}, nil) + return + } + params := model.QueryRuleStateHistory{} - err := json.NewDecoder(r.Body).Decode(¶ms) + err = json.NewDecoder(r.Body).Decode(¶ms) if err != nil { RespondError(w, &model.ApiError{Typ: model.ErrorBadData, Err: err}, nil) return @@ -1002,13 +1014,13 @@ func (aH *APIHandler) getRuleStateHistory(w http.ResponseWriter, r *http.Request return } - res, err := aH.reader.ReadRuleStateHistoryByRuleID(r.Context(), ruleID, ¶ms) + res, err := aH.reader.ReadRuleStateHistoryByRuleID(r.Context(), id.StringValue(), ¶ms) if err != nil { RespondError(w, &model.ApiError{Typ: model.ErrorInternal, Err: err}, nil) return } - rule, err := aH.ruleManager.GetRule(r.Context(), ruleID) + rule, err := aH.ruleManager.GetRule(r.Context(), id) if err == nil { for idx := range res.Items { lbls := make(map[string]string) @@ -1036,21 +1048,27 @@ func (aH *APIHandler) getRuleStateHistory(w http.ResponseWriter, r *http.Request } func (aH *APIHandler) getRuleStateHistoryTopContributors(w http.ResponseWriter, r *http.Request) { - ruleID := mux.Vars(r)["id"] - params := model.QueryRuleStateHistory{} - err := json.NewDecoder(r.Body).Decode(¶ms) + idStr := mux.Vars(r)["id"] + id, err := valuer.NewUUID(idStr) if err != nil { RespondError(w, &model.ApiError{Typ: model.ErrorBadData, Err: err}, nil) return } - res, err := aH.reader.ReadRuleStateHistoryTopContributorsByRuleID(r.Context(), ruleID, ¶ms) + params := model.QueryRuleStateHistory{} + err = json.NewDecoder(r.Body).Decode(¶ms) + if err != nil { + RespondError(w, &model.ApiError{Typ: model.ErrorBadData, Err: err}, nil) + return + } + + res, err := aH.reader.ReadRuleStateHistoryTopContributorsByRuleID(r.Context(), id.StringValue(), ¶ms) if err != nil { RespondError(w, &model.ApiError{Typ: model.ErrorInternal, Err: err}, nil) return } - rule, err := aH.ruleManager.GetRule(r.Context(), ruleID) + rule, err := aH.ruleManager.GetRule(r.Context(), id) if err == nil { for idx := range res { lbls := make(map[string]string) @@ -1330,7 +1348,12 @@ func (aH *APIHandler) deleteRule(w http.ResponseWriter, r *http.Request) { // patchRule updates only requested changes in the rule func (aH *APIHandler) patchRule(w http.ResponseWriter, r *http.Request) { - id := mux.Vars(r)["id"] + idStr := mux.Vars(r)["id"] + id, err := valuer.NewUUID(idStr) + if err != nil { + RespondError(w, &model.ApiError{Typ: model.ErrorBadData, Err: err}, nil) + return + } defer r.Body.Close() body, err := io.ReadAll(r.Body) @@ -1351,7 +1374,12 @@ func (aH *APIHandler) patchRule(w http.ResponseWriter, r *http.Request) { } func (aH *APIHandler) editRule(w http.ResponseWriter, r *http.Request) { - id := mux.Vars(r)["id"] + idStr := mux.Vars(r)["id"] + id, err := valuer.NewUUID(idStr) + if err != nil { + RespondError(w, &model.ApiError{Typ: model.ErrorBadData, Err: err}, nil) + return + } defer r.Body.Close() body, err := io.ReadAll(r.Body) diff --git a/pkg/query-service/rules/manager.go b/pkg/query-service/rules/manager.go index b7f3f20fb1c7..a827e49d0771 100644 --- a/pkg/query-service/rules/manager.go +++ b/pkg/query-service/rules/manager.go @@ -5,7 +5,6 @@ import ( "encoding/json" "fmt" "sort" - "strconv" "strings" "sync" "time" @@ -320,7 +319,7 @@ func (m *Manager) Stop(ctx context.Context) { // EditRuleDefinition writes the rule definition to the // datastore and also updates the rule executor -func (m *Manager) EditRule(ctx context.Context, ruleStr string, idStr string) error { +func (m *Manager) EditRule(ctx context.Context, ruleStr string, id valuer.UUID) error { claims, err := authtypes.ClaimsFromContext(ctx) if err != nil { return err @@ -330,26 +329,12 @@ func (m *Manager) EditRule(ctx context.Context, ruleStr string, idStr string) er return err } - ruleUUID, err := valuer.NewUUID(idStr) - if err != nil { - id, err := strconv.Atoi(idStr) - if err != nil { - return err - } - ruleHistory, err := m.ruleStore.GetRuleUUID(ctx, id) - if err != nil { - return err - } - - ruleUUID = ruleHistory.RuleUUID - } - parsedRule, err := ruletypes.ParsePostableRule([]byte(ruleStr)) if err != nil { return err } - existingRule, err := m.ruleStore.GetStoredRule(ctx, ruleUUID) + existingRule, err := m.ruleStore.GetStoredRule(ctx, id) if err != nil { return err } @@ -378,7 +363,7 @@ func (m *Manager) EditRule(ctx context.Context, ruleStr string, idStr string) er preferredChannels = parsedRule.PreferredChannels } - err = cfg.UpdateRuleIDMatcher(ruleUUID.StringValue(), preferredChannels) + err = cfg.UpdateRuleIDMatcher(id.StringValue(), preferredChannels) if err != nil { return err } @@ -826,22 +811,8 @@ func (m *Manager) ListRuleStates(ctx context.Context) (*ruletypes.GettableRules, return &ruletypes.GettableRules{Rules: resp}, nil } -func (m *Manager) GetRule(ctx context.Context, idStr string) (*ruletypes.GettableRule, error) { - ruleUUID, err := valuer.NewUUID(idStr) - if err != nil { - id, err := strconv.Atoi(idStr) - if err != nil { - return nil, err - } - ruleHistory, err := m.ruleStore.GetRuleUUID(ctx, id) - if err != nil { - return nil, err - } - - ruleUUID = ruleHistory.RuleUUID - } - - s, err := m.ruleStore.GetStoredRule(ctx, ruleUUID) +func (m *Manager) GetRule(ctx context.Context, id valuer.UUID) (*ruletypes.GettableRule, error) { + s, err := m.ruleStore.GetStoredRule(ctx, id) if err != nil { return nil, err } @@ -849,7 +820,7 @@ func (m *Manager) GetRule(ctx context.Context, idStr string) (*ruletypes.Gettabl if err := json.Unmarshal([]byte(s.Data), r); err != nil { return nil, err } - r.Id = ruleUUID.StringValue() + r.Id = id.StringValue() // fetch state of rule from memory if rm, ok := m.rules[r.Id]; !ok { r.State = model.StateDisabled @@ -899,7 +870,7 @@ func (m *Manager) syncRuleStateWithTask(ctx context.Context, orgID valuer.UUID, // - over write the patch attributes received in input (ruleStr) // - re-deploy or undeploy task as necessary // - update the patched rule in the DB -func (m *Manager) PatchRule(ctx context.Context, ruleStr string, ruleIdStr string) (*ruletypes.GettableRule, error) { +func (m *Manager) PatchRule(ctx context.Context, ruleStr string, id valuer.UUID) (*ruletypes.GettableRule, error) { claims, err := authtypes.ClaimsFromContext(ctx) if err != nil { return nil, err @@ -910,24 +881,19 @@ func (m *Manager) PatchRule(ctx context.Context, ruleStr string, ruleIdStr strin return nil, err } - ruleID, err := valuer.NewUUID(ruleIdStr) - if err != nil { - return nil, errors.New(err.Error()) - } - - taskName := prepareTaskName(ruleID.StringValue()) + taskName := prepareTaskName(id.StringValue()) // retrieve rule from DB - storedJSON, err := m.ruleStore.GetStoredRule(ctx, ruleID) + storedJSON, err := m.ruleStore.GetStoredRule(ctx, id) if err != nil { - zap.L().Error("failed to get stored rule with given id", zap.String("id", ruleID.StringValue()), zap.Error(err)) + zap.L().Error("failed to get stored rule with given id", zap.String("id", id.StringValue()), zap.Error(err)) return nil, err } // storedRule holds the current stored rule from DB storedRule := ruletypes.PostableRule{} if err := json.Unmarshal([]byte(storedJSON.Data), &storedRule); err != nil { - zap.L().Error("failed to unmarshal stored rule with given id", zap.String("id", ruleID.StringValue()), zap.Error(err)) + zap.L().Error("failed to unmarshal stored rule with given id", zap.String("id", id.StringValue()), zap.Error(err)) return nil, err } @@ -964,12 +930,12 @@ func (m *Manager) PatchRule(ctx context.Context, ruleStr string, ruleIdStr strin // prepare http response response := ruletypes.GettableRule{ - Id: ruleID.StringValue(), + Id: id.StringValue(), PostableRule: *patchedRule, } // fetch state of rule from memory - if rm, ok := m.rules[ruleID.StringValue()]; !ok { + if rm, ok := m.rules[id.StringValue()]; !ok { response.State = model.StateDisabled response.Disabled = true } else { diff --git a/pkg/ruler/rulestore/sqlrulestore/rule.go b/pkg/ruler/rulestore/sqlrulestore/rule.go index 68093eb233b0..8875a05b3e0f 100644 --- a/pkg/ruler/rulestore/sqlrulestore/rule.go +++ b/pkg/ruler/rulestore/sqlrulestore/rule.go @@ -103,17 +103,3 @@ func (r *rule) GetStoredRule(ctx context.Context, id valuer.UUID) (*ruletypes.Ru } return rule, nil } - -func (r *rule) GetRuleUUID(ctx context.Context, ruleID int) (*ruletypes.RuleHistory, error) { - ruleHistory := new(ruletypes.RuleHistory) - err := r.sqlstore. - BunDB(). - NewSelect(). - Model(ruleHistory). - Where("rule_id = ?", ruleID). - Scan(ctx) - if err != nil { - return nil, err - } - return ruleHistory, nil -} diff --git a/pkg/signoz/provider.go b/pkg/signoz/provider.go index 0d007b0f7513..aadc8166d2d3 100644 --- a/pkg/signoz/provider.go +++ b/pkg/signoz/provider.go @@ -91,6 +91,7 @@ func NewSQLMigrationProviderFactories(sqlstore sqlstore.SQLStore) factory.NamedM sqlmigration.NewAddTraceFunnelsFactory(sqlstore), sqlmigration.NewUpdateDashboardFactory(sqlstore), sqlmigration.NewDropFeatureSetFactory(), + sqlmigration.NewDropDeprecatedTablesFactory(), ) } diff --git a/pkg/sqlmigration/040_drop_deprecated_tables.go b/pkg/sqlmigration/040_drop_deprecated_tables.go new file mode 100644 index 000000000000..e5a43bcc14de --- /dev/null +++ b/pkg/sqlmigration/040_drop_deprecated_tables.go @@ -0,0 +1,66 @@ +package sqlmigration + +import ( + "context" + + "github.com/SigNoz/signoz/pkg/factory" + "github.com/uptrace/bun" + "github.com/uptrace/bun/migrate" +) + +type dropDeprecatedTables struct{} + +func NewDropDeprecatedTablesFactory() factory.ProviderFactory[SQLMigration, Config] { + return factory.NewProviderFactory(factory.MustNewName("drop_deprecated_tables"), func(ctx context.Context, ps factory.ProviderSettings, c Config) (SQLMigration, error) { + return newDropDeprecatedTables(ctx, ps, c) + }) +} + +func newDropDeprecatedTables(_ context.Context, _ factory.ProviderSettings, _ Config) (SQLMigration, error) { + return &dropDeprecatedTables{}, nil +} + +func (migration *dropDeprecatedTables) Register(migrations *migrate.Migrations) error { + if err := migrations.Register(migration.Up, migration.Down); err != nil { + return err + } + + return nil +} + +func (migration *dropDeprecatedTables) Up(ctx context.Context, db *bun.DB) error { + tx, err := db.BeginTx(ctx, nil) + if err != nil { + return err + } + + defer func() { + _ = tx.Rollback() + }() + + if _, err := tx. + NewDropTable(). + IfExists(). + Table("rule_history"). + Exec(ctx); err != nil { + return err + } + + if _, err := tx. + NewDropTable(). + IfExists(). + Table("data_migrations"). + Exec(ctx); err != nil { + return err + } + + if err := tx.Commit(); err != nil { + return err + } + + return nil +} + +func (migration *dropDeprecatedTables) Down(context.Context, *bun.DB) error { + return nil +} diff --git a/pkg/types/datamigration.go b/pkg/types/datamigration.go deleted file mode 100644 index eb70473144b1..000000000000 --- a/pkg/types/datamigration.go +++ /dev/null @@ -1,15 +0,0 @@ -package types - -import ( - "time" - - "github.com/uptrace/bun" -) - -type DataMigration struct { - bun.BaseModel `bun:"table:data_migrations"` - ID int `bun:"id,pk,autoincrement"` - Version string `bun:"version,unique,notnull,type:VARCHAR(255)"` - CreatedAt time.Time `bun:"created_at,notnull,default:current_timestamp"` - Succeeded bool `bun:"succeeded,notnull,default:false"` -} diff --git a/pkg/types/ruletypes/rule.go b/pkg/types/ruletypes/rule.go index ffe5774f2746..beb6fff3fe43 100644 --- a/pkg/types/ruletypes/rule.go +++ b/pkg/types/ruletypes/rule.go @@ -18,17 +18,10 @@ type Rule struct { OrgID string `bun:"org_id,type:text"` } -type RuleHistory struct { - bun.BaseModel `bun:"table:rule_history"` - RuleID int `bun:"rule_id"` - RuleUUID valuer.UUID `bun:"rule_uuid"` -} - type RuleStore interface { CreateRule(context.Context, *Rule, func(context.Context, valuer.UUID) error) (valuer.UUID, error) EditRule(context.Context, *Rule, func(context.Context) error) error DeleteRule(context.Context, valuer.UUID, func(context.Context) error) error GetStoredRules(context.Context, string) ([]*Rule, error) GetStoredRule(context.Context, valuer.UUID) (*Rule, error) - GetRuleUUID(context.Context, int) (*RuleHistory, error) }