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

Open/Closed filter for observability alerts page #99217

Merged
merged 16 commits into from
May 25, 2021
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ kibana_vars=(
xpack.reporting.roles.allow
xpack.reporting.roles.enabled
xpack.rollup.enabled
xpack.ruleRegistry.unsafe.write.enabled
xpack.ruleRegistry.write.enabled
xpack.searchprofiler.enabled
xpack.security.audit.enabled
xpack.security.audit.appender.type
Expand Down
40 changes: 39 additions & 1 deletion x-pack/plugins/observability/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ This will only enable the UI for these pages. In order to have alert data indexe
you'll need to enable writing in the [Rule Registry plugin](../rule_registry/README.md):

```yaml
xpack.ruleRegistry.unsafe.write.enabled: true
xpack.ruleRegistry.write.enabled: true
```

When both of the these are set to `true`, your alerts should show on the alerts page.
Expand Down Expand Up @@ -47,3 +47,41 @@ HTML coverage report can be found in target/coverage/jest after tests have run.
```bash
open target/coverage/jest/index.html
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding this! 👏

```

## API integration testing

API tests are separated in two suites:

- a basic license test suite
- a trial license test suite (the equivalent of gold+)

This requires separate test servers and test runners.

### Basic

```
# Start server
node scripts/functional_tests_server --config x-pack/test/observability_api_integration/basic/config.ts

# Run tests
node scripts/functional_test_runner --config x-pack/test/observability_api_integration/basic/config.ts
```

The API tests for "basic" are located in `x-pack/test/observability_api_integration/basic/tests`.

### Trial

```
# Start server
node scripts/functional_tests_server --config x-pack/test/observability_api_integration/trial/config.ts

# Run tests
node scripts/functional_test_runner --config x-pack/test/observability_api_integration/trial/config.ts
```

The API tests for "trial" are located in `x-pack/test/observability_api_integration/trial/tests`.

### API test tips

- For debugging access Elasticsearch on http://localhost:9220` (elastic/changeme)
- To update snapshots append `--updateSnapshots` to the functional_test_runner command
4 changes: 4 additions & 0 deletions x-pack/plugins/observability/common/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,9 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import * as t from 'io-ts';

export type Maybe<T> = T | null | undefined;

export const alertStatusRt = t.union([t.literal('all'), t.literal('open'), t.literal('closed')]);
Copy link
Member

Choose a reason for hiding this comment

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

This isn't really in scope of this ticket directly, but I'm nervous that we're still using "open" and "closed" to mean "active" and "recovered" because they feel like very different concepts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only place "active" and "recovered" is used is on the control itself. kibana.rac.alert.status can be "open" or "closed". They aren't different in the context of observability alerts. @katrin-freihofner do you think we should change the text in the toggle control to be "open"/"closed"?

Copy link
Member

Choose a reason for hiding this comment

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

I think we want to keep active and recovered as the language here (imo), but that the concepts are different in the underlying data so we'll eventually not want them linked. We've talked about it in 3 or 4 places but these things are hard to keep track of. But there will eventually be a difference between workflow status and active/recovered, since one is controlled by humans and the other is controlled by data + conditions of the rule.

Does the alert schema not have any other status fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Oh thanks. I think we may need to use alerts that have this value set:

kibana.rac.alert.end: the ISO timestamp of the time at which the alert recovered.

I just now realized we probably want a way to have the investigation window (date range) apply to recovery date and not just start date ... but that's for later :P

Copy link
Member

Choose a reason for hiding this comment

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

Let's leave it for now and address it as it comes up, but it will be important not to forget because I'm not 100% sure we will set that status to closed on resolution ... or if the semantics of when it gets set to closed / open could change and be surprising to us later.

export type AlertStatus = t.TypeOf<typeof alertStatusRt>;
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,16 @@ export function AlertsTable(props: AlertsTableProps) {

return active ? (
<EuiIconTip
content={i18n.translate('xpack.observability.alertsTable.statusActiveDescription', {
defaultMessage: 'Active',
content={i18n.translate('xpack.observability.alertsTable.statusOpenDescription', {
defaultMessage: 'Open',
})}
color="danger"
type="alert"
/>
) : (
<EuiIconTip
content={i18n.translate('xpack.observability.alertsTable.statusRecoveredDescription', {
defaultMessage: 'Recovered',
content={i18n.translate('xpack.observability.alertsTable.statusClosedDescription', {
defaultMessage: 'Closed',
})}
type="check"
/>
Expand Down
43 changes: 33 additions & 10 deletions x-pack/plugins/observability/public/pages/alerts/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,23 @@ import {
EuiFlexItem,
EuiLink,
EuiPageTemplate,
EuiSpacer,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { useHistory } from 'react-router-dom';
import { format, parse } from 'url';
import {
ALERT_START,
EVENT_ACTION,
ALERT_STATUS,
RULE_ID,
RULE_NAME,
} from '@kbn/rule-data-utils/target/technical_field_names';
import React from 'react';
import { useHistory } from 'react-router-dom';
import { format, parse } from 'url';
import {
ParsedTechnicalFields,
parseTechnicalFields,
} from '../../../../rule_registry/common/parse_technical_fields';
import type { AlertStatus } from '../../../common/typings';
import { asDuration, asPercent } from '../../../common/utils/formatters';
import { ExperimentalBadge } from '../../components/shared/experimental_badge';
import { useFetcher } from '../../hooks/use_fetcher';
Expand All @@ -37,6 +39,7 @@ import type { ObservabilityAPIReturnType } from '../../services/call_observabili
import { getAbsoluteDateRange } from '../../utils/date';
import { AlertsSearchBar } from './alerts_search_bar';
import { AlertsTable } from './alerts_table';
import { StatusFilter } from './status_filter';

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

Expand All @@ -57,7 +60,7 @@ export function AlertsPage({ routeParams }: AlertsPageProps) {
const { prepend } = core.http.basePath;
const history = useHistory();
const {
query: { rangeFrom = 'now-15m', rangeTo = 'now', kuery = '' },
query: { rangeFrom = 'now-15m', rangeTo = 'now', kuery = '', status = 'open' },
} = routeParams;

// In a future milestone we'll have a page dedicated to rule management in
Expand All @@ -81,6 +84,7 @@ export function AlertsPage({ routeParams }: AlertsPageProps) {
start,
end,
kuery,
status,
},
},
}).then((alerts) => {
Expand Down Expand Up @@ -108,15 +112,24 @@ export function AlertsPage({ routeParams }: AlertsPageProps) {
},
})
: undefined,
active: parsedFields[EVENT_ACTION] !== 'close',
active: parsedFields[ALERT_STATUS] !== 'closed',
start: new Date(parsedFields[ALERT_START]!).getTime(),
};
});
});
},
[kuery, observabilityRuleTypeRegistry, rangeFrom, rangeTo]
[kuery, observabilityRuleTypeRegistry, rangeFrom, rangeTo, status]
);

function setStatusFilter(value: AlertStatus) {
const nextSearchParams = new URLSearchParams(history.location.search);
nextSearchParams.set('status', value);
history.push({
...history.location,
search: nextSearchParams.toString(),
});
}

return (
<EuiPageTemplate
pageHeader={{
Expand Down Expand Up @@ -179,9 +192,19 @@ export function AlertsPage({ routeParams }: AlertsPageProps) {
}}
/>
</EuiFlexItem>
<EuiFlexItem>
<AlertsTable items={topAlerts ?? []} />
</EuiFlexItem>
<EuiSpacer size="s" />
<EuiFlexGroup direction="column">
<EuiFlexItem>
<EuiFlexGroup justifyContent="flexEnd">
<EuiFlexItem grow={false}>
<StatusFilter status={status} onChange={setStatusFilter} />
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexItem>
<EuiFlexItem>
<AlertsTable items={topAlerts ?? []} />
</EuiFlexItem>
</EuiFlexGroup>
</EuiFlexGroup>
</EuiPageTemplate>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import React, { ComponentProps, useState } from 'react';
import type { AlertStatus } from '../../../common/typings';
import { StatusFilter } from './status_filter';

type Args = ComponentProps<typeof StatusFilter>;

export default {
title: 'app/Alerts/StatusFilter',
component: StatusFilter,
argTypes: {
onChange: { action: 'change' },
},
};

export function Example({ onChange }: Args) {
const [status, setStatus] = useState<AlertStatus>('open');

return (
<StatusFilter
status={status}
onChange={(value) => {
setStatus(value);
onChange(value);
}}
/>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { render } from '@testing-library/react';
import React from 'react';
import type { AlertStatus } from '../../../common/typings';
import { StatusFilter } from './status_filter';

describe('StatusFilter', () => {
describe('render', () => {
it('renders', () => {
const onChange = jest.fn();
const status: AlertStatus = 'all';
const props = { onChange, status };

expect(() => render(<StatusFilter {...props} />)).not.toThrowError();
});

(['all', 'open', 'closed'] as AlertStatus[]).map((status) => {
describe(`when clicking the ${status} button`, () => {
it('calls the onChange callback with "${status}"', () => {
const onChange = jest.fn();
const props = { onChange, status };

const { getByTestId } = render(<StatusFilter {...props} />);
const button = getByTestId(`StatusFilter ${status} button`);

button.click();

expect(onChange).toHaveBeenCalledWith(status);
});
});
});
});
});
56 changes: 56 additions & 0 deletions x-pack/plugins/observability/public/pages/alerts/status_filter.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { EuiFilterButton, EuiFilterGroup } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import type { AlertStatus } from '../../../common/typings';

export interface StatusFilterProps {
status: AlertStatus;
onChange: (value: AlertStatus) => void;
}

export function StatusFilter({ status = 'open', onChange }: StatusFilterProps) {
return (
<EuiFilterGroup
aria-label={i18n.translate('xpack.observability.alerts.statusFilterAriaLabel', {
defaultMessage: 'Filter alerts by open and closed status',
})}
>
<EuiFilterButton
smith marked this conversation as resolved.
Show resolved Hide resolved
data-test-subj="StatusFilter open button"
hasActiveFilters={status === 'open'}
onClick={() => onChange('open')}
withNext={true}
>
{i18n.translate('xpack.observability.alerts.statusFilter.openButtonLabel', {
defaultMessage: 'Open',
})}
</EuiFilterButton>
<EuiFilterButton
data-test-subj="StatusFilter closed button"
hasActiveFilters={status === 'closed'}
onClick={() => onChange('closed')}
withNext={true}
>
{i18n.translate('xpack.observability.alerts.statusFilter.closedButtonLabel', {
defaultMessage: 'Closed',
})}
</EuiFilterButton>
<EuiFilterButton
data-test-subj="StatusFilter all button"
hasActiveFilters={status === 'all'}
onClick={() => onChange('all')}
>
{i18n.translate('xpack.observability.alerts.statusFilter.allButtonLabel', {
defaultMessage: 'All',
})}
</EuiFilterButton>
</EuiFilterGroup>
);
}
2 changes: 2 additions & 0 deletions x-pack/plugins/observability/public/routes/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { jsonRt } from './json_rt';
import { AlertsPage } from '../pages/alerts';
import { CasesPage } from '../pages/cases';
import { ExploratoryViewPage } from '../components/shared/exploratory_view';
import { alertStatusRt } from '../../common/typings';

export type RouteParams<T extends keyof typeof routes> = DecodeParams<typeof routes[T]['params']>;

Expand Down Expand Up @@ -105,6 +106,7 @@ export const routes = {
rangeFrom: t.string,
rangeTo: t.string,
kuery: t.string,
status: alertStatusRt,
refreshPaused: jsonRt.pipe(t.boolean),
refreshInterval: jsonRt.pipe(t.number),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,29 @@
*/
import { ALERT_UUID, TIMESTAMP } from '@kbn/rule-data-utils/target/technical_field_names';
import { RuleDataClient } from '../../../../rule_registry/server';
import { kqlQuery, rangeQuery } from '../../utils/queries';
import type { AlertStatus } from '../../../common/typings';
import { kqlQuery, rangeQuery, alertStatusQuery } from '../../utils/queries';

export async function getTopAlerts({
ruleDataClient,
start,
end,
kuery,
size,
status,
}: {
ruleDataClient: RuleDataClient;
start: number;
end: number;
kuery?: string;
size: number;
status: AlertStatus;
}) {
const response = await ruleDataClient.getReader().search({
body: {
query: {
bool: {
filter: [...rangeQuery(start, end), ...kqlQuery(kuery)],
filter: [...rangeQuery(start, end), ...kqlQuery(kuery), ...alertStatusQuery(status)],
},
},
fields: ['*'],
Expand Down
Loading