Skip to content

[Lens][Agg based table]Navigate to lens for agg based table vis#140791

Merged
Kuznietsov merged 264 commits intoelastic:mainfrom
VladLasitsa:navigate-to-lens-agg-based-vis
Sep 26, 2022
Merged

[Lens][Agg based table]Navigate to lens for agg based table vis#140791
Kuznietsov merged 264 commits intoelastic:mainfrom
VladLasitsa:navigate-to-lens-agg-based-vis

Conversation

@VladLasitsa
Copy link
Copy Markdown
Contributor

@VladLasitsa VladLasitsa commented Sep 15, 2022 •
edited by Kuznietsov
Loading

Summary

Completes part of #138236.

As part of phasing out TSVB and Visualize all Legacy agg based visulizations should support "open in lens" functionality.
In that PR converter for Agg based table was added.

Kuznietsov and others added 30 commits August 19, 2022 12:26
@Kuznietsov
Removed console.log.
a2d51eb
@Kuznietsov
Other aggs to formula convertion added.
78b715d
@Kuznietsov
Filter ratio added.
0549bdd
@VladLasitsa
Add XY Configuration
9d1b9c9
@Kuznietsov
Added last_value convertion.
7b8786f
@Kuznietsov
Merge branch 'navigate-to-lens-context-converting-imporvement' of git…
8b57f8d 
…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement
@Kuznietsov
Updated ColumnWithMeta types.
6766d81
@Kuznietsov
Fixed for loop.
edbeea0
@Kuznietsov
Added StaticValue convertion.
5221a01
@Kuznietsov
Added default agg convertion.
ecdf3e2
@VladLasitsa
Add filters column
faab810
@Kuznietsov
Refactored filter part.
08a0539
@Kuznietsov
Added meta to all the cols.
defc3a0
@Kuznietsov
Fixed problem with columns.
0da77f1
@VladLasitsa
Added date_histogram column, fixed layer configuration
fcdcb41
@Kuznietsov
Added terms support.
bfd1dff
@Kuznietsov
Refactored.
8f4c8d5
@Kuznietsov
Refactored for consistancy.
a21b64e
@Kuznietsov
Removed getVisualizationSuggestionFromContext fn.
6b0f939
@Kuznietsov
Added references.
a36fabc
@Kuznietsov
Fixed percentile and percentile ranks.
a9d19b7
@Kuznietsov
removed unused return statement.
f1b8303
@VladLasitsa
Refactoring 'getEmptyLayersSuggestionsForVisualizeCharts' method
00dc472
@Kuznietsov
TimeScaleUnit fix.
b146838
@Kuznietsov
Fixed references.
97187d6
@Kuznietsov
Fixed types.
b2817cc
@VladLasitsa
Fixed terms column
798998b
@Kuznietsov
Fixed count custom label.
b26ca08
@Kuznietsov
Merge branch 'navigate-to-lens-context-converting-imporvement' of git…
28e20f7 
…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement
@VladLasitsa
Fix multi series
ac20b5a
@Kuznietsov
Copy link
Copy Markdown
Contributor

Kuznietsov commented Sep 22, 2022 •
edited
Loading

  1. The operations that allow dates (or geofield) in datatable but not in Lens cause improper dimensions created. I think we should either remove them or not allow to convert to lens when they occur. Here’s the 3 examples:
Screenshot 2022-09-22 at 11 13 13

Done.

@Kuznietsov
Copy link
Copy Markdown
Contributor

Kuznietsov commented Sep 22, 2022

@elasticmachine merge upstream

@spalger
Copy link
Copy Markdown
Contributor

spalger commented Sep 22, 2022

@elasticmachine merge upstream

(to resolve ci issues caused by Github outage)

mbondyra
mbondyra approved these changes Sep 23, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@mbondyra mbondyra left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There are two small inconsistencies that can be target later if at all:

  1. The histogram is not giving the same intervals as converted intervals:

Screenshot 2022-09-23 at 12 09 20

Screenshot 2022-09-23 at 12 09 25

  1. For the field types, it doesn't allow to convert for simple metric if used the not suppored fieldtype, like this one:

Screenshot 2022-09-23 at 12 00 58

But for cumulative_sum of the same metric it allows to convert and then shows nice readable error message:
Screenshot 2022-09-23 at 12 01 15
converts to:
Screenshot 2022-09-23 at 12 01 19

I find neither of them critical so approving. It can be addressed later.

@VladLasitsa
Copy link
Copy Markdown
Contributor Author

VladLasitsa commented Sep 23, 2022

There are two small inconsistencies that can be target later if at all:

  1. The histogram is not giving the same intervals as converted intervals:
Screenshot 2022-09-23 at 12 09 20 Screenshot 2022-09-23 at 12 09 25

@mbondyra About that: we don't have possibility to provide interval for "intervals" in Lens.
We have only these params:
Снимок экрана 2022-09-23 в 14 08 51
In Lens implementation we always provide "auto" as interval for histogram:

Снимок экрана 2022-09-23 в 14 10 41

After discussion with Joe we decided to ignore interval for histogram.

cc @flash1293

@Kuznietsov
Copy link
Copy Markdown
Contributor

Kuznietsov commented Sep 23, 2022

  1. For the field types, it doesn't allow to convert for simple metric if used the not suppored fieldtype, like this one:
Screenshot 2022-09-23 at 12 00 58

But for cumulative_sum of the same metric it allows to convert and then shows nice readable error message: Screenshot 2022-09-23 at 12 01 15 converts to: Screenshot 2022-09-23 at 12 01 19

@mbondyra, we've fixed this problem.

@mbondyra
Copy link
Copy Markdown
Contributor

mbondyra commented Sep 23, 2022

Rechecked, and re-approving!

stratoula
stratoula previously requested changes Sep 23, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@stratoula stratoula left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Sorry for being so annoying with the sizes but + 24KB are a lot :) Can we look into it?

image

@stratoula stratoula removed the WIP Work in progress label Sep 23, 2022
ppisljar
ppisljar approved these changes Sep 23, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

LGTM

@stratoula stratoula dismissed their stale review September 26, 2022 05:59

The bundle size has been fixed, so I am dismissing my review

@Kuznietsov
Copy link
Copy Markdown
Contributor

Kuznietsov commented Sep 26, 2022

@elasticmachine merge upstream

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented Sep 26, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
lens 905 906 +1
visTypeTable 35 42 +7
visualizations 318 352 +34
total +42

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
data 2429 2506 +77
visualizations 602 649 +47
total +124

Async chunks

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

id before after diff
lens 1.2MB 1.2MB +567.0B
visTypeTimeseries 475.4KB 475.8KB +390.0B
visualizations 218.6KB 239.2KB +20.6KB
total +21.5KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
visualizations 14 18 +4

Page load bundle

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

id before after diff
data 431.8KB 432.2KB +400.0B
lens 34.9KB 35.4KB +466.0B
visTypeTable 15.6KB 19.4KB +3.8KB
visTypeTimeseries 19.1KB 19.2KB +25.0B
visualizations 49.1KB 51.2KB +2.2KB
total +6.8KB
Unknown metric groups

API count

id before after diff
data 3132 3209 +77
visualizations 631 679 +48
total +125

async chunk count

id before after diff
visualizations 13 14 +1

History

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

cc @VladLasitsa @Kunzetsov

@Kuznietsov Kuznietsov merged commit f576b1f into elastic:main Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mbondyra mbondyra mbondyra approved these changes

@ppisljar ppisljar ppisljar approved these changes

@stratoula stratoula stratoula left review comments

@alexwizp alexwizp Awaiting requested review from alexwizp

+2 more reviewers

@Kuznietsov Kuznietsov Kuznietsov left review comments

@spalger spalger spalger approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@VladLasitsa VladLasitsa

@Kuznietsov Kuznietsov

Labels

backport:skip This PR does not require backporting Feature:Lens Feature:Vis Editor Visualization editor issues impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:large Large Level of Effort release_note:enhancement Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v8.6.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@VladLasitsa @elasticmachine @Kuznietsov @stratoula @mbondyra @flash1293 @spalger @kibana-ci @ppisljar @kibanamachine