[ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement #83216

qn895 · 2020-11-11T21:30:02Z

Summary

This PR addresses a performance issue which cause typing in the annotation flyout editor in Single Metric View to be slow and unresponsive when typing fast. Follow up of #72299.

Previously, renderFocusChart() will be called with every keystroke. This PR fixes this issue.

Before
After
It also updates the button placement in the footer to match with Kibana's design convention [ML] Anomaly Detection: Annotations enhancements #70198 (comment).
Before
After
Previously, switching from the Single Metric Viewer to the Anomaly Explorer and vice versa will keep persisting the annotation fly-out. This PR fixes so that there are three separate AnnotationUpdatesService instances for the job list, the Single Metric Viewer, and the Anomaly Explorer page.
Before
After

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
This was checked for cross-browser compatibility

elasticmachine · 2020-11-11T21:30:05Z

Pinging @elastic/ml-ui (:ml)

...ins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js

walterra

LGTM

x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx

...gins/ml/public/application/components/annotations/annotations_service/annotation_service.tsx

x-pack/plugins/ml/public/application/services/annotations_service.tsx

…v-improvements

darnautov

Great job with AnnotationUpdatesService, just some minor issues around one method need to be resolved

x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.tsx

x-pack/plugins/ml/public/application/services/annotations_service.tsx

x-pack/plugins/ml/public/application/services/annotations_service.test.tsx

x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.test.tsx

darnautov · 2020-11-17T12:31:12Z

x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.test.tsx


-  test('Update button is disabled with empty annotation', () => {
+const annotationUpdatesService = new AnnotationUpdatesService();


you should instantiate a service in before hook

@qn895 inside of describe('AnnotationFlyout', () => {
creating the following

let annotationUpdatesService: AnnotationUpdatesService = null beforeEach(() => { annotationUpdatesService = new AnnotationUpdatesService(); })

to make sure each test is isolated

My apologies - missed this comment yesterday. Updated here: 9b8020b

x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.test.tsx

darnautov · 2020-11-17T12:43:24Z

@qn895 I see you've created a context, but you haven't provided a value itself using Provider of this context. For some reason, you only did it for unit tests :)

…v-improvements

qn895 · 2020-11-17T18:11:51Z

@qn895 I see you've created a context, but you haven't provided a value itself using Provider of this context. For some reason, you only did it for unit tests :)

Thanks. I have added the AnnotationUpdatesProvider here 2165da7 instead of using the default static instance.

darnautov

LGTM! 🚀

…v-improvements

kibanamachine · 2020-11-18T16:48:27Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9b8020b

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`ml`	1565	1566	+1

Async chunks

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

id	before	after	diff
`ml`	5.2MB	5.2MB	+3.9KB

History

💚 Build #88382 succeeded f179e79
💚 Build #88192 succeeded 6eff2c8
💔 Build #87764 failed 528a8de
💚 Build #87467 succeeded 3d854a2

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

… Viewer & buttons placement (elastic#83216)

…Metric Viewer & buttons placement (#83216) (#83671)

…o-node-details * 'master' of github.com:elastic/kibana: [ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement (elastic#83216) [Metrics UI] Add Process tab to Enhanced Node Details (elastic#83477)

… into add-logs-to-node-details * 'add-logs-to-node-details' of github.com:phillipb/kibana: (87 commits) [Maps] Add 'crossed' & 'exited' events to tracking alert (elastic#82463) Updating code-owners to use new core/app-services team names (elastic#83731) Add Managed label to data streams and a view switch for the table (elastic#83049) [Maps] Add query bar inputs to geo threshold alerts tracked points & boundaries (elastic#80871) fix(NA): search examples kibana version declaration (elastic#83182) Fixed console error, which appears when saving changes in Edit Alert flyout (elastic#83610) [Alerting] Add `alert.updatedAt` field to represent date of last user edit (elastic#83578) Not resetting server log level if level is defined (elastic#83651) disable incremenetal build for legacy tsconfig.json (elastic#82986) [Workplace Search] Migrate SourceLogic from ent-search (elastic#83593) [Workplace Search] Port Box changes from ent-search (elastic#83675) [APM] Improve router types (elastic#83620) Bump flat to v4.1.1 (elastic#83647) Bump y18n@5 to v5.0.5 (elastic#83644) Bump jsonpointer to v4.1.0 (elastic#83641) Bump is-my-json-valid to v2.20.5 (elastic#83642) [Telemetry] Move Monitoring collection strategy to a collector (elastic#82638) Update typescript eslint to v4.8 (elastic#83520) [ML] Persist URL state for Anomaly detection jobs using metric function (elastic#83507) [ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement (elastic#83216) ...

qn895 added 2 commits November 11, 2020 14:55

[ML] Change placement of annotation footer buttons

db8673c

[ML] Fix performance issue with annotation/renderFocusChart on SMV

3d854a2

qn895 added release_note:enhancement :ml Feature:Anomaly Detection ML anomaly detection v8.0.0 v7.11.0 labels Nov 11, 2020

qn895 requested review from walterra, darnautov and peteharverson November 11, 2020 21:30

qn895 self-assigned this Nov 11, 2020

darnautov reviewed Nov 12, 2020

View reviewed changes

...ins/ml/public/application/timeseriesexplorer/components/timeseries_chart/timeseries_chart.js Outdated Show resolved Hide resolved

walterra approved these changes Nov 12, 2020

View reviewed changes

qn895 added 2 commits November 12, 2020 15:56

[ML] Add AnnotationUpdatesService and MlAnnotationComponent

b5166e6

[ML] Update type definition

528a8de

qn895 requested a review from a team as a code owner November 12, 2020 21:57

darnautov requested changes Nov 16, 2020

View reviewed changes

qn895 added 7 commits November 16, 2020 13:03

[ML] Add to AnnotationUpdatesService context, update tests

81e20c4

[ML] Update snapshots for AnnotationsTable

bafde0d

Merge remote upstream into ml-annotations-smv-improvements

927b38d

Merge remote-tracking branch 'upstream/master' into ml-annotations-sm…

c4ebb5e

…v-improvements

Merge remote-tracking branch 'upstream/master' into ml-annotations-sm…

d3d4ce4

…v-improvements

Merge remote-tracking branch 'upstream/master' into ml-annotations-sm…

eb1da71

…v-improvements

[ML] Delete MlAnnotationComponent

6eff2c8

qn895 requested a review from darnautov November 17, 2020 00:21

darnautov reviewed Nov 17, 2020

View reviewed changes

x-pack/plugins/ml/public/application/components/annotations/annotation_flyout/index.test.tsx Outdated Show resolved Hide resolved

darnautov reviewed Nov 17, 2020

View reviewed changes

qn895 added 6 commits November 17, 2020 10:17

[ML] Update updateBtn to grab by id

1365c52

[ML] Rename update$, isAnnotationInitialized$

0cf7ad0

[ML] Add MlAnnotationUpdatesContext.Provider, rename file

2165da7

Merge upstream/main into ml-annotations-smv-improvements

d2cb370

Update test types

3575d6a

Merge remote-tracking branch 'upstream/master' into ml-annotations-sm…

f179e79

…v-improvements

darnautov approved these changes Nov 18, 2020

View reviewed changes

qn895 added 3 commits November 18, 2020 08:38

[ML] Remove console log

588bedd

Merge remote-tracking branch 'upstream/master' into ml-annotations-sm…

b67515d

…v-improvements

[ML] Move annotationUpdatesService to beforeEach

9b8020b

qn895 merged commit 7a7057e into elastic:master Nov 18, 2020

qn895 deleted the ml-annotations-smv-improvements branch November 18, 2020 16:49

qn895 mentioned this pull request Nov 18, 2020

[7.x] [ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement (#83216) #83671

Merged

qn895 added a commit to qn895/kibana that referenced this pull request Nov 18, 2020

[ML] Performance improvements to annotations editing in Single Metric…

aaea3a1

… Viewer & buttons placement (elastic#83216)

qn895 added a commit that referenced this pull request Nov 18, 2020

[7.x] [ML] Performance improvements to annotations editing in Single …

fa52b06

…Metric Viewer & buttons placement (#83216) (#83671)

peteharverson mentioned this pull request Dec 22, 2020

[Meta] [ML] Improve Annotations #69538

Closed

25 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement #83216

[ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement #83216

qn895 commented Nov 11, 2020 •

edited

Loading

elasticmachine commented Nov 11, 2020

walterra left a comment

darnautov left a comment

darnautov Nov 17, 2020

darnautov Nov 18, 2020

qn895 Nov 18, 2020

darnautov commented Nov 17, 2020

qn895 commented Nov 17, 2020

darnautov left a comment

kibanamachine commented Nov 18, 2020


		test('Update button is disabled with empty annotation', () => {
		const annotationUpdatesService = new AnnotationUpdatesService();

[ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement #83216

[ML] Performance improvements to annotations editing in Single Metric Viewer & buttons placement #83216

Conversation

qn895 commented Nov 11, 2020 • edited Loading

Summary

Checklist

elasticmachine commented Nov 11, 2020

walterra left a comment

Choose a reason for hiding this comment

darnautov left a comment

Choose a reason for hiding this comment

darnautov Nov 17, 2020

Choose a reason for hiding this comment

darnautov Nov 18, 2020

Choose a reason for hiding this comment

qn895 Nov 18, 2020

Choose a reason for hiding this comment

darnautov commented Nov 17, 2020

qn895 commented Nov 17, 2020

darnautov left a comment

Choose a reason for hiding this comment

kibanamachine commented Nov 18, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

qn895 commented Nov 11, 2020 •

edited

Loading