[ML] Anomaly Detection: add anomalies map to explorer for jobs with 'lat_long' function#88416
Conversation
|
Pinging @elastic/ml-ui (:ml) |
...ck/plugins/ml/public/application/explorer/components/explorer_map_container/embedded_map.tsx
Outdated
Show resolved
Hide resolved
.../ml/public/application/explorer/components/explorer_map_container/explorer_map_container.tsx
Outdated
Show resolved
Hide resolved
|
Love the embedded map in the Anomaly Explorer! Some thoughts:
|
|
This looks great! Agree with Pete's thought: What if we make this "just" another chart type of the charts when you drill down and click on anomalies (e.g. regular line chart, rare chart, population chart, map). For those charts we don't show |
There was a problem hiding this comment.
I think we can remove this className here
398e64e to
90141a2
Compare
|
This has been updated and is ready for another look when you get a chance. cc @walterra, @peteharverson 🙏 |
walterra
left a comment
There was a problem hiding this comment.
Great progress! Added some more comments.
There was a problem hiding this comment.
Since all other charts code is still plain JS, agree to use the any here in this PR. I added an item to this charts meta-issue that we should migrate to TS #21163.
There was a problem hiding this comment.
Updated in a9b5c32580d12c8a582b382049631e9709a7230a
There was a problem hiding this comment.
If I remember correctly, for the file/data visualizer the maps plugin is defined as an optional plugin. Are there plans for a more graceful fallback in the UI if the plugin is not available?
There was a problem hiding this comment.
You're correct - will update to optional as it should be and show a toast with a message that the plugin is disabled but let the anomaly explorer load normally otherwise 👍
There was a problem hiding this comment.
Looks like embeddablePlugin is fully typed, can we get rid of the any?
There was a problem hiding this comment.
Removed in a9b5c32580d12c8a582b382049631e9709a7230a
There was a problem hiding this comment.
Based on other EUI examples, we use EUI's htmlIdGenerator is other places like this:
import { htmlIdGenerator } from '@elastic/eui';
const id = useMemo(() => htmlIdGenerator()(), []);There was a problem hiding this comment.
Updated in 478a75725eb997b45a1c62becae5c72bb4d9a1c2
There was a problem hiding this comment.
Would be great to get rid of that any.
There was a problem hiding this comment.
Updated in 478a75725eb997b45a1c62becae5c72bb4d9a1c2
There was a problem hiding this comment.
Is this class necessary? I don't see any other reference to it in the ML plugin anywhere else.
There was a problem hiding this comment.
Removed in 478a75725eb997b45a1c62becae5c72bb4d9a1c2
There was a problem hiding this comment.
Can we move this to a custom class in an SCSS file?
There was a problem hiding this comment.
There was a problem hiding this comment.
Should be fixed by 478a75725eb997b45a1c62becae5c72bb4d9a1c2
x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_embedded_map.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_embedded_map.tsx
Outdated
Show resolved
Hide resolved
|
Looks like there is an issue with display of the detector function in the chart info tooltip for Thanks for catching that, @peteharverson - fixed in 478a75725eb997b45a1c62becae5c72bb4d9a1c2 |
33a7908 to
dff81a9
Compare
3737edb to
ea9414a
Compare
x-pack/plugins/ml/public/application/explorer/explorer_charts/map_config.ts
Show resolved
Hide resolved
| )} | ||
| </MlTooltipComponent> | ||
| ); | ||
| if (chartType === CHART_TYPE.SINGLE_METRIC) { |
|
LGTM 🎉 |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
peteharverson
left a comment
There was a problem hiding this comment.
Tested latest changes, and LGTM
…lat_long' function (#88416) (#89738) * wip: create embedded map component for explorer * add embeddedMap component to explorer * use geo_results * remove charts callout when map is shown * add translation, round geo coordinates * create GEO_MAP chart type and move embedded map to charts area * remove embedded map that is no longer used * fix type and fail silently if plugin not available * fix multiple type of jobs charts view * fix tooltip function and remove single viewer link for latlong * ensure diff types of jobs show correct charts. fix jest test * show errorCallout if maps not enabled and is lat_long job * use shared MlEmbeddedMapComponent in explorer * ensure latLong jobs not viewable in single metric viewer * update jest test



Summary
Different types of jobs selected:
When maps or embeddable plugin are not enabled we show a message to the user and only show charts that can be shown:
NOTES
Checklist
Delete any items that are not applicable to this PR.