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

[Observability] Remove outdated top_alerts route and related types #107579

Merged
merged 24 commits into from
Aug 12, 2021
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
7f8d440
Remove outdated top_alerts route and related types
weltenwort Aug 3, 2021
a6f2c40
Remove tests for deleted code
weltenwort Aug 3, 2021
6bf826a
Remove test for deleted API
weltenwort Aug 3, 2021
de546f2
Merge branch 'master' into obs-rac-remove-unused-api
weltenwort Aug 3, 2021
3a22b8f
Remove reference to deleted type
weltenwort Aug 4, 2021
564b740
Merge branch 'master' into obs-rac-remove-unused-api
weltenwort Aug 4, 2021
c0c794e
Remove unused translations
weltenwort Aug 4, 2021
8ccbead
Remove unused mock from story
weltenwort Aug 4, 2021
2917b5e
Merge branch 'master' into obs-rac-remove-unused-api
weltenwort Aug 4, 2021
76c6964
Merge branch 'master' into obs-rac-remove-unused-api
weltenwort Aug 9, 2021
78a7b2f
Remove no-op alerts page story for now
weltenwort Aug 9, 2021
29d9a16
Merge branch 'master' into obs-rac-remove-unused-api
weltenwort Aug 10, 2021
c88d4cd
Remove unsafe type assertions
weltenwort Aug 11, 2021
f9965c5
Merge branch 'master' into obs-rac-remove-unused-api
weltenwort Aug 11, 2021
def6987
Factor out alert field type
weltenwort Aug 11, 2021
a1267b1
Compile kbn-io-ts-utils for the browser as well
weltenwort Aug 11, 2021
492378c
Avoid deep import which doesn't work cross-platform
weltenwort Aug 11, 2021
714020a
Revert "Avoid deep import which doesn't work cross-platform"
weltenwort Aug 12, 2021
0d60ce7
Revert "Compile kbn-io-ts-utils for the browser as well"
weltenwort Aug 12, 2021
9475988
Revert "Factor out alert field type"
weltenwort Aug 12, 2021
d66a474
Revert "Remove unsafe type assertions"
weltenwort Aug 12, 2021
77631aa
Remove unsafe type assertions (again)
weltenwort Aug 12, 2021
4bbb744
Merge branch 'master' into obs-rac-remove-unused-api
weltenwort Aug 12, 2021
ad01a55
Merge branch 'master' into obs-rac-remove-unused-api
weltenwort Aug 12, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
* 2.0.
*/

import { StoryContext } from '@storybook/react';
import React, { ComponentType } from 'react';
import { __IntlProvider as IntlProvider } from '@kbn/i18n/react';
import { MemoryRouter } from 'react-router-dom';
Expand All @@ -18,23 +17,16 @@ import {
import { PluginContext, PluginContextValue } from '../../context/plugin_context';
import { createObservabilityRuleTypeRegistryMock } from '../../rules/observability_rule_type_registry_mock';
import { createCallObservabilityApi } from '../../services/call_observability_api';
import type { ObservabilityAPIReturnType } from '../../services/call_observability_api/types';
import { apmAlertResponseExample, dynamicIndexPattern } from './example_data';

interface PageArgs {
items: ObservabilityAPIReturnType<'GET /api/observability/rules/alerts/top'>;
}
import { dynamicIndexPattern } from './example_data';

export default {
title: 'app/Alerts',
component: AlertsPage,
decorators: [
(Story: ComponentType, { args: { items = [] } }: StoryContext) => {
(Story: ComponentType) => {
createCallObservabilityApi(({
get: async (endpoint: string) => {
if (endpoint === '/api/observability/rules/alerts/top') {
return items;
} else if (endpoint === '/api/observability/rules/alerts/dynamic_index_pattern') {
if (endpoint === '/api/observability/rules/alerts/dynamic_index_pattern') {
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 know this doesn't fix the story but it also doesn't break it further. But IMHO it would exceed the scope of the PR to implement a working t-grid mock.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the story isn't doing doing anything we should just delete it.

return dynamicIndexPattern;
}
},
Expand Down Expand Up @@ -83,16 +75,12 @@ export default {
],
};

export function Example(_args: PageArgs) {
export function Example() {
return (
<AlertsPage routeParams={{ query: { rangeFrom: 'now-15m', rangeTo: 'now', kuery: '' } }} />
);
}
Example.args = {
items: apmAlertResponseExample,
} as PageArgs;

export function EmptyState(_args: PageArgs) {
export function EmptyState() {
return <AlertsPage routeParams={{ query: {} }} />;
}
EmptyState.args = { items: [] } as PageArgs;
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@

import { ALERT_UUID } from '@kbn/rule-data-utils';
import React, { ComponentType } from 'react';
import type { TopAlertResponse } from '../';
import { KibanaContextProvider } from '../../../../../../../src/plugins/kibana_react/public';
import { PluginContext, PluginContextValue } from '../../../context/plugin_context';
import { createObservabilityRuleTypeRegistryMock } from '../../../rules/observability_rule_type_registry_mock';
import { apmAlertResponseExample } from '../example_data';
import { AlertsFlyout } from './';

interface Args {
alerts: TopAlertResponse[];
alerts: Array<Record<string, unknown>>;
}

export default {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,16 @@ import {
} from '@kbn/rule-data-utils/target/technical_field_names';
import moment from 'moment-timezone';
import React, { useMemo } from 'react';
import type { TopAlert, TopAlertResponse } from '../';
import type { TopAlert } from '../';
import { useKibana, useUiSetting } from '../../../../../../../src/plugins/kibana_react/public';
import { asDuration } from '../../../../common/utils/formatters';
import type { ObservabilityRuleTypeRegistry } from '../../../rules/create_observability_rule_type_registry';
import { decorateResponse } from '../decorate_response';
import { parseAlert } from '../parse_alert';
import { SeverityBadge } from '../severity_badge';

type AlertsFlyoutProps = {
alert?: TopAlert;
alerts?: TopAlertResponse[];
alerts?: Array<Record<string, unknown>>;
Copy link
Member Author

Choose a reason for hiding this comment

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

TopAlertResponse was only every indirectly defined via the return type of the server-side route registration. The removal of that route meant the type went away as well. The data fetching now happens via the timeline search strategy, which doesn't provide much type safety around the data, which is reflected by this Record<string, unknown>. The object is decoded using parseAlert anyway (which is the successor to decorateResponse).

Copy link
Contributor

Choose a reason for hiding this comment

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

Even if we don't have much type safety now, would it make sense to extract Record<string, unknown> to a separate type? That way if/when we get better typings it's a simpler change

isInApp?: boolean;
observabilityRuleTypeRegistry: ObservabilityRuleTypeRegistry;
selectedAlertId?: string;
Expand All @@ -59,7 +59,8 @@ export function AlertsFlyout({
const { http } = services;
const prepend = http?.basePath.prepend;
const decoratedAlerts = useMemo(() => {
return decorateResponse(alerts ?? [], observabilityRuleTypeRegistry);
const parseObservabilityAlert = parseAlert(observabilityRuleTypeRegistry);
return (alerts ?? []).map(parseObservabilityAlert);
}, [alerts, observabilityRuleTypeRegistry]);

let alertData = alert;
Expand Down
167 changes: 0 additions & 167 deletions x-pack/plugins/observability/public/pages/alerts/alerts_table.tsx

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { EuiButtonIcon, EuiDataGridColumn } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import styled from 'styled-components';

import React, { Suspense, useState } from 'react';
import React, { Suspense, useMemo, useState } from 'react';
import {
ALERT_DURATION,
ALERT_SEVERITY_LEVEL,
Expand All @@ -30,8 +30,8 @@ import type {

import { getRenderCellValue } from './render_cell_value';
import { usePluginContext } from '../../hooks/use_plugin_context';
import { decorateResponse } from './decorate_response';
import { LazyAlertsFlyout } from '../..';
import { parseAlert } from './parse_alert';

interface AlertsTableTGridProps {
indexName: string;
Expand Down Expand Up @@ -122,6 +122,10 @@ export function AlertsTableTGrid(props: AlertsTableTGridProps) {
const handleFlyoutClose = () => setFlyoutAlert(undefined);
const { timelines } = useKibana<{ timelines: TimelinesUIStart }>().services;

const parseObservabilityAlert = useMemo(() => parseAlert(observabilityRuleTypeRegistry), [
observabilityRuleTypeRegistry,
]);

const leadingControlColumns = [
{
id: 'expand',
Expand All @@ -137,11 +141,7 @@ export function AlertsTableTGrid(props: AlertsTableTGridProps) {
},
rowCellRender: ({ data }: ActionProps) => {
const dataFieldEs = data.reduce((acc, d) => ({ ...acc, [d.field]: d.value }), {});
const decoratedAlerts = decorateResponse(
[dataFieldEs] ?? [],
observabilityRuleTypeRegistry
);
const alert = decoratedAlerts[0];
const alert = parseObservabilityAlert(dataFieldEs);
return (
<EuiButtonIcon
size="s"
Expand All @@ -158,11 +158,7 @@ export function AlertsTableTGrid(props: AlertsTableTGridProps) {
headerCellRender: () => null,
rowCellRender: ({ data }: ActionProps) => {
const dataFieldEs = data.reduce((acc, d) => ({ ...acc, [d.field]: d.value }), {});
const decoratedAlerts = decorateResponse(
[dataFieldEs] ?? [],
observabilityRuleTypeRegistry
);
const alert = decoratedAlerts[0];
const alert = parseObservabilityAlert(dataFieldEs);
return (
<EuiButtonIcon
size="s"
Expand Down

This file was deleted.

3 changes: 0 additions & 3 deletions x-pack/plugins/observability/public/pages/alerts/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,12 @@ import { ExperimentalBadge } from '../../components/shared/experimental_badge';
import { useBreadcrumbs } from '../../hooks/use_breadcrumbs';
import { usePluginContext } from '../../hooks/use_plugin_context';
import { RouteParams } from '../../routes';
import type { ObservabilityAPIReturnType } from '../../services/call_observability_api/types';
import { AlertsSearchBar } from './alerts_search_bar';
import { AlertsTableTGrid } from './alerts_table_t_grid';
import { StatusFilter } from './status_filter';
import { useFetcher } from '../../hooks/use_fetcher';
import { callObservabilityApi } from '../../services/call_observability_api';

export type TopAlertResponse = ObservabilityAPIReturnType<'GET /api/observability/rules/alerts/top'>[number];

export interface TopAlert {
fields: ParsedTechnicalFields;
start: number;
Expand Down
Loading