Alerting: Fix rule state history with annotations backend (grafana#10…

…1174) * add alertUID to annotations API query parameter * update state history UI to fetch rule by UID --------- Signed-off-by: Yuri Tseretyan <yuriy.tseretyan@grafana.com>
tipee-sa · Feb 21, 2025 · bbeae46 · bbeae46
1 parent c33e908
commit bbeae46
Show file tree

Hide file tree

Showing 14 changed files with 97 additions and 24 deletions.
diff --git a/pkg/api/annotations.go b/pkg/api/annotations.go
@@ -41,6 +41,7 @@ func (hs *HTTPServer) GetAnnotations(c *contextmodel.ReqContext) response.Respon
 		OrgID:        c.SignedInUser.GetOrgID(),
 		UserID:       c.QueryInt64("userId"),
 		AlertID:      c.QueryInt64("alertId"),
+		AlertUID:     c.Query("alertUID"),
 		DashboardID:  c.QueryInt64("dashboardId"),
 		DashboardUID: c.Query("dashboardUID"),
 		PanelID:      c.QueryInt64("panelId"),
@@ -744,10 +745,15 @@ type GetAnnotationsParams struct {
 	// in:query
 	// required:false
 	UserID int64 `json:"userId"`
-	// Find annotations for a specified alert.
+	// Find annotations for a specified alert rule by its ID.
+	// deprecated: AlertID is deprecated and will be removed in future versions. Please use AlertUID instead.
 	// in:query
 	// required:false
 	AlertID int64 `json:"alertId"`
+	// Find annotations for a specified alert rule by its UID.
+	// in:query
+	// required:false
+	AlertUID string `json:"alertUID"`
 	// Find annotations that are scoped to a specific dashboard
 	// in:query
 	// required:false

diff --git a/pkg/services/annotations/annotationsimpl/loki/historian_store.go b/pkg/services/annotations/annotationsimpl/loki/historian_store.go
@@ -91,20 +91,22 @@ func (r *LokiHistorianStore) Get(ctx context.Context, query annotations.ItemQuer
 		return make([]*annotations.ItemDTO, 0), nil
 	}
 
-	rule := &ngmodels.AlertRule{}
-	if query.AlertID != 0 {
-		var err error
-		rule, err = r.ruleStore.GetRuleByID(ctx, ngmodels.GetAlertRuleByIDQuery{OrgID: query.OrgID, ID: query.AlertID})
+	var ruleUID string
+	if query.AlertUID != "" {
+		ruleUID = query.AlertUID
+	} else if query.AlertID != 0 {
+		rule, err := r.ruleStore.GetRuleByID(ctx, ngmodels.GetAlertRuleByIDQuery{OrgID: query.OrgID, ID: query.AlertID})
 		if err != nil {
 			if errors.Is(err, ngmodels.ErrAlertRuleNotFound) {
 				return make([]*annotations.ItemDTO, 0), ErrLokiStoreNotFound.Errorf("rule with ID %d does not exist", query.AlertID)
 			}
 			return make([]*annotations.ItemDTO, 0), ErrLokiStoreInternal.Errorf("failed to query rule: %w", err)
 		}
+		ruleUID = rule.UID
 	}
 
 	// No folders in the filter because it filter by Dashboard UID, and the request is already authorized.
-	logQL, err := historian.BuildLogQuery(buildHistoryQuery(&query, accessResources.Dashboards, rule.UID), nil, r.client.MaxQuerySize())
+	logQL, err := historian.BuildLogQuery(buildHistoryQuery(&query, accessResources.Dashboards, ruleUID), nil, r.client.MaxQuerySize())
 	if err != nil {
 		grafanaErr := errutil.Error{}
 		if errors.As(err, &grafanaErr) {

diff --git a/pkg/services/annotations/annotationsimpl/loki/historian_store_test.go b/pkg/services/annotations/annotationsimpl/loki/historian_store_test.go
@@ -108,7 +108,34 @@ func TestIntegrationAlertStateHistoryStore(t *testing.T) {
 			require.Len(t, res, numTransitions)
 		})
 
-		t.Run("should return ErrLokiStoreNotFound if rule is not found", func(t *testing.T) {
+		t.Run("can query history by alert uid", func(t *testing.T) {
+			rule := dashboardRules[dashboard1.UID][0]
+
+			fakeLokiClient.rangeQueryRes = []historian.Stream{
+				historian.StatesToStream(ruleMetaFromRule(t, rule), transitions, map[string]string{}, log.NewNopLogger()),
+			}
+
+			query := annotations.ItemQuery{
+				OrgID:    1,
+				AlertUID: rule.UID,
+				From:     start.UnixMilli(),
+				To:       start.Add(time.Second * time.Duration(numTransitions+1)).UnixMilli(),
+			}
+			res, err := store.Get(
+				context.Background(),
+				query,
+				&annotation_ac.AccessResources{
+					Dashboards: map[string]int64{
+						dashboard1.UID: dashboard1.ID,
+					},
+					CanAccessDashAnnotations: true,
+				},
+			)
+			require.NoError(t, err)
+			require.Len(t, res, numTransitions)
+		})
+
+		t.Run("should return ErrLokiStoreNotFound if rule is not found by ID", func(t *testing.T) {
 			var rules = slices.Concat(maps.Values(dashboardRules)...)
 			id := rand.Int63n(1000) // in Postgres ID is integer, so limit range
 			// make sure id is not known
@@ -137,6 +164,27 @@ func TestIntegrationAlertStateHistoryStore(t *testing.T) {
 			require.ErrorIs(t, err, ErrLokiStoreNotFound)
 		})
 
+		t.Run("should return empty response if rule is not found by UID", func(t *testing.T) {
+			query := annotations.ItemQuery{
+				OrgID:    1,
+				AlertUID: "not-found-uid",
+				From:     start.UnixMilli(),
+				To:       start.Add(time.Second * time.Duration(numTransitions+1)).UnixMilli(),
+			}
+			res, err := store.Get(
+				context.Background(),
+				query,
+				&annotation_ac.AccessResources{
+					Dashboards: map[string]int64{
+						dashboard1.UID: dashboard1.ID,
+					},
+					CanAccessDashAnnotations: true,
+				},
+			)
+			require.NoError(t, err)
+			require.Empty(t, res)
+		})
+
 		t.Run("can query history by dashboard id", func(t *testing.T) {
 			fakeLokiClient.rangeQueryRes = []historian.Stream{
 				historian.StatesToStream(ruleMetaFromRule(t, dashboardRules[dashboard1.UID][0]), transitions, map[string]string{}, log.NewNopLogger()),

diff --git a/pkg/services/annotations/annotationsimpl/xorm_store.go b/pkg/services/annotations/annotationsimpl/xorm_store.go
@@ -267,10 +267,10 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query annotations.ItemQuer
 				annotation.updated,
 				usr.email,
 				usr.login,
-				alert.name as alert_name
+				r.title as alert_name
 			FROM annotation
 			LEFT OUTER JOIN ` + r.db.GetDialect().Quote("user") + ` as usr on usr.id = annotation.user_id
-			LEFT OUTER JOIN alert on alert.id = annotation.alert_id
+			LEFT OUTER JOIN alert_rule as r on r.id = annotation.alert_id
 			INNER JOIN (
 				SELECT a.id from annotation a
 			`)
@@ -287,6 +287,9 @@ func (r *xormRepositoryImpl) Get(ctx context.Context, query annotations.ItemQuer
 		if query.AlertID != 0 {
 			sql.WriteString(` AND a.alert_id = ?`)
 			params = append(params, query.AlertID)
+		} else if query.AlertUID != "" {
+			sql.WriteString(` AND a.alert_id = (SELECT id FROM alert_rule WHERE uid = ? and org_id = ?)`)
+			params = append(params, query.AlertUID, query.OrgID)
 		}
 
 		if query.DashboardID != 0 {

diff --git a/pkg/services/annotations/models.go b/pkg/services/annotations/models.go
@@ -11,6 +11,7 @@ type ItemQuery struct {
 	To           int64    `json:"to"`
 	UserID       int64    `json:"userId"`
 	AlertID      int64    `json:"alertId"`
+	AlertUID     string   `json:"alertUID"`
 	DashboardID  int64    `json:"dashboardId"`
 	DashboardUID string   `json:"dashboardUID"`
 	PanelID      int64    `json:"panelId"`

diff --git a/public/api-merged.json b/public/api-merged.json
@@ -1833,10 +1833,16 @@
           {
             "type": "integer",
             "format": "int64",
-            "description": "Find annotations for a specified alert.",
+            "description": "Find annotations for a specified alert rule by its ID.\ndeprecated: AlertID is deprecated and will be removed in future versions. Please use AlertUID instead.",
             "name": "alertId",
             "in": "query"
           },
+          {
+            "type": "string",
+            "description": "Find annotations for a specified alert rule by its UID.",
+            "name": "alertUID",
+            "in": "query"
+          },
           {
             "type": "integer",
             "format": "int64",

diff --git a/public/app/features/alerting/unified/api/annotations.test.ts b/public/app/features/alerting/unified/api/annotations.test.ts
@@ -21,7 +21,7 @@ describe('annotations', () => {
   it('should fetch annotation for an alertId', () => {
     const ALERT_ID = 'abc123';
     fetchAnnotations(ALERT_ID);
-    expect(get).toBeCalledWith('/api/annotations', { alertId: ALERT_ID });
+    expect(get).toBeCalledWith('/api/annotations', { alertUID: ALERT_ID });
   });
 });
 

diff --git a/public/app/features/alerting/unified/api/annotations.ts b/public/app/features/alerting/unified/api/annotations.ts
@@ -1,10 +1,10 @@
 import { getBackendSrv } from '@grafana/runtime';
 import { StateHistoryItem } from 'app/types/unified-alerting';
 
-export function fetchAnnotations(alertId: string): Promise<StateHistoryItem[]> {
+export function fetchAnnotations(alertUID: string): Promise<StateHistoryItem[]> {
   return getBackendSrv()
     .get('/api/annotations', {
-      alertId,
+      alertUID,
     })
     .then((result) => {
       return result?.sort(sortStateHistory);

diff --git a/public/app/features/alerting/unified/components/rule-viewer/tabs/History.tsx b/public/app/features/alerting/unified/components/rule-viewer/tabs/History.tsx
@@ -26,13 +26,12 @@ const History = ({ rule }: HistoryProps) => {
     ? StateHistoryImplementation.Loki
     : StateHistoryImplementation.Annotations;
 
-  const ruleID = rule.grafana_alert.id ?? '';
   const ruleUID = rule.grafana_alert.uid;
 
   return (
     <Suspense fallback={'Loading...'}>
       {implementation === StateHistoryImplementation.Loki && <LokiStateHistory ruleUID={ruleUID} />}
-      {implementation === StateHistoryImplementation.Annotations && <AnnotationsStateHistory alertId={ruleID} />}
+      {implementation === StateHistoryImplementation.Annotations && <AnnotationsStateHistory ruleUID={ruleUID} />}
     </Suspense>
   );
 };

diff --git a/public/app/features/alerting/unified/components/rules/state-history/StateHistory.tsx b/public/app/features/alerting/unified/components/rules/state-history/StateHistory.tsx
@@ -27,16 +27,16 @@ type StateHistoryMap = Record<string, StateHistoryRowItem[]>;
 type StateHistoryRow = DynamicTableItemProps<StateHistoryRowItem>;
 
 interface Props {
-  alertId: string;
+  ruleUID: string;
 }
 
-const StateHistory = ({ alertId }: Props) => {
+const StateHistory = ({ ruleUID }: Props) => {
   const [textFilter, setTextFilter] = useState<string>('');
   const handleTextFilter = useCallback((event: FormEvent<HTMLInputElement>) => {
     setTextFilter(event.currentTarget.value);
   }, []);
 
-  const { loading, error, result = [] } = useManagedAlertStateHistory(alertId);
+  const { loading, error, result = [] } = useManagedAlertStateHistory(ruleUID);
 
   const styles = useStyles2(getStyles);
 

diff --git a/public/app/features/alerting/unified/hooks/useManagedAlertStateHistory.ts b/public/app/features/alerting/unified/hooks/useManagedAlertStateHistory.ts
@@ -8,15 +8,15 @@ import { AsyncRequestState } from '../utils/redux';
 
 import { useUnifiedAlertingSelector } from './useUnifiedAlertingSelector';
 
-export function useManagedAlertStateHistory(alertId: string) {
+export function useManagedAlertStateHistory(ruleUID: string) {
   const dispatch = useDispatch();
   const history = useUnifiedAlertingSelector<AsyncRequestState<StateHistoryItem[]>>(
     (state) => state.managedAlertStateHistory
   );
 
   useEffect(() => {
-    dispatch(fetchGrafanaAnnotationsAction(alertId));
-  }, [dispatch, alertId]);
+    dispatch(fetchGrafanaAnnotationsAction(ruleUID));
+  }, [dispatch, ruleUID]);
 
   return history;
 }
diff --git a/public/app/features/alerting/unified/hooks/useStateHistoryModal.tsx b/public/app/features/alerting/unified/hooks/useStateHistoryModal.tsx
@@ -61,7 +61,7 @@ function useStateHistoryModal() {
         <Suspense fallback={'Loading...'}>
           {implementation === StateHistoryImplementation.Loki && <LokiStateHistory ruleUID={rule.grafana_alert.uid} />}
           {implementation === StateHistoryImplementation.Annotations && (
-            <AnnotationsStateHistory alertId={rule.grafana_alert.id ?? ''} />
+            <AnnotationsStateHistory ruleUID={rule.grafana_alert.id ?? ''} />
           )}
         </Suspense>
       </Modal>

diff --git a/public/app/features/alerting/unified/state/actions.ts b/public/app/features/alerting/unified/state/actions.ts
@@ -183,7 +183,7 @@ export function fetchAllPromRulesAction(
 
 export const fetchGrafanaAnnotationsAction = createAsyncThunk(
   'unifiedalerting/fetchGrafanaAnnotations',
-  (alertId: string): Promise<StateHistoryItem[]> => withSerializedError(fetchAnnotations(alertId))
+  (ruleUID: string): Promise<StateHistoryItem[]> => withSerializedError(fetchAnnotations(ruleUID))
 );
 
 interface UpdateAlertManagerConfigActionOptions {

diff --git a/public/openapi3.json b/public/openapi3.json
@@ -15316,14 +15316,22 @@
             }
           },
           {
-            "description": "Find annotations for a specified alert.",
+            "description": "Find annotations for a specified alert rule by its ID.\ndeprecated: AlertID is deprecated and will be removed in future versions. Please use AlertUID instead.",
             "in": "query",
             "name": "alertId",
             "schema": {
               "format": "int64",
               "type": "integer"
             }
           },
+          {
+            "description": "Find annotations for a specified alert rule by its UID.",
+            "in": "query",
+            "name": "alertUID",
+            "schema": {
+              "type": "string"
+            }
+          },
           {
             "description": "Find annotations that are scoped to a specific dashboard",
             "in": "query",