[ML] Explain Log Rate Spikes: adds popover to analysis table for viewing other field values #154689

alvarezmelissa87 · 2023-04-10T22:13:45Z

Summary

Related meta issue: #146065

Adds unified field list popover to show top values for fields in the Explain Log Rate Spikes analysis table.

Field pairs table:

Expanded row in grouped table:

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

elasticmachine · 2023-04-10T22:13:49Z

Pinging @elastic/ml-ui (:ml)

…fix'

x-pack/plugins/aiops/public/components/field_stats_popover/field_stats_popover.tsx

x-pack/plugins/aiops/public/components/spike_analysis_table/spike_analysis_table.tsx

walterra

Great addition! Some things I noticed:

Layout nitpick: The popover icon has an influence on the upper padding of the field name cell so the text appears lower than in the field value cell.

With the icon after the field name, if you want to walk through the rows quickly and inspect the field statistics, it requires quite some targeting with the mouse. If the icon was before the field name it would be easier with vertically aligned icons. Not sure but maybe it's worth moving the icon to its own column, that would also solve the problem with vertical alignment?

One final thought: The field/value badges in the groups table themselves could be clickable and trigger the popover for that field.

walterra · 2023-04-12T07:48:04Z

x-pack/plugins/aiops/public/components/field_stats_popover/field_stats_popover.tsx

+      button={trigger}
+      renderHeader={() => <FieldPopoverHeader field={fieldForStats} closePopover={closePopover} />}
+      renderContent={() => (
+        <>


Nit: Only one element in there so we could remove the <> wrapper.

Removed in a44cade

walterra · 2023-04-12T08:08:56Z

...plugins/aiops/public/components/explain_log_rate_spikes/explain_log_rate_spikes_analysis.tsx

@@ -284,13 +285,19 @@ export const ExplainLogRateSpikesAnalysis: FC<ExplainLogRateSpikesAnalysisProps>
          groupTableItems={groupTableItems}
          loading={isRunning}
          dataViewId={dataView.id}
+          dataView={dataView}


If we need to pass in the full dataView let's try to get rid of the dataViewId prop and access the id from dataView within the component.

Removed in a44cade

walterra · 2023-04-12T08:09:02Z

...plugins/aiops/public/components/explain_log_rate_spikes/explain_log_rate_spikes_analysis.tsx

        />
      ) : null}
      {showSpikeAnalysisTable && (!groupResults || !foundGroups) ? (
        <SpikeAnalysisTable
          significantTerms={data.significantTerms}
          loading={isRunning}
          dataViewId={dataView.id}
+          dataView={dataView}


If we need to pass in the full dataView let's try to get rid of the dataViewId prop and access the id from dataView within the component.

Good catch - removed in a44cade

alvarezmelissa87 · 2023-04-12T21:07:28Z

@walterra - thanks for giving this a test! 🙏 Fixed the alignment issue in a44cade
As far as making the badges clickable - I think it's better to keep using the icon we're using everywhere else to be consistent and so users already know what to expect when they see it.

peteharverson

Tested latest changes and LGTM

qn895 · 2023-04-13T14:25:24Z

Code LGTM 🎉

walterra

Overall LGTM - We should update the color of the highlighted field/value pair in the popover to match the highlight color in the mini histograms and overall histogram.

kibana-ci · 2023-04-13T18:57:02Z

💚 Build Succeeded

Buildkite Build
Commit: d756ecb

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`aiops`	490	493	+3

Async chunks

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

id	before	after	diff
`aiops`	766.1KB	769.8KB	+3.7KB

Page load bundle

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

id	before	after	diff
`aiops`	5.3KB	5.4KB	+146.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`securitySolution`	432	435	+3

Total ESLint disabled count

id	before	after	diff
`securitySolution`	512	515	+3

History

💛 Build #119603 was flaky a5cc5f3
💚 Build #118823 succeeded f4c65cd

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

cc @alvarezmelissa87

add unified fields popover to analysis table field names

43762e0

alvarezmelissa87 added release_note:enhancement :ml Feature:ML/AIOps ML AIOps features: Change Point Detection, Log Pattern Analysis, Log Rate Analysis v8.8.0 labels Apr 10, 2023

alvarezmelissa87 self-assigned this Apr 10, 2023

alvarezmelissa87 requested a review from a team as a code owner April 10, 2023 22:13

alvarezmelissa87 requested review from walterra, peteharverson and qn895 April 10, 2023 22:14

alvarezmelissa87 and others added 2 commits April 10, 2023 16:17

remove unused function

98ae785

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

f4c65cd

…fix'

peteharverson reviewed Apr 11, 2023

View reviewed changes

x-pack/plugins/aiops/public/components/field_stats_popover/field_stats_popover.tsx Outdated Show resolved Hide resolved

peteharverson reviewed Apr 11, 2023

View reviewed changes

x-pack/plugins/aiops/public/components/field_stats_popover/field_stats_popover.tsx Outdated Show resolved Hide resolved

x-pack/plugins/aiops/public/components/spike_analysis_table/spike_analysis_table.tsx Show resolved Hide resolved

walterra reviewed Apr 12, 2023

View reviewed changes

alvarezmelissa87 added 2 commits April 12, 2023 14:34

add highlighting and individual stat

ae6e32c

fix alignment, move icon before fieldName

a44cade

Merge branch 'main' into ml-aiops-unified-fields

a5cc5f3

peteharverson changed the title ~~[ML] Explain Log Rate Spikes: add unified fields popover to analysis table fields~~ [ML] Explain Log Rate Spikes: add popover to analysis table for viewing other field values Apr 13, 2023

peteharverson approved these changes Apr 13, 2023

View reviewed changes

qn895 approved these changes Apr 13, 2023

View reviewed changes

alvarezmelissa87 added 2 commits April 13, 2023 11:33

Merge branch 'main' into ml-aiops-unified-fields

8387514

add missing import

d756ecb

walterra approved these changes Apr 13, 2023

View reviewed changes

alvarezmelissa87 merged commit ec0a87c into elastic:main Apr 17, 2023

kibanamachine added the backport:skip This commit does not require backporting label Apr 17, 2023

alvarezmelissa87 deleted the ml-aiops-unified-fields branch April 17, 2023 19:50

szabosteve changed the title ~~[ML] Explain Log Rate Spikes: add popover to analysis table for viewing other field values~~ [ML] Explain Log Rate Spikes: adds popover to analysis table for viewing other field values Apr 21, 2023

peteharverson mentioned this pull request Apr 26, 2023

[ML] Explain Log Rate Spikes: MVP 8.8 improvements. #146065

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Explain Log Rate Spikes: adds popover to analysis table for viewing other field values #154689

[ML] Explain Log Rate Spikes: adds popover to analysis table for viewing other field values #154689

alvarezmelissa87 commented Apr 10, 2023 •

edited

Loading

elasticmachine commented Apr 10, 2023

walterra left a comment

walterra Apr 12, 2023

alvarezmelissa87 Apr 12, 2023

walterra Apr 12, 2023

alvarezmelissa87 Apr 12, 2023

walterra Apr 12, 2023

alvarezmelissa87 Apr 12, 2023

alvarezmelissa87 commented Apr 12, 2023

peteharverson left a comment

qn895 commented Apr 13, 2023

walterra left a comment

kibana-ci commented Apr 13, 2023

ESLint disabled line counts

Total ESLint disabled count

[ML] Explain Log Rate Spikes: adds popover to analysis table for viewing other field values #154689

[ML] Explain Log Rate Spikes: adds popover to analysis table for viewing other field values #154689

Conversation

alvarezmelissa87 commented Apr 10, 2023 • edited Loading

Summary

Checklist

elasticmachine commented Apr 10, 2023

walterra left a comment

Choose a reason for hiding this comment

walterra Apr 12, 2023

Choose a reason for hiding this comment

alvarezmelissa87 Apr 12, 2023

Choose a reason for hiding this comment

walterra Apr 12, 2023

Choose a reason for hiding this comment

alvarezmelissa87 Apr 12, 2023

Choose a reason for hiding this comment

walterra Apr 12, 2023

Choose a reason for hiding this comment

alvarezmelissa87 Apr 12, 2023

Choose a reason for hiding this comment

alvarezmelissa87 commented Apr 12, 2023

peteharverson left a comment

Choose a reason for hiding this comment

qn895 commented Apr 13, 2023

walterra left a comment

Choose a reason for hiding this comment

kibana-ci commented Apr 13, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

alvarezmelissa87 commented Apr 10, 2023 •

edited

Loading