Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(Diagnostics): display db feature flags #1229

Merged
merged 15 commits into from
Sep 9, 2024
8 changes: 8 additions & 0 deletions src/containers/Tenant/Diagnostics/Configs/Configs.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.ydb-diagnostics-configs {
&__icon-touched {
line-height: 1;
cursor: default !important;

color: var(--g-color-text-secondary);
}
}
149 changes: 149 additions & 0 deletions src/containers/Tenant/Diagnostics/Configs/Configs.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
import React from 'react';

import {PersonPencil} from '@gravity-ui/icons';
import type {Column} from '@gravity-ui/react-data-table';
import {Icon, Popover, Switch} from '@gravity-ui/uikit';
import {StringParam, useQueryParam} from 'use-query-params';

import {ResizeableDataTable} from '../../../../components/ResizeableDataTable/ResizeableDataTable';
import {Search} from '../../../../components/Search';
import {TableWithControlsLayout} from '../../../../components/TableWithControlsLayout/TableWithControlsLayout';
import {setFeatureFlagsFilter, tenantApi} from '../../../../store/reducers/tenant/tenant';
import type {TConfigFeatureFlag} from '../../../../types/api/featureFlags';
import {cn} from '../../../../utils/cn';
import {DEFAULT_TABLE_SETTINGS} from '../../../../utils/constants';
import {useTypedDispatch, useTypedSelector} from '../../../../utils/hooks';

import i18n from './i18n';

import './Configs.scss';

const FEATURE_FLAGS_COLUMNS_WIDTH_LS_KEY = 'featureFlagsColumnsWidth';

const b = cn('ydb-diagnostics-configs');

const columns: Column<TConfigFeatureFlag>[] = [
{
name: ' ',
sareyu marked this conversation as resolved.
Show resolved Hide resolved
render: ({row}) => (
<React.Fragment>
{row.Current && (
<Popover
content={i18n('flag-touched')}
className={b('icon-touched')}
placement="left"
>
<Icon data={PersonPencil} />
</Popover>
)}
</React.Fragment>
sareyu marked this conversation as resolved.
Show resolved Hide resolved
),
width: 36,
sortable: false,
resizeable: false,
},
{
name: i18n('td-feature-flag'),
sareyu marked this conversation as resolved.
Show resolved Hide resolved
render: ({row}) => (row.Current ? <b>{row.Name}</b> : row.Name),
width: 400,
sortable: true,
sortAccessor: ({Current, Name}) => {
return Number(!Current) + Name.toLowerCase();
},
},
{
name: i18n('td-default'),
render: ({row}) => {
switch (row.Default) {
case true:
return i18n('enabled');
case false:
return i18n('disabled');
default:
return '-';
}
},
width: 100,
sortable: false,
resizeable: false,
},
{
name: i18n('td-current'),
render: ({row}) => <Switch disabled checked={(row.Current ?? row.Default) || false} />,
width: 100,
sortable: false,
resizeable: false,
},
];

interface Props {
path: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

path is not used

database: string;
}

export const Configs = ({database}: Props) => {
const dispatch = useTypedDispatch();
const [search, setSearch] = useQueryParam('search', StringParam);
const {currentData = [], isFetching, isError} = tenantApi.useGetClusterConfigQuery({database});
const featureFlagsFilter = useTypedSelector(
(state) => state.tenant.featureFlagsFilter,
)?.toLocaleLowerCase();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need this state in the store?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes. in some other tabs search filter is saved between tab navigation

Copy link
Collaborator

Choose a reason for hiding this comment

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

But current solution is not good, we have two sources of truth. On other tabs behavior is different. For example, on Top queries tab the search is saved only in the store, and on Nodes and Storage tabs the search is saved only in query params.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suggested to have same UX in such cases - sync btw store and url, others agreed.
Why do you think it's not good?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

discussed in pm, fixed


// sync store and query filter
React.useEffect(() => {
if (search) {
dispatch(setFeatureFlagsFilter(search));
} else if (featureFlagsFilter) {
setSearch(featureFlagsFilter, 'replaceIn');
}
}, []);

const onChange = (value: string) => {
dispatch(setFeatureFlagsFilter(value));
setSearch(value || undefined, 'replaceIn');
};

const featureFlags = featureFlagsFilter
? currentData.filter((item) => item.Name.toLocaleLowerCase().includes(featureFlagsFilter))
: currentData;

const renderContent = () => {
if (isError) {
return i18n('error-msg');
}

if (!featureFlags.length) {
return i18n(featureFlagsFilter ? 'search-empty' : 'no-data');
}

return (
<ResizeableDataTable
columnsWidthLSKey={FEATURE_FLAGS_COLUMNS_WIDTH_LS_KEY}
columns={columns}
data={featureFlags}
settings={DEFAULT_TABLE_SETTINGS}
/>
);
};

const renderControls = () => {
return (
<React.Fragment>
<Search
value={featureFlagsFilter}
onChange={onChange}
placeholder={i18n('search-placeholder')}
/>
</React.Fragment>
);
};

return (
<TableWithControlsLayout>
<TableWithControlsLayout.Controls>{renderControls()}</TableWithControlsLayout.Controls>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not inline controls here?

<TableWithControlsLayout.Table loading={isFetching}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's move this and fetching and filtering data to the FeatureFlagsList.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why? Now Configs component includes logic and FeatureFlagsList is stupid (and is not very necessary imho). After moving logic Config will be almost redundant

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

discussed in pm, fixed

{renderContent()}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's extract this to another component

</TableWithControlsLayout.Table>
</TableWithControlsLayout>
);
};
14 changes: 14 additions & 0 deletions src/containers/Tenant/Diagnostics/Configs/i18n/en.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"td-feature-flag": "Feature flag",
"td-default": "Default",
"td-current": "Current",

"enabled": "Enabled",
"disabled": "Disabled",
"flag-touched": "Flag is changed",

"error-msg": "Error while loading configs",
"search-placeholder": "Search by feature flag",
"search-empty": "Empty search result",
"no-data": "No data"
}
7 changes: 7 additions & 0 deletions src/containers/Tenant/Diagnostics/Configs/i18n/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import {registerKeysets} from '../../../../../utils/i18n';

import en from './en.json';

const COMPONENT = 'ydb-diagnostics-configs';

export default registerKeysets(COMPONENT, {en});
4 changes: 4 additions & 0 deletions src/containers/Tenant/Diagnostics/Diagnostics.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {SchemaViewer} from '../Schema/SchemaViewer/SchemaViewer';
import {TenantTabsGroups} from '../TenantPages';
import {isDatabaseEntityType} from '../utils/schema';

import {Configs} from './Configs/Configs';
import {Consumers} from './Consumers';
import Describe from './Describe/Describe';
import DetailedOverview from './DetailedOverview/DetailedOverview';
Expand Down Expand Up @@ -131,6 +132,9 @@ function Diagnostics(props: DiagnosticsProps) {
case TENANT_DIAGNOSTICS_TABS_IDS.partitions: {
return <Partitions path={path} database={tenantName} />;
}
case TENANT_DIAGNOSTICS_TABS_IDS.configs: {
return <Configs path={path} database={tenantName} />;
}
default: {
return <div>No data...</div>;
}
Expand Down
6 changes: 6 additions & 0 deletions src/containers/Tenant/Diagnostics/DiagnosticsPages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ const partitions = {
title: 'Partitions',
};

const configs = {
id: TENANT_DIAGNOSTICS_TABS_IDS.configs,
title: 'Configs',
};

export const ASYNC_REPLICATION_PAGES = [overview, tablets, describe];

export const DATABASE_PAGES = [
Expand All @@ -81,6 +86,7 @@ export const DATABASE_PAGES = [
storage,
network,
describe,
configs,
];

export const TABLE_PAGES = [overview, schema, topShards, nodes, graph, tablets, hotKeys, describe];
Expand Down
10 changes: 10 additions & 0 deletions src/services/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {CapabilitiesResponse} from '../types/api/capabilities';
import type {TClusterInfo} from '../types/api/cluster';
import type {TComputeInfo} from '../types/api/compute';
import type {DescribeConsumerResult} from '../types/api/consumer';
import type {TConfigs} from '../types/api/featureFlags';
import type {HealthCheckAPIResponse} from '../types/api/healthcheck';
import type {JsonHotKeysResponse} from '../types/api/hotkeys';
import type {MetaCluster, MetaClusters, MetaTenants} from '../types/api/meta';
Expand Down Expand Up @@ -119,6 +120,15 @@ export class YdbEmbeddedAPI extends AxiosWrapper {
{concurrentId: concurrentId || `getClusterInfo`, requestConfig: {signal}},
);
}
getClusterConfig(database?: string, {concurrentId, signal}: AxiosOptions = {}) {
return this.get<TConfigs>(
this.getPath('/viewer/feature_flags'),
{
database,
},
{concurrentId, requestConfig: {signal}},
);
}
getNodeInfo(id?: string | number, {concurrentId, signal}: AxiosOptions = {}) {
return this.get<TEvSystemStateResponse>(
this.getPath('/viewer/json/sysinfo?enums=true'),
Expand Down
1 change: 1 addition & 0 deletions src/store/reducers/tenant/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export const TENANT_DIAGNOSTICS_TABS_IDS = {
graph: 'graph',
consumers: 'consumers',
partitions: 'partitions',
configs: 'configs',
} as const;

export const TENANT_SUMMARY_TABS_IDS = {
Expand Down
26 changes: 24 additions & 2 deletions src/store/reducers/tenant/tenant.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,21 @@ const slice = createSlice({
setMetricsTab: (state, action: PayloadAction<TenantMetricsTab>) => {
state.metricsTab = action.payload;
},
setFeatureFlagsFilter: (state, action: PayloadAction<string>) => {
state.featureFlagsFilter = action.payload;
},
},
});

export default slice.reducer;
export const {setTenantPage, setQueryTab, setDiagnosticsTab, setSummaryTab, setMetricsTab} =
slice.actions;
export const {
setTenantPage,
setQueryTab,
setDiagnosticsTab,
setSummaryTab,
setMetricsTab,
setFeatureFlagsFilter,
} = slice.actions;

export const tenantApi = api.injectEndpoints({
endpoints: (builder) => ({
Expand All @@ -62,6 +71,19 @@ export const tenantApi = api.injectEndpoints({
},
providesTags: ['All'],
}),
getClusterConfig: builder.query({
queryFn: async ({database}: {database?: string}, {signal}) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
queryFn: async ({database}: {database?: string}, {signal}) => {
queryFn: async ({database}: {database: string}, {signal}) => {

try {
// FIXME: pass database param when supported, remove filter
const res = await window.api.getClusterConfig(undefined, {signal});
const db = res.Databases.filter((item) => item.Name === database)[0];

return {data: db.FeatureFlags};
} catch (error) {
return {error};
}
},
}),
}),
overrideExisting: 'throw',
});
1 change: 1 addition & 0 deletions src/store/reducers/tenant/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,5 @@ export interface TenantState {
diagnosticsTab?: TenantDiagnosticsTab;
summaryTab?: TenantSummaryTab;
metricsTab?: TenantMetricsTab;
featureFlagsFilter?: string;
}
14 changes: 14 additions & 0 deletions src/types/api/featureFlags.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
export interface TConfigFeatureFlag {
Name: string;
Current?: boolean;
Default?: boolean;
}

export interface TConfigDb {
Name: string;
FeatureFlags: TConfigFeatureFlag[];
}

export interface TConfigs {
Databases: TConfigDb[];
}
Copy link
Member

Choose a reason for hiding this comment

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

Not a request, but for you information: types are called TSomething and ESomething to match with names from Swagger or backend types, you may not follow the same pattern for your own types

Loading