Skip to content

Commit

Permalink
Use non-blocking way of fetching ruler api availability (grafana#52102)
Browse files Browse the repository at this point in the history
Co-authored-by: Joe Blubaugh <[email protected]>
  • Loading branch information
konrad147 and joeblubaugh authored Jul 13, 2022
1 parent 2163281 commit 169a1c5
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 39 deletions.
3 changes: 0 additions & 3 deletions .betterer.results
Original file line number Diff line number Diff line change
Expand Up @@ -3910,9 +3910,6 @@ exports[`better eslint`] = {
[0, 0, 0, "Do not use any type assertions.", "0"],
[0, 0, 0, "Do not use any type assertions.", "1"]
],
"public/app/features/alerting/unified/components/rules/RulesGroup.test.tsx:5381": [
[0, 0, 0, "Unexpected any. Specify a different type.", "0"]
],
"public/app/features/alerting/unified/components/silences/SilencesEditor.tsx:5381": [
[0, 0, 0, "Do not use any type assertions.", "0"]
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@ interface Props {
export const CloudRules: FC<Props> = ({ namespaces, expandAll }) => {
const styles = useStyles2(getStyles);

const dsConfigs = useUnifiedAlertingSelector((state) => state.dataSources);
const rules = useUnifiedAlertingSelector((state) => state.promRules);
const rulesDataSources = useMemo(getRulesDataSources, []);
const groupsWithNamespaces = useCombinedGroupNamespace(namespaces);

const dataSourcesLoading = useMemo(
() => rulesDataSources.filter((ds) => rules[ds.name]?.loading),
[rules, rulesDataSources]
() => rulesDataSources.filter((ds) => rules[ds.name]?.loading || dsConfigs[ds.name]?.loading),
[rules, dsConfigs, rulesDataSources]
);

const { numberOfPages, onPageChange, page, pageItems } = usePagination(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,26 @@ import { contextSrv } from 'app/core/services/context_srv';
import { configureStore } from 'app/store/configureStore';
import { CombinedRuleGroup, CombinedRuleNamespace } from 'app/types/unified-alerting';

import { useHasRuler } from '../../hooks/useHasRuler';
import { disableRBAC, mockCombinedRule, mockDataSource } from '../../mocks';

import { RulesGroup } from './RulesGroup';

const hasRulerMock = jest.fn<boolean, any>();
jest.mock('../../hooks/useHasRuler', () => ({
useHasRuler: () => hasRulerMock,
}));
jest.mock('../../hooks/useHasRuler');

const mocks = {
useHasRuler: jest.mocked(useHasRuler),
};

function mockUseHasRuler(hasRuler: boolean, rulerRulesLoaded: boolean) {
mocks.useHasRuler.mockReturnValue({
hasRuler: () => hasRuler,
rulerRulesLoaded: () => rulerRulesLoaded,
});
}

beforeEach(() => {
hasRulerMock.mockReset();
mocks.useHasRuler.mockReset();
});

const ui = {
Expand Down Expand Up @@ -55,6 +64,7 @@ describe('Rules group tests', () => {

it('Should hide delete and edit group buttons', () => {
// Act
mockUseHasRuler(true, true);
renderRulesGroup(namespace, group);

// Assert
Expand Down Expand Up @@ -83,33 +93,33 @@ describe('Rules group tests', () => {

it('When ruler enabled should display delete and edit group buttons', () => {
// Arrange
hasRulerMock.mockReturnValue(true);
mockUseHasRuler(true, true);

// Act
renderRulesGroup(namespace, group);

// Assert
expect(hasRulerMock).toHaveBeenCalled();
expect(mocks.useHasRuler).toHaveBeenCalled();
expect(ui.deleteGroupButton.get()).toBeInTheDocument();
expect(ui.editGroupButton.get()).toBeInTheDocument();
});

it('When ruler disabled should hide delete and edit group buttons', () => {
// Arrange
hasRulerMock.mockReturnValue(false);
mockUseHasRuler(false, false);

// Act
renderRulesGroup(namespace, group);

// Assert
expect(hasRulerMock).toHaveBeenCalled();
expect(mocks.useHasRuler).toHaveBeenCalled();
expect(ui.deleteGroupButton.query()).not.toBeInTheDocument();
expect(ui.editGroupButton.query()).not.toBeInTheDocument();
});

it('Delete button click should display confirmation modal', async () => {
// Arrange
hasRulerMock.mockReturnValue(true);
mockUseHasRuler(true, true);

// Act
renderRulesGroup(namespace, group);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,14 @@ export const RulesGroup: FC<Props> = React.memo(({ group, namespace, expandAll }
setIsCollapsed(!expandAll);
}, [expandAll]);

const hasRuler = useHasRuler();
const { hasRuler, rulerRulesLoaded } = useHasRuler();
const rulerRule = group.rules[0]?.rulerRule;
const folderUID = (rulerRule && isGrafanaRulerRule(rulerRule) && rulerRule.grafana_alert.namespace_uid) || undefined;
const { folder } = useFolder(folderUID);

// group "is deleting" if rules source has ruler, but this group has no rules that are in ruler
const isDeleting = hasRuler(rulesSource) && !group.rules.find((rule) => !!rule.rulerRule);
const isDeleting =
hasRuler(rulesSource) && rulerRulesLoaded(rulesSource) && !group.rules.find((rule) => !!rule.rulerRule);
const isFederated = isFederatedRuleGroup(group);

const deleteGroup = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ export const getStyles = (theme: GrafanaTheme2) => ({
});

function useColumns(showSummaryColumn: boolean, showGroupColumn: boolean) {
const hasRuler = useHasRuler();
const { hasRuler, rulerRulesLoaded } = useHasRuler();

return useMemo((): RuleTableColumnProps[] => {
const columns: RuleTableColumnProps[] = [
Expand All @@ -118,8 +118,8 @@ function useColumns(showSummaryColumn: boolean, showGroupColumn: boolean) {
const { namespace } = rule;
const { rulesSource } = namespace;
const { promRule, rulerRule } = rule;
const isDeleting = !!(hasRuler(rulesSource) && promRule && !rulerRule);
const isCreating = !!(hasRuler(rulesSource) && rulerRule && !promRule);
const isDeleting = !!(hasRuler(rulesSource) && rulerRulesLoaded(rulesSource) && promRule && !rulerRule);
const isCreating = !!(hasRuler(rulesSource) && rulerRulesLoaded(rulesSource) && rulerRule && !promRule);
return <RuleState rule={rule} isDeleting={isDeleting} isCreating={isCreating} />;
},
size: '165px',
Expand Down Expand Up @@ -188,5 +188,5 @@ function useColumns(showSummaryColumn: boolean, showGroupColumn: boolean) {
});
}
return columns;
}, [hasRuler, showSummaryColumn, showGroupColumn]);
}, [hasRuler, rulerRulesLoaded, showSummaryColumn, showGroupColumn]);
}
19 changes: 16 additions & 3 deletions public/app/features/alerting/unified/hooks/useHasRuler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,31 @@ import { useCallback } from 'react';

import { RulesSource } from 'app/types/unified-alerting';

import { GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource';
import { getRulesSourceName, GRAFANA_RULES_SOURCE_NAME } from '../utils/datasource';

import { useUnifiedAlertingSelector } from './useUnifiedAlertingSelector';

// datasource has ruler if it's grafana managed or if we're able to load rules from it
export function useHasRuler(): (rulesSource: string | RulesSource) => boolean {
export function useHasRuler() {
const rulerRules = useUnifiedAlertingSelector((state) => state.rulerRules);
return useCallback(

const hasRuler = useCallback(
(rulesSource: string | RulesSource) => {
const rulesSourceName = typeof rulesSource === 'string' ? rulesSource : rulesSource.name;
return rulesSourceName === GRAFANA_RULES_SOURCE_NAME || !!rulerRules[rulesSourceName]?.result;
},
[rulerRules]
);

const rulerRulesLoaded = useCallback(
(rulesSource: RulesSource) => {
const rulesSourceName = getRulesSourceName(rulesSource);
const result = rulerRules[rulesSourceName]?.result;

return Boolean(result);
},
[rulerRules]
);

return { hasRuler, rulerRulesLoaded };
}
34 changes: 19 additions & 15 deletions public/app/features/alerting/unified/state/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,30 +255,34 @@ export const fetchRulesSourceBuildInfoAction = createAsyncThunk(
const dataSources: AsyncRequestMapSlice<PromBasedDataSource> = (getState() as StoreState).unifiedAlerting
.dataSources;
const hasLoaded = Boolean(dataSources[rulesSourceName]?.result);
return !hasLoaded;
const hasError = Boolean(dataSources[rulesSourceName]?.error);

return !(hasLoaded || hasError);
},
}
);

export function fetchAllPromAndRulerRulesAction(force = false): ThunkResult<void> {
return async (dispatch, getStore) => {
await dispatch(fetchAllPromBuildInfoAction());
return Promise.all(
getAllRulesSourceNames().map(async (rulesSourceName) => {
await dispatch(fetchRulesSourceBuildInfoAction({ rulesSourceName }));

const { promRules, rulerRules, dataSources } = getStore().unifiedAlerting;
const { promRules, rulerRules, dataSources } = getStore().unifiedAlerting;
const dataSourceConfig = dataSources[rulesSourceName].result;

getAllRulesSourceNames().map((rulesSourceName) => {
const dataSourceConfig = dataSources[rulesSourceName].result;
if (!dataSourceConfig) {
return;
}
if (!dataSourceConfig) {
return;
}

if (force || !promRules[rulesSourceName]?.loading) {
dispatch(fetchPromRulesAction({ rulesSourceName }));
}
if ((force || !rulerRules[rulesSourceName]?.loading) && dataSourceConfig.rulerConfig) {
dispatch(fetchRulerRulesAction({ rulesSourceName }));
}
});
if (force || !promRules[rulesSourceName]?.loading) {
dispatch(fetchPromRulesAction({ rulesSourceName }));
}
if ((force || !rulerRules[rulesSourceName]?.loading) && dataSourceConfig.rulerConfig) {
dispatch(fetchRulerRulesAction({ rulesSourceName }));
}
})
);
};
}

Expand Down

0 comments on commit 169a1c5

Please sign in to comment.