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 Explorer swim lane pagination #70063

Merged
merged 39 commits into from
Jul 2, 2020

Conversation

darnautov
Copy link
Contributor

@darnautov darnautov commented Jun 26, 2020

Summary

Resolves #68360 and fixes #70420

Adds pagination support for the Anomaly swim lane on the Anomaly Explorer page and the Anomaly swim lane embeddable.

Anomaly Explorer page
Jun-30-2020 11-37-02

Dashboard with anomaly swim lane embeddables
Jun-30-2020 11-41-04

You may notice that there is no "Limit" control anymore. The actual size of the request is limited by the field cardinality or by a hard limit of 1000 unique terms to avoid extremely heavy aggregations. The paging is achieved by removing the limit as a size value with cardinality and applying bucket_score aggregation to truncate buckets based on the page size.

Some changes related to the Anomaly explorer page:

  • Some legacy utils have been removed. AnomalyTimelineService is used both on the Anomaly Explorer page and the embeddable.
  • Container width of the swim lane always come from the state, there is no imperative check using document selector anymore
  • Fixed immediate re-render on the initial page load

Embeddable input also lacking the "limit" control during setup. When the dashboard is in edit mode, the "Row per page" values persist in the embeddable configuration.

Some other highlights:

  • There is no global loading indicator for the Anomaly Explorer page anymore because it was blocking the initial render of the Anomaly Timeline panel. Each panel on the page has it's own loading state, e.g. "Top influencer" now using loading content indicator.
  • Added empty state for overall in case there is no data
  • Added empty states for the embeddable

Checklist

@darnautov darnautov self-assigned this Jun 26, 2020
@darnautov darnautov force-pushed the ML-68360-swimlane-pagination branch from a24cb41 to 483ee8a Compare June 28, 2020 14:53
@darnautov darnautov marked this pull request as ready for review June 30, 2020 09:45
@darnautov darnautov requested review from a team as code owners June 30, 2020 09:45
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@mdefazio
Copy link
Contributor

mdefazio commented Jul 1, 2020

Specific to the pagination this time :D

  • Let's set the pagination in the Anomaly timeline to use the xSmall button for rows per page so it's consistent with the Anomalies and Annotations pagination rows per page buttons.
  • It looks like there is some extra padding or margins below the pagination. Best I could find was on .ml-explorer.mlExplorerSwimlane which has bottom-margin: 8px. I know this adds margins below the swimlanes above the pagination, but I think we're using spacers in there anyway so we shouldn't need it.

@darnautov
Copy link
Contributor Author

Specific to the pagination this time :D

  • Let's set the pagination in the Anomaly timeline to use the xSmall button for rows per page so it's consistent with the Anomalies and Annotations pagination rows per page buttons.
  • It looks like there is some extra padding or margins below the pagination. Best I could find was on .ml-explorer.mlExplorerSwimlane which has bottom-margin: 8px. I know this adds margins below the swimlanes above the pagination, but I think we're using spacers in there anyway so we shouldn't need it.

thanks for spotting it @mdefazio , just pushed changes cda437d

@qn895
Copy link
Member

qn895 commented Jul 1, 2020

The swim lane doesn't seem to pick up the removal of a filter in the KQL query bar:

Fixed in d1919b3

When we remove a filter, should we expect it to show it resets? E.g. after removing the filter should it show 5/10/20 of the rows before the filter was applied.

@walterra

This comment has been minimized.

@walterra
Copy link
Contributor

walterra commented Jul 2, 2020

Not sure if related to this PR, but when I view a job without influencers, the "View by" label and its dropdown overlap:

@darnautov
Copy link
Contributor Author

Suggest to use the black&white loading indicators. From EUI docs:

The colored versions should be used sparingly, only when a single large visualization is loaded. When loading smaller groups of panels, the smaller, mono versions should be used.

@walterra changed in f8af4b0

@darnautov
Copy link
Contributor Author

If I change the time range from one which doesn't cover the full range of the job, to the full time range (by opening the job selector, ensuring 'Apply time range' is enabled) and applying, the swim lanes don't re-render for the full time range:

image

@peteharverson fixed in e6fd3af

@darnautov darnautov requested a review from walterra July 2, 2020 10:08
Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

Latest Code LGTM

@peteharverson
Copy link
Contributor

@qn895 regarding #70063 (comment), testing the removal of a filter, the swim lane rerenders with the page size setting back to where it was before the filter was applied for me. This is what I expect. Did you see something different?

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested latest edits and LGTM. Great job!

Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

Pagination part looks good. I think there are a few other items we can address but is more suited for #70198

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
ml 356 -1 357

History

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

@darnautov darnautov merged commit 854e7a5 into elastic:master Jul 2, 2020
@darnautov darnautov deleted the ML-68360-swimlane-pagination branch July 2, 2020 14:30
darnautov added a commit to darnautov/kibana that referenced this pull request Jul 2, 2020
* [ML] use explorer service

* [ML] WIP pagination

* [ML] add to dashboard without the limit

* [ML] WIP

* [ML] loading states

* [ML] viewBySwimlaneDataLoading on field change

* [ML] fix dashboard control

* [ML] universal swim lane container, embeddable pagination

* [ML] fix css issue

* [ML] rename anomalyTimelineService

* [ML] rename callback

* [ML] rename container component

* [ML] empty state, increase pagination margin

* [ML] check for loading

* [ML] fix i18n

* [ML] fix unit test

* [ML] improve selected cells

* [ML] fix overall selection with changing job selection

* [ML] required props for pagination component

* [ML] move RESIZE_IGNORED_DIFF_PX

* [ML] jest tests

* [ML] add test subject

* [ML] SWIM_LANE_DEFAULT_PAGE_SIZE

* [ML] change empty state styling

* [ML] fix agg size for influencer filters

* [ML] remove debounce

* [ML] SCSS variables, rename swim lane class

* [ML] job selector using context

* [ML] set padding for embeddable panel

* [ML] adjust pagination styles

* [ML] replace custom time range subject with timefilter

* [ML] change loading indicator to mono

* [ML] use swim lane type constant

* [ML] change context naming

* [ML] update jest snapshot

* [ML] fix tests
gmmorris added a commit to gmmorris/kibana that referenced this pull request Jul 2, 2020
* master:
  Update known-plugins.asciidoc (elastic#69370)
  [ML] Anomaly Explorer swim lane pagination (elastic#70063)
  [Ingest Manager] Update asset paths to use _ instead of - (elastic#70320)
  Fix discover, tsvb and Lens chart theming issues (elastic#69695)
  Allow Saved Object type mappings to set a field's `doc_values` property (elastic#70433)
  [S&R] Support data streams (elastic#68078)
  [Maps] Add styling and tooltip support to mapbox mvt vector tile sources (elastic#64488)
  redirect to default app if hash can not be forwarded (elastic#70417)
  [APM] Don't fetch dynamic index pattern in setupRequest (elastic#70308)
  [Security_Solution][Endpoint] Leveraging msearch and ancestry array for resolver (elastic#70134)
  Update docs for api authentication usage (elastic#66819)
  chore(NA): disable alerts_detection_rules cypress suites (elastic#70577)
  add getVisibleTypes API to SO type registry (elastic#70559)
darnautov added a commit that referenced this pull request Jul 2, 2020
* [ML] use explorer service

* [ML] WIP pagination

* [ML] add to dashboard without the limit

* [ML] WIP

* [ML] loading states

* [ML] viewBySwimlaneDataLoading on field change

* [ML] fix dashboard control

* [ML] universal swim lane container, embeddable pagination

* [ML] fix css issue

* [ML] rename anomalyTimelineService

* [ML] rename callback

* [ML] rename container component

* [ML] empty state, increase pagination margin

* [ML] check for loading

* [ML] fix i18n

* [ML] fix unit test

* [ML] improve selected cells

* [ML] fix overall selection with changing job selection

* [ML] required props for pagination component

* [ML] move RESIZE_IGNORED_DIFF_PX

* [ML] jest tests

* [ML] add test subject

* [ML] SWIM_LANE_DEFAULT_PAGE_SIZE

* [ML] change empty state styling

* [ML] fix agg size for influencer filters

* [ML] remove debounce

* [ML] SCSS variables, rename swim lane class

* [ML] job selector using context

* [ML] set padding for embeddable panel

* [ML] adjust pagination styles

* [ML] replace custom time range subject with timefilter

* [ML] change loading indicator to mono

* [ML] use swim lane type constant

* [ML] change context naming

* [ML] update jest snapshot

* [ML] fix tests
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.

[ML] Swim lane cell selection reverts the dashboard time range [ML] Add paging to Anomaly Explorer swimlane
7 participants