Skip to content
Open
19 changes: 12 additions & 7 deletions shell-ui/src/alerts/AlertProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { useQuery } from 'react-query';
import { Loader } from '@scality/core-ui/dist/components/loader/Loader.component';
import { useAuth } from '../auth/AuthProvider';
import { getAlerts } from './services/alertManager';
import { AlertContext } from './alertContext';
/**
Expand All @@ -18,13 +19,17 @@ export default function AlertProvider({
alertManagerUrl: string;
children: React.ReactNode;
}) {
const query = useQuery('activeAlerts', () => getAlerts(alertManagerUrl), {
// refetch the alerts every 10 seconds
refetchInterval: 30000,
// TODO manage this refresh interval globally
// avoid stucking at the hard loading state before alertmanager is ready
initialData: [],
});
const { getToken } = useAuth();
const query = useQuery(
['activeAlerts'],
async () => getAlerts(alertManagerUrl, await getToken()),
{
refetchInterval: 30000,
// TODO manage this refresh interval globally
// avoid stucking at the hard loading state before alertmanager is ready
initialData: [],
},
);
return (
<AlertContext.Provider value={{ ...query }}>
{query.status === 'loading' && (
Expand Down
10 changes: 10 additions & 0 deletions shell-ui/src/alerts/alertHooks.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ import AlertProvider from './AlertProvider';
import { useHighestSeverityAlerts } from './alertHooks';
import { afterAll, beforeAll, jest } from '@jest/globals';
import { QueryClientProvider } from '../QueryClientProvider';

jest.mock('../auth/AuthProvider', () => ({
useAuth: () => ({
userData: {
token: 'test-token',
},
getToken: () => Promise.resolve('test-token'),
}),
}));

const testService = 'http://10.0.0.1/api/alertmanager';

const VOLUME_DEGRADED_ALERT = {
Expand Down
6 changes: 4 additions & 2 deletions shell-ui/src/alerts/services/alertManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@ export type AlertLabels = {
selectors?: string[];
[labelName: string]: string;
};
export function getAlerts(alertManagerUrl: string) {
return fetch(alertManagerUrl + '/api/v2/alerts')
export function getAlerts(alertManagerUrl: string, token: string) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The token parameter is typed as string but it can be undefined (since userData?.token may be undefined). The type should be string | undefined to match the actual usage in AlertProvider.tsx line 26.

— Claude Code

return fetch(alertManagerUrl + '/api/v2/alerts', {
headers: { Authorization: `Bearer ${token}` },
})
.then((r) => {
if (r.ok) {
return r.json();
Expand Down
13 changes: 8 additions & 5 deletions ui/src/FederableApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { applyMiddleware, compose, createStore, Store } from 'redux';
import createSagaMiddleware from 'redux-saga';
import 'regenerator-runtime/runtime';
import App from './containers/App';
import PrometheusAuthProvider from './containers/PrometheusAuthProvider';
import { authErrorAction } from './ducks/app/authError';
import { setApiConfigAction } from './ducks/config';
import { setHistory as setReduxHistory } from './ducks/history';
Expand Down Expand Up @@ -125,11 +126,13 @@ export default function FederableApp(props: FederatedAppProps) {
>
<Provider store={store}>
<AppConfigProvider>
<ToastProvider>
<RouterWithBaseName>
<App />
</RouterWithBaseName>
</ToastProvider>
<PrometheusAuthProvider>
<ToastProvider>
<RouterWithBaseName>
<App />
</RouterWithBaseName>
</ToastProvider>
</PrometheusAuthProvider>
</AppConfigProvider>
</Provider>
</ShellHooksProvider>
Expand Down
18 changes: 18 additions & 0 deletions ui/src/components/DashboardAlerts.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,24 @@ describe('the dashboard alerts sub-panel', () => {
expect(queryByTestId('critical-alert-badge')).not.toBeInTheDocument();
expect(queryByTestId('view-all-link')).not.toBeInTheDocument();
});
test('should display a warning banner when alerts fail to fetch', () => {
(useAlertLibrary as any).mockImplementation(() => ({
getPlatformAlertSelectors: () => ({
alertname: ['ClusterAtRisk', 'ClusterDegraded'],
}),
}));
(useAlerts as any).mockImplementation(() => ({
alerts: [],
error: new Error('Alert manager responded with 401'),
}));
const { getByText } = render(<DashboardAlerts />);
expect(
getByText('Monitoring information unavailable'),
).toBeInTheDocument();
expect(
getByText('Some data can not be well retrieved'),
).toBeInTheDocument();
});
test('should redirect to alert page with View All link', () => {
(useAlerts as any).mockImplementation(() => ({
alerts: alerts,
Expand Down
60 changes: 40 additions & 20 deletions ui/src/components/DashboardAlerts.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { spacing, Text, TextBadge } from '@scality/core-ui';
import { Banner, Icon, spacing, Text, TextBadge } from '@scality/core-ui';
import { Box } from '@scality/core-ui/dist/next';
import { useMemo } from 'react';
import { useIntl } from 'react-intl';
Expand Down Expand Up @@ -61,25 +61,45 @@ const DashboardAlerts = () => {
const totalAlerts = criticalAlerts.length + warningAlerts.length;
return (
<AlertsContainer>
<div>
<Text isEmphazed>
{intl.formatMessage({
id: 'platform_active_alerts',
})}
</Text>
<TextBadge
variant="infoPrimary"
data-testid="all-alert-badge"
text={totalAlerts}
/>
</div>
{totalAlerts === 0 ? (
<Text variant="Smaller" color="textSecondary">
{intl.formatMessage({
id: 'no_active_alerts',
})}
</Text>
) : (
<Box display="flex" alignItems="center" gap={spacing.r8}>
<div>
<Text isEmphazed>
{intl.formatMessage({
id: 'platform_active_alerts',
})}
</Text>
<TextBadge
variant="infoPrimary"
data-testid="all-alert-badge"
text={totalAlerts}
/>
{totalAlerts === 0 && (
<div>
<Text variant="Smaller" color="textSecondary">
{intl.formatMessage({
id: 'no_active_alerts',
})}
</Text>
</div>
)}
</div>
{alerts.error && (
<div style={{ flex: 1 }}>
<Banner
variant="warning"
icon={<Icon name="Exclamation-circle" />}
title={intl.formatMessage({
id: 'monitoring_information_unavailable',
})}
>
{intl.formatMessage({
id: 'some_data_not_retrieved',
})}
</Banner>
</div>
)}
</Box>
{totalAlerts === 0 ? null : (
<Box pr={24}>
<BadgesContainer>
<div>
Expand Down
35 changes: 24 additions & 11 deletions ui/src/containers/AlertPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { STATUS_CRITICAL, STATUS_HEALTH, STATUS_WARNING } from '../constants';
import { useUserAccessRight } from '../hooks';
import { compareHealth } from '../services/utils';
import { useAlerts } from './AlertProvider';
import type { Alert } from '../services/alertUtils';
import type { QueryStatus } from 'react-query';
import { useBasenameRelativeNavigate } from '@scality/module-federation';

const AlertPageHeaderContainer = styled.div`
Expand Down Expand Up @@ -94,8 +96,8 @@ const getAlertStatus = (numbersOfCritical, numbersOfWarning) =>
numbersOfCritical > 0
? STATUS_CRITICAL
: numbersOfWarning > 0
? STATUS_WARNING
: STATUS_HEALTH;
? STATUS_WARNING
: STATUS_HEALTH;

function AlertPageHeader({
activeAlerts,
Expand All @@ -118,7 +120,7 @@ function AlertPageHeader({
<Title>
<AlertStatusIcon>
<StatusWrapper status={alertStatus}>
<StatusIcon status={alertStatus} name="Alert" entity='Alerts' />
<StatusIcon status={alertStatus} name="Alert" entity="Alerts" />
</StatusWrapper>
</AlertStatusIcon>
<>
Expand Down Expand Up @@ -163,9 +165,14 @@ function AlertPageHeader({
);
}

type ActiveAlertTabProps = {
columns: Record<string, unknown>[];
data: Alert[];
status: QueryStatus;
};

const ActiveAlertTab = React.memo(
// @ts-expect-error - FIXME when you are working on it
({ columns, data }) => {
({ columns, data, status }: ActiveAlertTabProps) => {
const sortTypes = React.useMemo(() => {
return {
severity: (row1, row2) => {
Expand Down Expand Up @@ -197,6 +204,7 @@ const ActiveAlertTab = React.memo(
data={data}
defaultSortingKey={DEFAULT_SORTING_KEY}
sortTypes={sortTypes}
status={status}
entityName={{
en: {
singular: 'active alert',
Expand All @@ -219,10 +227,12 @@ const ActiveAlertTab = React.memo(
</Table>
);
},
(a, b) => {
// compare the alert only on id and severity
// @ts-expect-error - FIXME when you are working on it
return isEqual(a.columns, b.columns) && isEqualAlert(a.data, b.data);
(prevProps: ActiveAlertTabProps, nextProps: ActiveAlertTabProps) => {
return (
isEqual(prevProps.columns, nextProps.columns) &&
isEqualAlert(prevProps.data, nextProps.data) &&
prevProps.status === nextProps.status
);
},
);
export default function AlertPage() {
Expand Down Expand Up @@ -295,8 +305,11 @@ export default function AlertPage() {
/>
</AppContainer.OverallSummary>
<AppContainer.MainContent>
{/* @ts-expect-error - FIXME when you are working on it */}
<ActiveAlertTab data={leafAlerts} columns={columns} />
<ActiveAlertTab
data={leafAlerts}
columns={columns}
status={alerts.status}
/>
</AppContainer.MainContent>
</AppContainer>
);
Expand Down
28 changes: 28 additions & 0 deletions ui/src/containers/PrometheusAuthProvider.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { ReactNode, useEffect } from 'react';
import { useAuth } from './PrivateRoute';
import { setHeaders } from '../services/prometheus/api';

// Temporary solution: The Prometheus API client is initialized with the URL in the saga
// (setApiConfig), but the auth token is not available at that point. Rather than modifying
// the saga (which will be removed soon), this provider sets the auth header on the shared
// Prometheus client once the token is available, and blocks rendering until it's ready.
// TODO: Remove this provider once the saga is removed.

export default function PrometheusAuthProvider({
children,
}: {
children: ReactNode;
}) {
const { userData } = useAuth();
const token = userData?.token;

useEffect(() => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Race condition: useEffect runs after render, so when token first becomes available, children render (and can trigger Prometheus API calls) before setHeaders has executed. Use useLayoutEffect instead to ensure headers are set before children mount.

— Claude Code

if (token) {
setHeaders({ Authorization: `Bearer ${token}` });
}
}, [token]);

if (!token) return null;

return <>{children}</>;
}
2 changes: 1 addition & 1 deletion ui/src/containers/VolumePageRSP.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export const VolumePageRSP = (props) => {
const alertsVolume = useAlerts({
persistentvolumeclaim: PVCName,
});
const alertlist = alertsVolume && alertsVolume.alerts;
const alertlist = alertsVolume?.alerts ?? [];
const criticalAlerts = alertlist.filter(
(alert) => alert.severity === 'critical',
);
Expand Down
6 changes: 1 addition & 5 deletions ui/src/hooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,9 @@ export const useVolumesWithAlerts = (nodeName?: string) => {
// @ts-expect-error - FIXME when you are working on it
getVolumeListData(state, null, nodeName),
);
//This forces alerts to have been fetched at least once (watchdog alert should be present)
// before rendering volume list
// TODO enhance this using useAlerts status
if (!alerts || alerts.length === 0) return [];
// @ts-expect-error - FIXME when you are working on it
const volumeListWithStatus = volumeListData.map((volume) => {
const volumeAlerts = filterAlerts(alerts, {
const volumeAlerts = filterAlerts(alerts || [], {
persistentvolumeclaim: volume.persistentvolumeclaim,
});
// For the unbound volume, the health status should be none.
Expand Down
20 changes: 12 additions & 8 deletions ui/src/services/ApiClient.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,24 @@
import axios from 'axios';

class ApiClient {
constructor({ apiUrl, headers = {} }) {
// @ts-expect-error - FIXME when you are working on it
headers: Record<string, string>;
settings: { baseURL: string };

constructor({
apiUrl,
headers = {},
}: {
apiUrl: string;
headers?: Record<string, string>;
}) {
this.headers = headers;
// @ts-expect-error - FIXME when you are working on it
this.settings = {
baseURL: apiUrl,
};
}

setHeaders = (headers) => {
// @ts-expect-error - FIXME when you are working on it
this.headers = headers;
setHeaders = (headers: Record<string, string>) => {
this.headers = { ...this.headers, ...headers };
};

async get(endpoint, params = {}, opts = {}) {
Expand Down Expand Up @@ -72,12 +78,10 @@ class ApiClient {
try {
const response = await axios({
method,
// @ts-expect-error - FIXME when you are working on it
headers: { ...this.headers, ...headers },
params,
url: endpoint,
data: payload,
// @ts-expect-error - FIXME when you are working on it
...this.settings,
});
return response.data;
Expand Down
6 changes: 6 additions & 0 deletions ui/src/services/prometheus/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,12 @@ export function initialize(apiUrl: string) {
prometheusApiClient = new ApiClient({ apiUrl });
}

export function setHeaders(headers: Record<string, string>) {
if (prometheusApiClient) {
prometheusApiClient.setHeaders(headers);
}
}

export function getAlerts() {
if (prometheusApiClient) {
return prometheusApiClient.get('/api/v1/alerts');
Expand Down
Loading