Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 41 additions & 10 deletions web/src/components/MetricsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ import { MonitoringProvider } from '../contexts/MonitoringContext';
import { DataTestIDs } from './data-test';
import { useMonitoring } from '../hooks/useMonitoring';
import { useMonitoringNamespace } from './hooks/useMonitoringNamespace';
import { useMultiNamespace } from './hooks/useMultiNamespace';
import { MultiNamespaceSelector } from './multi-namespace-selector';
import { useSearchParams } from 'react-router';

// Stores information about the currently focused query input
Expand Down Expand Up @@ -952,7 +954,8 @@ const Query: FC<{
index: number;
customDatasource?: CustomDataSource;
units: GraphUnits;
}> = ({ index, customDatasource, units }) => {
namespaceOverride?: string | string[];
}> = ({ index, customDatasource, units, namespaceOverride }) => {
const { t } = useTranslation(process.env.I18N_NAMESPACE);
const { plugin } = useMonitoring();

Expand Down Expand Up @@ -1069,7 +1072,7 @@ const Query: FC<{
<QueryTable
index={index}
customDatasource={customDatasource}
namespace={activeNamespace}
namespace={namespaceOverride ?? activeNamespace}
units={units}
/>
</DataListContent>
Expand All @@ -1082,7 +1085,14 @@ const QueryBrowserWrapper: FC<{
customDataSource: CustomDataSource;
customDatasourceError: boolean;
units: GraphUnits;
}> = ({ customDataSourceName, customDataSource, customDatasourceError, units }) => {
namespaceOverride?: string | string[];
}> = ({
customDataSourceName,
customDataSource,
customDatasourceError,
units,
namespaceOverride,
}) => {
const { t } = useTranslation(process.env.I18N_NAMESPACE);
const { plugin } = useMonitoring();
const [activeNamespace] = useActiveNamespace();
Expand Down Expand Up @@ -1209,6 +1219,7 @@ const QueryBrowserWrapper: FC<{
<QueryBrowser
customDataSource={customDataSource}
disabledSeries={disabledSeries}
namespace={namespaceOverride}
queries={queryStrings}
units={units}
showStackedControl
Expand Down Expand Up @@ -1263,10 +1274,11 @@ const RunQueriesButton: FC = () => {
);
};

const QueriesList: FC<{ customDatasource?: CustomDataSource; units: GraphUnits }> = ({
customDatasource,
units,
}) => {
const QueriesList: FC<{
customDatasource?: CustomDataSource;
units: GraphUnits;
namespaceOverride?: string | string[];
}> = ({ customDatasource, units, namespaceOverride }) => {
const { plugin } = useMonitoring();
const count = useSelector(
(state: MonitoringState) => getObserveState(plugin, state).queryBrowser.queries.length,
Expand All @@ -1282,6 +1294,7 @@ const QueriesList: FC<{ customDatasource?: CustomDataSource; units: GraphUnits }
key={reversedIndex}
customDatasource={customDatasource}
units={units}
namespaceOverride={namespaceOverride}
/>
);
})}
Expand Down Expand Up @@ -1339,7 +1352,8 @@ const MetricsPage_: FC = () => {
const [units, setUnits] = useQueryParam(QueryParams.Units, StringParam);
const [customDataSourceName] = useQueryParam(QueryParams.Datasource, StringParam);
const { namespace, setNamespace } = useMonitoringNamespace();
const { displayNamespaceSelector } = useMonitoring();
const { displayNamespaceSelector, useMetricsTenancy } = useMonitoring();
const multiNs = useMultiNamespace();

const dispatch = useDispatch();

Expand Down Expand Up @@ -1456,6 +1470,18 @@ const MetricsPage_: FC = () => {
</ListPageHeader>
<PageSection hasBodyWrapper={false}>
<Stack hasGutter>
{useMetricsTenancy && (
<StackItem>
<MultiNamespaceSelector
accessibleNamespaces={multiNs.accessibleNamespaces}
selectedNamespaces={multiNs.selectedNamespaces}
allSelected={multiNs.allSelected}
toggleNamespace={multiNs.toggleNamespace}
toggleAll={multiNs.toggleAll}
deselectAll={multiNs.deselectAll}
/>
</StackItem>
)}
<StackItem>
<ToggleGraph />
</StackItem>
Expand All @@ -1465,6 +1491,7 @@ const MetricsPage_: FC = () => {
customDataSourceName={customDataSourceName}
customDatasourceError={customDatasourceError}
units={units as GraphUnits}
namespaceOverride={useMetricsTenancy ? multiNs.namespaceForQuery : undefined}
/>
</StackItem>
<StackItem>
Expand All @@ -1482,7 +1509,11 @@ const MetricsPage_: FC = () => {
</Flex>
</StackItem>
<StackItem>
<QueriesList customDatasource={customDataSource} units={units as GraphUnits} />
<QueriesList
customDatasource={customDataSource}
units={units as GraphUnits}
namespaceOverride={useMetricsTenancy ? multiNs.namespaceForQuery : undefined}
/>
</StackItem>
</Stack>
</PageSection>
Expand Down Expand Up @@ -1516,7 +1547,7 @@ export const MpCmoDevMetricsPage: FC = () => {

type QueryTableProps = {
index: number;
namespace?: string;
namespace?: string | string[];
customDatasource?: CustomDataSource;
units: GraphUnits;
};
Expand Down
95 changes: 95 additions & 0 deletions web/src/components/hooks/useMultiNamespace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import {
K8sResourceKind,
useActiveNamespace,
useK8sWatchResource,
} from '@openshift-console/dynamic-plugin-sdk';
import { ALL_NAMESPACES_KEY } from '../utils';
import { ProjectModel } from '../console/models';

/**
* Hook to manage multi-namespace selection for the monitoring plugin.
* Returns the list of accessible namespaces, selected namespaces, and a setter.
* When "All Namespaces" is toggled, all accessible namespaces are selected.
*/
export const useMultiNamespace = () => {
const [activeNamespace] = useActiveNamespace();

const [namespaceResources] = useK8sWatchResource<K8sResourceKind[]>({
kind: ProjectModel.kind,
isList: true,
optional: true,
});

const accessibleNamespaces = useMemo(
() =>
(namespaceResources || [])
.map((ns) => ns.metadata?.name)
.filter(Boolean)
.sort() as string[],
[namespaceResources],
);

const [selectedNamespaces, setSelectedNamespaces] = useState<string[]>([]);
const [allSelected, setAllSelected] = useState(false);
const isInitialized = useRef(false);

useEffect(() => {
if (
!isInitialized.current &&
activeNamespace &&
activeNamespace !== ALL_NAMESPACES_KEY &&
selectedNamespaces.length === 0
) {
setSelectedNamespaces([activeNamespace]);
isInitialized.current = true;
}
}, [activeNamespace]);

const toggleNamespace = useCallback((ns: string) => {
setAllSelected(false);
setSelectedNamespaces((prev) =>
prev.includes(ns) ? prev.filter((n) => n !== ns) : [...prev, ns],
);
}, []);

const selectAll = useCallback(() => {
setAllSelected(true);
setSelectedNamespaces(accessibleNamespaces);
}, [accessibleNamespaces]);
Comment on lines +56 to +59

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Selection becomes stale when accessibleNamespaces changes after "All Namespaces" is selected.

If a new project is created after the user selects "All Namespaces", the selectedNamespaces array won't include the new namespace, but allSelected remains true. This creates inconsistent state where the UI indicates all namespaces are selected, but some are missing.

Consider adding a sync effect:

♻️ Suggested fix
+  useEffect(() => {
+    if (allSelected) {
+      setSelectedNamespaces(accessibleNamespaces);
+    }
+  }, [accessibleNamespaces, allSelected]);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@web/src/components/hooks/useMultiNamespace.ts` around lines 56 - 59, When
"All Namespaces" is selected via the selectAll callback (which sets
allSelected=true and selectedNamespaces=accessibleNamespaces), the
selectedNamespaces can become stale if accessibleNamespaces later changes; add a
sync effect that watches accessibleNamespaces and allSelected and, when
allSelected is true, updates selectedNamespaces to the new accessibleNamespaces
so the UI stays consistent (refer to selectAll, allSelected, setAllSelected,
setSelectedNamespaces, and accessibleNamespaces).


const deselectAll = useCallback(() => {
setAllSelected(false);
setSelectedNamespaces([]);
}, []);

const toggleAll = useCallback(() => {
if (allSelected) {
deselectAll();
} else {
selectAll();
}
}, [allSelected, selectAll, deselectAll]);

const namespaceForQuery = useMemo(() => {
if (selectedNamespaces.length === 0) {
return activeNamespace === ALL_NAMESPACES_KEY ? undefined : activeNamespace;
}
if (selectedNamespaces.length === 1) {
return selectedNamespaces[0];
}
return selectedNamespaces;
}, [selectedNamespaces, activeNamespace]);

return {
accessibleNamespaces,
selectedNamespaces,
allSelected,
toggleNamespace,
selectAll,
deselectAll,
toggleAll,
setSelectedNamespaces,
namespaceForQuery,
};
};
Loading