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: Adds popover links menu to anomaly explorer charts. #186587

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Jun 21, 2024

Summary

Adds support for clicking on Anomaly Explorer charts to trigger the actions popover menu.

  • ExplorerChartSingleMetric
  • ExplorerChartDistribution
  • Support for embedded charts

Anomaly Explorer

ml-anomaly-charts-actions-0001.webm

Embedding

ml-anomaly-charts-actions-embedding-0001.webm

Checklist

@walterra walterra self-assigned this Jun 21, 2024
@walterra walterra marked this pull request as ready for review June 21, 2024 11:32
@walterra walterra requested a review from a team as a code owner June 21, 2024 11:32
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

.on('click', function (d) {
d3.event.preventDefault();
if (d.anomalyScore === undefined) return;
showAnomalyPopover(d, this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you enable the popover on multi-bucket markers too? This is already supported in the Single Metric Viewer chart.

Copy link
Contributor Author

@walterra walterra Jun 21, 2024

Choose a reason for hiding this comment

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

Fixed in 7829012.

anomaly={this.state.popoverData}
bounds={this.props.bounds}
showMapsLink={false}
showViewSeriesLink={true}
Copy link
Contributor

Choose a reason for hiding this comment

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

The 'View series' link isn't working for me. I see this error:

Screenshot 2024-06-21 at 14 42 51

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in db0bd6f.

Copy link
Contributor

Choose a reason for hiding this comment

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

Time range is being passed correctly. But now I'm seeing that the partition field is often not set correctly in the SMV. Happens from the Anomaly Explorer or the embeddable.

@@ -108,6 +126,53 @@ const AnomalyChartsContainer: FC<AnomalyChartsContainerProps> = ({
error,
} = useAnomalyChartsData(api, services, chartWidth, severity.val, renderCallbacks);

const dateFormatTz = useDateFormatTz();

useEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

If I go to a Dashboard in which I have an anomaly charts after first being in the Anomaly Explorer, the popover isn't appearing. It only appears for me after doing a full refresh of the Dashboard page.

I am seeing these errors on going into the Dashboard page:

Screenshot 2024-06-21 at 15 23 50

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a race condition because in some places mlJobService was still used via dependency cache. Should be fixed in e35c568

Copy link
Contributor

Choose a reason for hiding this comment

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

I can still reproduce this - when starting in the Anomaly Explorer, loading some anomaly charts, doing a drilldown, and then switching to Dashboard.

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #4 / rules_list component empty hides MaintenanceWindowCallout if filterConsumers does not match the running maintenance window's category

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 4.1MB 4.1MB -36.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
ml 80.2KB 80.1KB -84.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
ml 556 562 +6

Total ESLint disabled count

id before after diff
ml 559 565 +6

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @walterra

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants