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

[ML] Anomaly Detection: when no anomalies present for time range show no results message #91151

Merged
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export type MlSummaryJobs = MlSummaryJob[];

export interface MlJobWithTimeRange extends CombinedJobWithStats {
id: string;
isRunning?: boolean;
isNotSingleMetricViewerJobMessage?: string;
timeRange: {
from: number;
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,52 @@ import { FormattedMessage } from '@kbn/i18n/react';

import { EuiEmptyPrompt } from '@elastic/eui';

export const ExplorerNoResultsFound = () => (
<EuiEmptyPrompt
iconType="iInCircle"
title={
<h2>
<FormattedMessage
id="xpack.ml.explorer.noResultsFoundLabel"
defaultMessage="No results found"
/>
</h2>
}
body={
<React.Fragment>
<p>
<FormattedMessage
id="xpack.ml.explorer.tryWideningTimeSelectionLabel"
defaultMessage="Try widening the time selection or moving further back in time"
/>
</p>
</React.Fragment>
}
/>
);
export const ExplorerNoResultsFound = ({
hasResults,
hasResultsWithAnomalies,
selectedJobsRunning,
}) => {
const resultsHaveNoAnomalies = hasResults === true && hasResultsWithAnomalies === false;
const noResults = hasResults === false && hasResultsWithAnomalies === false;
return (
<EuiEmptyPrompt
iconType="iInCircle"
title={
<h2>
{resultsHaveNoAnomalies && (
<FormattedMessage
id="xpack.ml.explorer.noAnomaliesFoundLabel"
defaultMessage="No anomalies found"
/>
)}
{noResults && (
<FormattedMessage
id="xpack.ml.explorer.noResultsFoundLabel"
defaultMessage="No results found"
/>
)}
</h2>
}
body={
<React.Fragment>
{selectedJobsRunning && noResults && (
<p>
<FormattedMessage
id="xpack.ml.explorer.selectedJobsRunningLabel"
defaultMessage="One or more selected jobs are still running and results may not be available yet."
/>
</p>
)}
{!selectedJobsRunning && (
<p>
<FormattedMessage
id="xpack.ml.explorer.tryWideningTimeSelectionLabel"
defaultMessage="Try widening the time selection or moving further back in time"
/>
</p>
)}
</React.Fragment>
}
/>
);
};
14 changes: 11 additions & 3 deletions x-pack/plugins/ml/public/application/explorer/explorer.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ export class Explorer extends React.Component {
setSelectedCells: PropTypes.func.isRequired,
severity: PropTypes.number.isRequired,
showCharts: PropTypes.bool.isRequired,
selectedJobsRunning: PropTypes.bool.isRequired,
};

state = { filterIconTriggeredQuery: undefined, language: DEFAULT_QUERY_LANG };
Expand Down Expand Up @@ -223,7 +224,7 @@ export class Explorer extends React.Component {
updateLanguage = (language) => this.setState({ language });

render() {
const { showCharts, severity, stoppedPartitions } = this.props;
const { showCharts, severity, stoppedPartitions, selectedJobsRunning } = this.props;

const {
annotations,
Expand All @@ -248,6 +249,9 @@ export class Explorer extends React.Component {

const noJobsFound = selectedJobs === null || selectedJobs.length === 0;
const hasResults = overallSwimlaneData.points && overallSwimlaneData.points.length > 0;
const hasResultsWithAnomalies =
Copy link
Contributor

Choose a reason for hiding this comment

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

This check needs amending as it means the 'No anomalies' message is shown in the following situations:

  • There are anomalous influencer and records results even though there are no anomalous overall buckets (the current message used below needs improving I think - something like 'No anomalous buckets in selected time range')

image

  • Changing the anomalies severity threshold to a value higher than highest record score (the current message is probably ok here):

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.

Good catch, @peteharverson. Updated hasResultsWithAnomalies check in feb0062

So hasResults means we get something back from the overall bucket endpoint (when there are no anomalous records at all we get empty results from the overall bucket endpoint) and hasResultsWithAnomalies means those overall bucket scores are > 0 OR we get some anomaly records back for populating the table.

Copy link
Contributor

Choose a reason for hiding this comment

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

"....No anomalous buckets in selected time range"

If it's true that this section is related to overall bucket scores, perhaps the message could be something like this:

No anomalies found in the overall bucket results for this time range

We talk about "overall buckets" in the docs in at least two places, in case it needs to be linked: https://www.elastic.co/guide/en/machine-learning/master/ml-buckets.html and https://www.elastic.co/guide/en/elasticsearch/reference/master/ml-get-overall-buckets.html

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 for taking a look @lcawl - updated the message in 1e10d7a
Added screen shot to the PR description above as well 👌

(hasResults && overallSwimlaneData.points.some((v) => v.value > 0)) ||
tableData.anomalies?.length > 0;

if (noJobsFound && !loading) {
return (
Expand All @@ -257,10 +261,14 @@ export class Explorer extends React.Component {
);
}

if (noJobsFound && hasResults === false && !loading) {
if (hasResultsWithAnomalies === false && !loading) {
return (
<ExplorerPage jobSelectorProps={jobSelectorProps}>
<ExplorerNoResultsFound />
<ExplorerNoResultsFound
hasResults={hasResults}
hasResultsWithAnomalies={hasResultsWithAnomalies}
selectedJobsRunning={selectedJobsRunning}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Since hasResultsWithAnomalies has the hard coded check against false here, it might not be necessary to pass it on as a prop to ExplorerNoResultsFound since we can assume within the component it will always be false..

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 - missed removing it. 👍 Updated in 1e10d7a

</ExplorerPage>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ const ExplorerUrlStateManager: FC<ExplorerUrlStateManagerProps> = ({ jobsWithTim
const timefilter = useTimefilter({ timeRangeSelector: true, autoRefreshSelector: true });

const { jobIds } = useJobSelection(jobsWithTimeRange);
const selectedJobsRunning = jobsWithTimeRange.some(
(job) => jobIds.includes(job.id) && job.isRunning === true
);

const explorerAppState = useObservable(explorerService.appState$);
const explorerState = useObservable(explorerService.state$);
Expand Down Expand Up @@ -261,6 +264,7 @@ const ExplorerUrlStateManager: FC<ExplorerUrlStateManagerProps> = ({ jobsWithTim
severity: tableSeverity.val,
stoppedPartitions,
invalidTimeRangeError,
selectedJobsRunning,
}}
/>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,6 @@ export class TimeSeriesExplorer extends React.Component {
}}
/>
}
color="warning"
iconType="help"
size="s"
/>
Expand Down