Skip to content

Commit

Permalink
fix test and address PR review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
soniaAguilarPeiron committed Apr 30, 2024
1 parent 8df79e3 commit 9a4b019
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 113 deletions.
Expand Up @@ -3,9 +3,9 @@ import React from 'react';
import { TestProvider } from 'test/helpers/TestProvider';
import { byTestId } from 'testing-library-selector';

import { DataSourceApi } from '@grafana/data';
import { DataSourceApi, PluginExtensionComponent } from '@grafana/data';
import { PromOptions, PrometheusDatasource } from '@grafana/prometheus';
import { setDataSourceSrv } from '@grafana/runtime';
import { UsePluginExtensions, setDataSourceSrv, setPluginExtensionsHook } from '@grafana/runtime';
import * as ruleActionButtons from 'app/features/alerting/unified/components/rules/RuleActionsButtons';
import { DashboardModel, PanelModel } from 'app/features/dashboard/state';
import { getDatasourceSrv } from 'app/features/plugins/datasource_srv';
Expand Down Expand Up @@ -203,6 +203,12 @@ describe('PanelAlertTabContent', () => {
AccessControlAction.AlertingRuleExternalWrite,
]);

const getter: UsePluginExtensions<PluginExtensionComponent> = jest
.fn()
.mockReturnValue({ extensions: [], isLoading: false });

setPluginExtensionsHook(getter);

mocks.getAllDataSources.mockReturnValue(Object.values(dataSources));
const dsService = new MockDataSourceSrv(dataSources);
dsService.datasources[dataSources.prometheus.uid] = new PrometheusDatasource(
Expand Down
112 changes: 4 additions & 108 deletions public/app/features/alerting/unified/hooks/useCombinedRule.ts
@@ -1,40 +1,25 @@
import { useEffect, useMemo, useRef } from 'react';
import { useEffect, useMemo } from 'react';
import { useAsync } from 'react-use';

import { useDispatch } from 'app/types';
import {
CombinedRule,
CombinedRuleNamespace,
RuleIdentifier,
RuleNamespace,
RulerDataSourceConfig,
} from 'app/types/unified-alerting';
import { CombinedRule, RuleIdentifier, RuleNamespace, RulerDataSourceConfig } from 'app/types/unified-alerting';
import { RulerRuleGroupDTO, RulerRulesConfigDTO } from 'app/types/unified-alerting-dto';

import { alertRuleApi } from '../api/alertRuleApi';
import { featureDiscoveryApi } from '../api/featureDiscoveryApi';
import { fetchPromAndRulerRulesAction } from '../state/actions';
import {
getDataSourceByName,
getRulesSourceByName,
GRAFANA_RULES_SOURCE_NAME,
isGrafanaRulesSource,
} from '../utils/datasource';
import { getDataSourceByName, GRAFANA_RULES_SOURCE_NAME, isGrafanaRulesSource } from '../utils/datasource';
import { AsyncRequestMapSlice, AsyncRequestState, initialAsyncRequestState } from '../utils/redux';
import * as ruleId from '../utils/rule-id';
import {
isCloudRuleIdentifier,
isGrafanaRuleIdentifier,
isGrafanaRulerRule,
isPrometheusRuleIdentifier,
isRulerNotSupportedResponse,
} from '../utils/rules';

import {
addPromGroupsToCombinedNamespace,
addRulerGroupsToCombinedNamespace,
attachRulerRulesToCombinedRules,
CacheValue,
combineRulesNamespaces,
useCombinedRuleNamespaces,
} from './useCombinedRuleNamespaces';
Expand Down Expand Up @@ -177,95 +162,6 @@ function getRequestState(
return state;
}

/*
This hook returns combined Grafana rules by dashpboard UID
*/
export function useCombinedRulesByDashboard(
dashboardUID: string,
panelId?: number
): {
loading: boolean;
result?: CombinedRuleNamespace[];
error?: unknown;
} {
const {
currentData: promRuleNs,
isLoading: isLoadingPromRules,
error: promRuleNsError,
} = alertRuleApi.endpoints.prometheusRuleNamespaces.useQuery({
ruleSourceName: GRAFANA_RULES_SOURCE_NAME,
dashboardUid: dashboardUID,
panelId,
});

const {
currentData: rulerRules,
isLoading: isLoadingRulerRules,
error: rulerRulesError,
} = alertRuleApi.endpoints.rulerRules.useQuery({
rulerConfig: grafanaRulerConfig,
filter: { dashboardUID: dashboardUID, panelId },
});

//---------
// cache results per rules source, so we only recalculate those for which results have actually changed
const cache = useRef<Record<string, CacheValue>>({});

const rulesSource = getRulesSourceByName(GRAFANA_RULES_SOURCE_NAME);

const rules = useMemo(() => {
if (!rulesSource) {
return [];
}

const cached = cache.current[GRAFANA_RULES_SOURCE_NAME];
if (cached && cached.promRules === promRuleNs && cached.rulerRules === rulerRules) {
return cached.result;
}
const namespaces: Record<string, CombinedRuleNamespace> = {};

// first get all the ruler rules from the data source
Object.entries(rulerRules || {}).forEach(([namespaceName, groups]) => {
const namespace: CombinedRuleNamespace = {
rulesSource,
name: namespaceName,
groups: [],
};

// We need to set the namespace_uid for grafana rules as it's required to obtain the rule's groups
// All rules from all groups have the same namespace_uid so we're taking the first one.
if (isGrafanaRulerRule(groups[0].rules[0])) {
namespace.uid = groups[0].rules[0].grafana_alert.namespace_uid;
}

namespaces[namespaceName] = namespace;
addRulerGroupsToCombinedNamespace(namespace, groups);
});

// then correlate with prometheus rules
promRuleNs?.forEach(({ name: namespaceName, groups }) => {
const ns = (namespaces[namespaceName] = namespaces[namespaceName] || {
rulesSource,
name: namespaceName,
groups: [],
});

addPromGroupsToCombinedNamespace(ns, groups);
});

const result = Object.values(namespaces);

cache.current[GRAFANA_RULES_SOURCE_NAME] = { promRules: promRuleNs, rulerRules, result };
return result;
}, [promRuleNs, rulerRules, rulesSource]);

return {
loading: isLoadingPromRules || isLoadingRulerRules,
error: promRuleNsError ?? rulerRulesError,
result: rules,
};
}

export function useCombinedRule({ ruleIdentifier }: { ruleIdentifier: RuleIdentifier }): {
loading: boolean;
result?: CombinedRule;
Expand Down Expand Up @@ -386,7 +282,7 @@ export function useCombinedRule({ ruleIdentifier }: { ruleIdentifier: RuleIdenti
};
}

const grafanaRulerConfig: RulerDataSourceConfig = {
export const grafanaRulerConfig: RulerDataSourceConfig = {
dataSourceName: GRAFANA_RULES_SOURCE_NAME,
apiVersion: 'legacy',
};
Expand Down
Expand Up @@ -21,6 +21,7 @@ import {
RulerRulesConfigDTO,
} from 'app/types/unified-alerting-dto';

import { alertRuleApi } from '../api/alertRuleApi';
import {
getAllRulesSources,
getRulesSourceByName,
Expand All @@ -36,6 +37,7 @@ import {
isRecordingRulerRule,
} from '../utils/rules';

import { grafanaRulerConfig } from './useCombinedRule';
import { useUnifiedAlertingSelector } from './useUnifiedAlertingSelector';

export interface CacheValue {
Expand Down Expand Up @@ -460,3 +462,92 @@ function hashQuery(query: string) {
// labels matchers can be reordered, so sort the enitre string, esentially comparing just the character counts
return query.split('').sort().join('');
}

/*
This hook returns combined Grafana rules by dashpboard UID
*/
export function useCombinedRulesByDashboard(
dashboardUID: string,
panelId?: number
): {
loading: boolean;
result?: CombinedRuleNamespace[];
error?: unknown;
} {
const {
currentData: promRuleNs,
isLoading: isLoadingPromRules,
error: promRuleNsError,
} = alertRuleApi.endpoints.prometheusRuleNamespaces.useQuery({
ruleSourceName: GRAFANA_RULES_SOURCE_NAME,
dashboardUid: dashboardUID,
panelId,
});

const {
currentData: rulerRules,
isLoading: isLoadingRulerRules,
error: rulerRulesError,
} = alertRuleApi.endpoints.rulerRules.useQuery({
rulerConfig: grafanaRulerConfig,
filter: { dashboardUID: dashboardUID, panelId },
});

//---------
// cache results per rules source, so we only recalculate those for which results have actually changed
const cache = useRef<Record<string, CacheValue>>({});

const rulesSource = getRulesSourceByName(GRAFANA_RULES_SOURCE_NAME);

const rules = useMemo(() => {
if (!rulesSource) {
return [];
}

const cached = cache.current[GRAFANA_RULES_SOURCE_NAME];
if (cached && cached.promRules === promRuleNs && cached.rulerRules === rulerRules) {
return cached.result;
}
const namespaces: Record<string, CombinedRuleNamespace> = {};

// first get all the ruler rules from the data source
Object.entries(rulerRules || {}).forEach(([namespaceName, groups]) => {
const namespace: CombinedRuleNamespace = {
rulesSource,
name: namespaceName,
groups: [],
};

// We need to set the namespace_uid for grafana rules as it's required to obtain the rule's groups
// All rules from all groups have the same namespace_uid so we're taking the first one.
if (isGrafanaRulerRule(groups[0].rules[0])) {
namespace.uid = groups[0].rules[0].grafana_alert.namespace_uid;
}

namespaces[namespaceName] = namespace;
addRulerGroupsToCombinedNamespace(namespace, groups);
});

// then correlate with prometheus rules
promRuleNs?.forEach(({ name: namespaceName, groups }) => {
const ns = (namespaces[namespaceName] = namespaces[namespaceName] || {
rulesSource,
name: namespaceName,
groups: [],
});

addPromGroupsToCombinedNamespace(ns, groups);
});

const result = Object.values(namespaces);

cache.current[GRAFANA_RULES_SOURCE_NAME] = { promRules: promRuleNs, rulerRules, result };
return result;
}, [promRuleNs, rulerRules, rulesSource]);

return {
loading: isLoadingPromRules || isLoadingRulerRules,
error: promRuleNsError ?? rulerRulesError,
result: rules,
};
}
@@ -1,6 +1,6 @@
import { CombinedRule } from 'app/types/unified-alerting';

import { useCombinedRulesByDashboard } from './useCombinedRule';
import { useCombinedRulesByDashboard } from './useCombinedRuleNamespaces';

interface Options {
dashboardUID: string;
Expand Down
Expand Up @@ -3,7 +3,7 @@ import React from 'react';
import { LoadingPlaceholder } from '@grafana/ui';

import { RulesTable } from '../components/rules/RulesTable';
import { useCombinedRulesByDashboard } from '../hooks/useCombinedRule';
import { useCombinedRulesByDashboard } from '../hooks/useCombinedRuleNamespaces';

interface Props {
dashboardUid: string;
Expand Down
Expand Up @@ -106,7 +106,6 @@ describe('UnifiedAlertStatesWorker', () => {
describe('when run repeatedly for the same dashboard and no alert rules are found', () => {
const nameSpaces = [mockPromRuleNamespace({ groups: [] })];
const { dispatchMock, options } = getTestContext();
//getMock.mockResolvedValue(getResults);
dispatchMock.mockResolvedValue(nameSpaces);
it('then canWork should start returning false', async () => {
const worker = new UnifiedAlertStatesWorker();
Expand Down

0 comments on commit 9a4b019

Please sign in to comment.