Skip to content
Merged
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
5 changes: 3 additions & 2 deletions src/components/Loader/Loader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ const b = cn('ydb-loader');

interface LoaderProps {
size?: LoaderSize;
className?: string;
}

export const Loader = ({size = 'm'}: LoaderProps) => {
export const Loader = ({size = 'm', className}: LoaderProps) => {
return (
<div className={b()}>
<div className={b(null, className)}>
<KitLoader size={size} />
</div>
);
Expand Down
49 changes: 45 additions & 4 deletions src/components/MetricChart/MetricChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,19 @@ import ChartKit, {settings} from '@gravity-ui/chartkit';
import type {IResponseError} from '../../types/api/error';
import type {TimeFrame} from '../../utils/timeframes';
import {useAutofetcher} from '../../utils/hooks';

import {COLORS} from '../../utils/versions';
import {cn} from '../../utils/cn';

import {Loader} from '../Loader';
import {ResponseError} from '../Errors/ResponseError';

import type {ChartOptions, MetricDescription, PreparedMetricsData} from './types';
import type {
ChartOptions,
MetricDescription,
OnChartDataStatusChange,
PreparedMetricsData,
} from './types';
import {convertResponse} from './convertReponse';
import {getDefaultDataFormatter} from './getDefaultDataFormatter';
import {getChartData} from './getChartData';
Expand Down Expand Up @@ -102,6 +108,15 @@ interface DiagnosticsChartProps {
width?: number;

chartOptions?: ChartOptions;

onChartDataStatusChange?: OnChartDataStatusChange;

/**
* YAGR charts don't render correctly inside not visible elements\
* So if chart is used inside component with 'display:none', it will be empty, when visibility change\
* Pass isChartVisible prop to ensure proper chart render
*/
isChartVisible?: boolean;
}

export const MetricChart = ({
Expand All @@ -112,6 +127,8 @@ export const MetricChart = ({
width = 400,
height = width / 1.5,
chartOptions,
onChartDataStatusChange,
isChartVisible,
}: DiagnosticsChartProps) => {
const mounted = useRef(false);

Expand All @@ -127,6 +144,20 @@ export const MetricChart = ({
initialChartState,
);

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should resolve problem with changable callback. It seems that such a combination of useState and useEffect may cause a lot of unneeded re-renders. Let's try to fix it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Replaced it with useEffect, dependant on chart state. It should not cause additional rerenders, since component already rerenders on reducer state change

if (error) {
return onChartDataStatusChange?.('error');
}
if (loading && !wasLoaded) {
return onChartDataStatusChange?.('loading');
}
if (!loading && wasLoaded) {
return onChartDataStatusChange?.('success');
}

return undefined;
}, [loading, wasLoaded, error, onChartDataStatusChange]);

const fetchChartData = useCallback(
async (isBackground: boolean) => {
dispatch(setChartDataLoading());
Expand All @@ -146,7 +177,9 @@ export const MetricChart = ({
});

// Hack to prevent setting value to state, if component unmounted
if (!mounted.current) return;
if (!mounted.current) {
return;
}

// In some cases error could be in response with 200 status code
// It happens when request is OK, but chart data cannot be returned due to some reason
Expand All @@ -155,10 +188,14 @@ export const MetricChart = ({
const preparedData = convertResponse(response, metrics);
dispatch(setChartData(preparedData));
} else {
dispatch(setChartError({statusText: response.error}));
const err = {statusText: response.error};

throw err;
}
} catch (err) {
if (!mounted.current) return;
if (!mounted.current) {
return;
}

dispatch(setChartError(err as IResponseError));
}
Expand All @@ -175,6 +212,10 @@ export const MetricChart = ({
return <Loader />;
}

if (!isChartVisible) {
return null;
}

return (
<div className={b('chart')}>
<ChartKit type="yagr" data={convertedData} />
Expand Down
2 changes: 1 addition & 1 deletion src/components/MetricChart/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export type {MetricDescription, Metric, ChartOptions} from './types';
export * from './types';
export {MetricChart} from './MetricChart';
3 changes: 3 additions & 0 deletions src/components/MetricChart/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,6 @@ export type ChartDataType = 'ms' | 'size';
export interface ChartOptions {
dataType?: ChartDataType;
}

export type ChartDataStatus = 'loading' | 'success' | 'error';
export type OnChartDataStatusChange = (newStatus: ChartDataStatus) => void;
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import {TenantDashboard} from '../TenantDashboard/TenantDashboard';
import {defaultDashboardConfig} from './defaultDashboardConfig';

export const DefaultOverviewContent = () => {
return <TenantDashboard charts={defaultDashboardConfig} />;
};
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {type ChartConfig, TenantDashboard} from './TenantDashboard/TenantDashboard';
import i18n from './i18n';
import type {ChartConfig} from '../TenantDashboard/TenantDashboard';
import i18n from '../i18n';

const defaultDashboardConfig: ChartConfig[] = [
export const defaultDashboardConfig: ChartConfig[] = [
{
title: i18n('charts.queries-per-second'),
metrics: [
Expand Down Expand Up @@ -44,7 +44,3 @@ const defaultDashboardConfig: ChartConfig[] = [
},
},
];

export const DefaultDashboard = () => {
return <TenantDashboard charts={defaultDashboardConfig} />;
};
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {AdditionalNodesProps} from '../../../../../types/additionalProps';
import {CpuDashboard} from './CpuDashboard';
import {TenantDashboard} from '../TenantDashboard/TenantDashboard';
import {cpuDashboardConfig} from './cpuDashboardConfig';
import {TopNodesByLoad} from './TopNodesByLoad';
import {TopNodesByCpu} from './TopNodesByCpu';
import {TopShards} from './TopShards';
Expand All @@ -13,7 +14,7 @@ interface TenantCpuProps {
export function TenantCpu({path, additionalNodesProps}: TenantCpuProps) {
return (
<>
<CpuDashboard />
<TenantDashboard charts={cpuDashboardConfig} />
<TopNodesByLoad path={path} additionalNodesProps={additionalNodesProps} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are all this components in one LoadingContainer and their visibility depends on dashboardLoading ?
I would better move loading logic to TenantDashboard. If you do so, you won't need LoadingContainer at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

uplot charts don't render correctly in components with display:none. Fixed this issue with a hack, removed additional containers

<TopNodesByCpu path={path} additionalNodesProps={additionalNodesProps} />
<TopShards path={path} />
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {type ChartConfig, TenantDashboard} from '../TenantDashboard/TenantDashboard';
import type {ChartConfig} from '../TenantDashboard/TenantDashboard';
import i18n from '../i18n';

const cpuDashboardConfig: ChartConfig[] = [
export const cpuDashboardConfig: ChartConfig[] = [
{
title: i18n('charts.cpu-usage'),
metrics: [
Expand All @@ -12,7 +12,3 @@ const cpuDashboardConfig: ChartConfig[] = [
],
},
];

export const CpuDashboard = () => {
return <TenantDashboard charts={cpuDashboardConfig} />;
};
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import {useState} from 'react';
import {StringParam, useQueryParam} from 'use-query-params';

import {cn} from '../../../../../utils/cn';
import type {TimeFrame} from '../../../../../utils/timeframes';
import {useSetting, useTypedSelector} from '../../../../../utils/hooks';
import {DISPLAY_CHARTS_IN_DB_DIAGNOSTICS_KEY} from '../../../../../utils/constants';
import {useTypedSelector} from '../../../../../utils/hooks';
import {TimeFrameSelector} from '../../../../../components/TimeFrameSelector/TimeFrameSelector';
import {
type ChartOptions,
MetricChart,
type MetricDescription,
type ChartDataStatus,
} from '../../../../../components/MetricChart';

import './TenantDashboard.scss';
Expand All @@ -29,39 +30,56 @@ interface TenantDashboardProps {
}

export const TenantDashboard = ({charts}: TenantDashboardProps) => {
const [isDashboardHidden, setIsDashboardHidden] = useState<boolean>(true);

const [timeFrame = '1h', setTimeframe] = useQueryParam('timeframe', StringParam);

const {autorefresh} = useTypedSelector((state) => state.schema);

const [chartsEnabled] = useSetting(DISPLAY_CHARTS_IN_DB_DIAGNOSTICS_KEY);
// Refetch data only if dashboard successfully loaded
const shouldRefresh = autorefresh && !isDashboardHidden;

if (!chartsEnabled) {
return null;
}
/**
* Charts should be hidden, if they are not enabled:
* 1. GraphShard is not enabled
* 2. ydb version does not have /viewer/json/render endpoint (400, 404, CORS error, etc.)
*
* If at least one chart successfully loaded, dashboard should be shown
* @link https://github.com/ydb-platform/ydb-embedded-ui/issues/659
* @todo disable only for specific errors ('GraphShard is not enabled') after ydb-stable-24 is generally used
*/
const handleChartDataStatusChange = (chartStatus: ChartDataStatus) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that you are interested only in success finish. So, lets rename this to let's say handleChartLoadingSucceeded and execute it only in one case - if chart was successfully loaded.

Copy link
Member Author

Choose a reason for hiding this comment

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

I made quite a universal solution, to make it possible to extend it later - hide charts only on specific errors.

Currently they are hidden on every error, including timeouts when node is not healthy, some internal problems, etc. The reason - versions without render endpoint don't return specific error.

Further charts should be hidden only on specific errors, when GraphShard is not enabled - added todo not to forger.

if (chartStatus === 'success') {
setIsDashboardHidden(false);
}
};

// If there is only one chart, display it with full width
const chartWidth = charts.length === 1 ? CHART_WIDTH_FULL : CHART_WIDTH;
const chartHeight = CHART_WIDTH / 1.5;

const renderContent = () => {
return charts.map((chartConfig) => {
const chartId = chartConfig.metrics.map(({target}) => target).join('&');
return (
<MetricChart
key={chartConfig.metrics.map(({target}) => target).join('&')}
key={chartId}
title={chartConfig.title}
metrics={chartConfig.metrics}
timeFrame={timeFrame as TimeFrame}
chartOptions={chartConfig.options}
autorefresh={autorefresh}
autorefresh={shouldRefresh}
width={chartWidth}
height={chartHeight}
onChartDataStatusChange={handleChartDataStatusChange}
isChartVisible={!isDashboardHidden}
/>
);
});
};

return (
<div className={b(null)}>
<div className={b(null)} style={{display: isDashboardHidden ? 'none' : undefined}}>
<div className={b('controls')}>
<TimeFrameSelector value={timeFrame as TimeFrame} onChange={setTimeframe} />
</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {MemoryDashboard} from './MemoryDashboard';
import {TenantDashboard} from '../TenantDashboard/TenantDashboard';
import {memoryDashboardConfig} from './memoryDashboardConfig';
import {TopNodesByMemory} from './TopNodesByMemory';

interface TenantMemoryProps {
Expand All @@ -8,7 +9,7 @@ interface TenantMemoryProps {
export function TenantMemory({path}: TenantMemoryProps) {
return (
<>
<MemoryDashboard />
<TenantDashboard charts={memoryDashboardConfig} />
<TopNodesByMemory path={path} />
</>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {type ChartConfig, TenantDashboard} from '../TenantDashboard/TenantDashboard';
import type {ChartConfig} from '../TenantDashboard/TenantDashboard';
import i18n from '../i18n';

const memoryDashboardConfig: ChartConfig[] = [
export const memoryDashboardConfig: ChartConfig[] = [
{
title: i18n('charts.memory-usage'),
metrics: [
Expand All @@ -15,7 +15,3 @@ const memoryDashboardConfig: ChartConfig[] = [
},
},
];

export const MemoryDashboard = () => {
return <TenantDashboard charts={memoryDashboardConfig} />;
};
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import cn from 'bem-cn-lite';
import {useCallback} from 'react';
import {useDispatch} from 'react-redux';

Expand All @@ -17,12 +16,11 @@ import {HealthcheckDetails} from './Healthcheck/HealthcheckDetails';
import {MetricsCards, type TenantMetrics} from './MetricsCards/MetricsCards';
import {TenantStorage} from './TenantStorage/TenantStorage';
import {TenantMemory} from './TenantMemory/TenantMemory';
import {DefaultDashboard} from './DefaultDashboard';
import {DefaultOverviewContent} from './DefaultOverviewContent/DefaultOverviewContent';
import {useHealthcheck} from './useHealthcheck';

import './TenantOverview.scss';

const b = cn('tenant-overview');
import {b} from './utils';

interface TenantOverviewProps {
tenantName: string;
Expand Down Expand Up @@ -141,7 +139,7 @@ export function TenantOverview({
);
}
default: {
return <DefaultDashboard />;
return <DefaultOverviewContent />;
}
}
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type {ReactNode} from 'react';
import cn from 'bem-cn-lite';

import DataTable from '@gravity-ui/react-data-table';
import type {DataTableProps} from '@gravity-ui/react-data-table';
Expand All @@ -11,8 +10,7 @@ import {
import type {IResponseError} from '../../../../types/api/error';
import {TableSkeleton} from '../../../../components/TableSkeleton/TableSkeleton';
import {ResponseError} from '../../../../components/Errors/ResponseError';

const b = cn('tenant-overview');
import {b} from './utils';

interface TenantOverviewTableLayoutProps<T> extends Omit<DataTableProps<T>, 'theme'> {
title: ReactNode;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import cn from 'bem-cn-lite';

import InfoViewer from '../../../../../components/InfoViewer/InfoViewer';
import {ProgressViewer} from '../../../../../components/ProgressViewer/ProgressViewer';
import {formatStorageValues} from '../../../../../utils/dataFormatters/dataFormatters';
import {getSizeWithSignificantDigits} from '../../../../../utils/bytesParsers';

import {TenantDashboard} from '../TenantDashboard/TenantDashboard';

import '../TenantOverview.scss';

import {StorageDashboard} from './StorageDashboard';
import {storageDashboardConfig} from './storageDashboardConfig';
import {TopTables} from './TopTables';
import {TopGroups} from './TopGroups';

const b = cn('tenant-overview');
import {b} from '../utils';

export interface TenantStorageMetrics {
blobStorageUsed?: number;
Expand Down Expand Up @@ -61,9 +60,10 @@ export function TenantStorage({tenantName, metrics}: TenantStorageProps) {
),
},
];

return (
<>
<StorageDashboard />
<TenantDashboard charts={storageDashboardConfig} />
<InfoViewer className={b('storage-info')} title="Storage details" info={info} />
<TopTables path={tenantName} />
<TopGroups tenant={tenantName} />
Expand Down
Loading