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.

Sign up for GitHub

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

[ML] Explain log rate spikes: adds table sorting #137110

Merged
merged 8 commits into from
Jul 26, 2022
Merged

[ML] Explain log rate spikes: adds table sorting #137110

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
26bfb56
add sorting for table
alvarezmelissa87 Jul 25, 2022
9d8fd4b
update analysis refresh button copy
alvarezmelissa87 Jul 25, 2022
7035d72
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Jul 25, 2022
55a84b3
add info tooltips for p-value, log rate, and impact columns
alvarezmelissa87 Jul 25, 2022
d582535
update tooltip copy
alvarezmelissa87 Jul 25, 2022
0e1e584
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Jul 25, 2022
2ce5f5a
update types and simplify tooltip content
alvarezmelissa87 Jul 26, 2022
4e9b521
make sort case insensitive
alvarezmelissa87 Jul 26, 2022
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: 4 additions & 1 deletion x-pack/packages/ml/aiops_components/src/progress_controls/progress_controls.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ export function ProgressControls({
<EuiFlexItem grow={false}>
{!isRunning && (
<EuiButton size="s" onClick={onRefresh}>
<FormattedMessage id="xpack.aiops.refreshButtonTitle" defaultMessage="Refresh" />
<FormattedMessage
id="xpack.aiops.rerunAnalysisButtonTitle"
defaultMessage="Rerun analysis"
/>
</EuiButton>
)}
{isRunning && (
Expand Down
85 changes: 56 additions & 29 deletions x-pack/plugins/aiops/public/components/spike_analysis_table/spike_analysis_table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
*/

import React, { FC, useCallback, useMemo, useState } from 'react';
import { EuiBadge, EuiBasicTable, EuiBasicTableColumn } from '@elastic/eui';
import { EuiBadge, EuiBasicTable, EuiBasicTableColumn, EuiTableSortingType, EuiToolTip } from '@elastic/eui';
import { sortBy } from 'lodash';

import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import type { ChangePoint } from '@kbn/ml-agg-utils';

import { MiniHistogram } from '../mini_histogram';
Expand All @@ -18,6 +21,8 @@ const PAGINATION_SIZE_OPTIONS = [5, 10, 20, 50];
const noDataText = i18n.translate('xpack.aiops.correlations.correlationsTable.noDataText', {
defaultMessage: 'No data',
});
const DEFAULT_SORT_FIELD = 'pValue';
const DEFAULT_SORT_DIRECTION = 'asc';

interface SpikeAnalysisTableProps {
changePoints: ChangePoint[];
Expand All @@ -38,6 +43,8 @@ export const SpikeAnalysisTable: FC<SpikeAnalysisTableProps> = ({
}) => {
const [pageIndex, setPageIndex] = useState(0);
const [pageSize, setPageSize] = useState(10);
const [sortField, setSortField] = useState(DEFAULT_SORT_FIELD);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest to use const [sortField, setSortField] = useState<keyof ChangePoint>(DEFAULT_SORT_FIELD); to limit allowed fields to ChangePoint attributes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call - updated in 2ce5f5a

const [sortDirection, setSortDirection] = useState(DEFAULT_SORT_DIRECTION);

const columns: Array<EuiBasicTableColumn<ChangePoint>> = [
{
Expand All @@ -60,14 +67,15 @@ export const SpikeAnalysisTable: FC<SpikeAnalysisTableProps> = ({
{
field: 'pValue',
name: (
<>
{i18n.translate(
'xpack.aiops.correlations.failedTransactions.correlationsTable.logRateLabel',
{
defaultMessage: 'Log rate',
}
)}
</>
<EuiToolTip position="top" content={i18n.translate(
'xpack.aiops.correlations.failedTransactions.correlationsTable.logRateColumnTooltip',
{ defaultMessage: 'Visual representation of amount of impact given field name and value have on metric change point.' }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{ defaultMessage: 'Visual representation of amount of impact given field name and value have on metric change point.' }
{ defaultMessage: 'Visual representation of amount of impact given field name and value have on the message rate difference.' }

)}>
<FormattedMessage
id='xpack.aiops.correlations.failedTransactions.correlationsTable.logRateLabel'
defaultMessage='Log rate'
/>
</EuiToolTip>
),
render: (_, { histogram, fieldName, fieldValue }) => {
return histogram ? (
Expand All @@ -78,56 +86,75 @@ export const SpikeAnalysisTable: FC<SpikeAnalysisTableProps> = ({
},
{
field: 'pValue',
name: 'p-value',
name: (
<EuiToolTip position="top" content={i18n.translate(
'xpack.aiops.correlations.failedTransactions.correlationsTable.pValueColumnTooltip',
{ defaultMessage: 'For statistically significant changes in the distribution of values, indicates how extreme the change is; lower values indicate greater change.' }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{ defaultMessage: 'For statistically significant changes in the distribution of values, indicates how extreme the change is; lower values indicate greater change.' }
{ defaultMessage: 'For statistically significant changes in the frequency of values, indicates how extreme the change is; lower values indicate greater change.' }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Updated all in d582535

)}>
<FormattedMessage
id='xpack.aiops.correlations.failedTransactions.correlationsTable.pValueLabel'
defaultMessage='p-value'
/>
</EuiToolTip>
),
render: (pValue: number) => pValue.toPrecision(3),
sortable: true,
},
{
field: 'pValue',
name: (
<>
{i18n.translate(
'xpack.aiops.correlations.failedTransactions.correlationsTable.impactLabel',
{
defaultMessage: 'Impact',
}
)}
</>
<EuiToolTip position="top" content={i18n.translate(
'xpack.aiops.correlations.failedTransactions.correlationsTable.impactLabelColumnTooltip',
{ defaultMessage: 'Indicates level of impact of given field name and value on metric change point.' }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{ defaultMessage: 'Indicates level of impact of given field name and value on metric change point.' }
{ defaultMessage: 'Indicates level of impact of given field name and value on the message rate difference.' }

)}>
<FormattedMessage
id='xpack.aiops.correlations.failedTransactions.correlationsTable.impactLabel'
defaultMessage='Impact'
/>
</EuiToolTip>
),
render: (_, { pValue }) => {
const label = getFailedTransactionsCorrelationImpactLabel(pValue);
return label ? <EuiBadge color={label.color}>{label.impact}</EuiBadge> : null;
},
sortable: true,
sortable: false,
peteharverson marked this conversation as resolved.
Show resolved Hide resolved
},
];

const onChange = useCallback((tableSettings) => {
const { index, size } = tableSettings.page;
const { field, direction } = tableSettings.sort;

setPageIndex(index);
setPageSize(size);
setSortField(field);
setSortDirection(direction);
}, []);

const { pagination, pageOfItems } = useMemo(() => {
const { pagination, pageOfItems, sorting } = useMemo(() => {
const pageStart = pageIndex * pageSize;

const itemCount = changePoints?.length ?? 0;

let items = changePoints ?? [];
items = sortBy(changePoints, (item: ChangePoint) => item[sortField as keyof ChangePoint]);
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 type is set on the first assignment the sorting line could be simpler:

let items: ChangePoint[] = changePoints ?? [];
items = sortBy(changePoints, (item) => item[sortField]);

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use case-insensitive sort?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 2ce5f5a

items = sortDirection === 'asc' ? items : items.reverse();

return {
pageOfItems: changePoints
// Temporary default sorting by ascending pValue until we add native table sorting
?.sort((a, b) => {
return (a?.pValue ?? 1) - (b?.pValue ?? 0);
})
.slice(pageStart, pageStart + pageSize),
pageOfItems: items.slice(pageStart, pageStart + pageSize),
pagination: {
pageIndex,
pageSize,
totalItemCount: itemCount,
pageSizeOptions: PAGINATION_SIZE_OPTIONS,
},
sorting: {
sort: {
field: sortField,
direction: sortDirection,
},
},
};
}, [pageIndex, pageSize, changePoints]);
}, [pageIndex, pageSize, sortField, sortDirection, changePoints]);

return (
<EuiBasicTable
Expand All @@ -139,7 +166,7 @@ export const SpikeAnalysisTable: FC<SpikeAnalysisTableProps> = ({
pagination={pagination}
loading={loading}
error={error}
// sorting={sorting}
sorting={sorting as EuiTableSortingType<ChangePoint>}
rowProps={(changePoint) => {
return {
onClick: () => {
Expand Down