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

[TSVB][Lens] Navigate to Lens TSVB Metric #140878

Merged
merged 289 commits into from
Sep 28, 2022

Conversation

VladLasitsa
Copy link
Contributor

@VladLasitsa VladLasitsa commented Sep 16, 2022

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 TSVB metric was added.

Color rules converting supported cases:

  • single less than or equal rule;
  • all less than except the last one and the last rule is less than or equal;
  • all greater than or equal except the last one and the last rule is less than or equal;
  • all greater than or equal rules, more than one.

Kuznietsov and others added 30 commits August 23, 2022 15:08
…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement
…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement
…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement
…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement
…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement

# Conflicts:
#	src/plugins/vis_types/timeseries/public/convert_to_lens/types.ts
…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement
@Kuznietsov Kuznietsov requested a review from alexwizp September 26, 2022 09:30
@Kuznietsov Kuznietsov marked this pull request as ready for review September 26, 2022 10:33
@Kuznietsov Kuznietsov requested a review from a team as a code owner September 26, 2022 10:33
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-vis-editors @elastic/kibana-vis-editors-external (Team:VisEditors)

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

This looks pretty cool. I have added some comments on enhancing our unit and functional tests.
I have some questions about colouring:
Some rules can be converted. For example
image

This is a dynamic confuguration for the new metric viz. Why aren't we translate them?

  • For rules that can't be converted, why don't we allow the transition?

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

Static Value is not available as separate 'type' along with quick functions and formula for Lens metric primary and secondary values.

So when converting this:
Screenshot 2022-09-27 at 11 20 47
Screenshot 2022-09-27 at 11 22 36
Screenshot 2022-09-27 at 11 22 41

the config panel gets a bit lost. Maybe instead let's convert Static Value to Formula?

@flash1293
Copy link
Contributor

Agree, static value in TSVB should become a formula with just the number in it.

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

Two little things, but otherwise I couldn't find anything else

  1. There’s this panel filter field, right? I think we already agreed we're ok with ignoring it because there's metric filter (transforms to filterBy) and a filter bar but just mentioning. @stratoula can you confirm we're ok with ignoring it?
  2. When the interval is auto, we don't always correctly pass it to Lens in the last value mode. I am not sure if we can do something about it, I am ok with the answer 'no' but just mentioning to see if you see any solutions.

Screenshot 2022-09-28 at 10 10 53

Screenshot 2022-09-28 at 10 10 58

but I guess it will never be the same because reducedTimeRange is not really a bucket, is it?

@Kuznietsov
Copy link
Contributor

@elasticmachine merge upstream

@stratoula
Copy link
Contributor

@mbondyra I confirm :)

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

This looks fine to me! Thanx Yaroslav for addressing all comments! If @mbondyra is also fine, I approve! LGTM

Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

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

After talking to Vlad and Yaroslav about my doubts, approved 👌🏼 Great progress, tested and reviewed 👏🏼

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
visTypeTimeseries 373 378 +5

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
visualizations 649 663 +14

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.3MB 1.3MB +267.0B
visTypeTimeseries 475.8KB 492.4KB +16.6KB
total +16.8KB

Page load bundle

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

id before after diff
visTypeTimeseries 19.2KB 19.3KB +122.0B
Unknown metric groups

API count

id before after diff
visualizations 679 693 +14

async chunk count

id before after diff
visTypeTimeseries 13 14 +1

History

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

cc @Kunzetsov

@Kuznietsov Kuznietsov removed the WIP Work in progress label Sep 28, 2022
@Kuznietsov Kuznietsov merged commit 8f793cb into elastic:main Sep 28, 2022
kibanamachine added a commit that referenced this pull request Dec 8, 2022
# Backport

This will backport the following commits from `main` to `8.6`:
- [Adds the VisEditor docs for 8.6
(#146471)](#146471)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Kaarina
Tungseth","email":"kaarina.tungseth@elastic.co"},"sourceCommit":{"committedDate":"2022-12-08T20:18:03Z","message":"Adds
the VisEditor docs for 8.6 (#146471)\n\n## Summary\r\n\r\nAdds the 8.6
for the following:\r\n\r\n- #140878, #143946 and
#142187\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/tsvb.html#edit-visualizations-in-lens)\r\n\r\n-
#142936, #142561, #143820, and
#142838\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/add-aggregation-based-visualization-panels.html#edit-agg-based-visualizations-in-lens)\r\n\r\n-
#138732\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#change-the-fields)\r\n\r\n-
#141626 and #141615\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#add-annotations)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Stratoula Kalafateli
<stratoula1@gmail.com>","sha":"f918a3745be3badff9cf05950db61d7f47877961","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Docs","release_note:skip","v8.6.0","v8.7.0"],"number":146471,"url":"https://github.com/elastic/kibana/pull/146471","mergeCommit":{"message":"Adds
the VisEditor docs for 8.6 (#146471)\n\n## Summary\r\n\r\nAdds the 8.6
for the following:\r\n\r\n- #140878, #143946 and
#142187\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/tsvb.html#edit-visualizations-in-lens)\r\n\r\n-
#142936, #142561, #143820, and
#142838\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/add-aggregation-based-visualization-panels.html#edit-agg-based-visualizations-in-lens)\r\n\r\n-
#138732\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#change-the-fields)\r\n\r\n-
#141626 and #141615\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#add-annotations)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Stratoula Kalafateli
<stratoula1@gmail.com>","sha":"f918a3745be3badff9cf05950db61d7f47877961"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/146471","number":146471,"mergeCommit":{"message":"Adds
the VisEditor docs for 8.6 (#146471)\n\n## Summary\r\n\r\nAdds the 8.6
for the following:\r\n\r\n- #140878, #143946 and
#142187\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/tsvb.html#edit-visualizations-in-lens)\r\n\r\n-
#142936, #142561, #143820, and
#142838\r\n\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/add-aggregation-based-visualization-panels.html#edit-agg-based-visualizations-in-lens)\r\n\r\n-
#138732\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#change-the-fields)\r\n\r\n-
#141626 and #141615\r\n
\r\n[Doc\r\npreview](https://kibana_146471.docs-preview.app.elstc.co/guide/en/kibana/master/lens.html#add-annotations)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Stratoula Kalafateli
<stratoula1@gmail.com>","sha":"f918a3745be3badff9cf05950db61d7f47877961"}}]}]
BACKPORT-->

Co-authored-by: Kaarina Tungseth <kaarina.tungseth@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Lens Feature:TSVB TSVB (Time Series Visual Builder) impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. loe:medium Medium Level of Effort release_note:feature Makes this part of the condensed release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants