[TSVB][Lens] Navigate to Lens context converting improvement. by Kuznietsov · Pull Request #139719 · elastic/kibana

Kuznietsov · 2022-08-30T13:21:20Z

Summary

Completes part of #138236.

Refactored code for the purpose of separating chart state and datasource/aggregations state.
Made solution extendable and stable.
Added support of counter_rate of TSVB at Lens.

Testing notes

[ link to a file with notes will be placed here ]

…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement

…ntext-converting-imporvement

flash1293

Still looking. Found two small things in the meantime

flash1293 · 2022-09-09T15:55:47Z

src/plugins/vis_types/timeseries/public/convert_to_lens/lib/convert/counter_rate.ts

+    operationType: Operations.MAX,
+    sourceField: field.name,
+    ...createColumn(series, metric, field),
+    params: { ...getFormat(series) },


"Inner" columns (which are not visible in the dimension list in Lens) shouldn't have the same label as the visible column, otherwise this is how it will look like in the chart:

It should be safe set the label to an empty string

Applies to all the cases where there is an inner column (cumulative sum, differences, moving average)

flash1293 · 2022-09-09T16:04:42Z

src/plugins/vis_types/timeseries/public/convert_to_lens/lib/metrics/filter_ratio_formula.ts

  if (aggregation.name === 'counter_rate') {
-    const numeratorFormula = constructFilterRationFormula(
-      `${aggregation.name}(max('${field}',`,
+    const numeratorFormula = `max(${constructFilterRationFormula(


some left-over max differences which should be counter_rate

flash1293

A new bunch of comments.

I didn't find the right place in the code, but time shift and normalize by time unit don't get translated anymore:

TSVB

Lens

flash1293 · 2022-09-12T08:19:34Z

src/plugins/vis_types/timeseries/public/convert_to_lens/lib/configurations/xy/configuration.ts

+      yRight: Boolean(model.show_grid),
+    },
+    yLeftExtent: extents.yLeftExtent,
+    yRightExtent: extents.yRightExtent,


Not for this PR, but I noticed we don't convert the axis scale (setting it to log doesn't carry that over to Lens but it could). I will open an issue for it

I added support of it here

src/plugins/vis_types/timeseries/public/convert_to_lens/lib/convert/terms.ts

flash1293 · 2022-09-12T08:28:48Z

src/plugins/vis_types/timeseries/public/convert_to_lens/lib/convert/terms.ts

+  if (series.terms_order_by === '_count' || !series.terms_order_by) {
+    const columnId = uuid();
+    return {
+      orderBy: { type: 'column', columnId },


orderBy should be { type: 'custom' } if you are specifying a local column, not type: 'column'. This is causing problems when navigating over:

kibana/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/types.ts

Line 26 in 004369d

| { type: 'custom' };

flash1293 · 2022-09-12T08:35:26Z

src/plugins/vis_types/timeseries/public/convert_to_lens/lib/convert/column.ts

+
+  // not supported formatters should be converted to number
+  if (!isSupportedFormat(series.formatter)) {
+    return { format: { id: DATA_FORMATTERS.NUMBER, ...(suffix && { params: { suffix } }) } };


If there is no suffix and no formatter is specified, we should leave this empty so it can fall back to the data view formatting (right now it's forcing it to number)

if we don't specify formatter (default), the series.formatter is undefined and above in code we provide empty object.

Not always - sometimes it says "default":

I guess that's the case that should be fixed here

oh, thank you Joe, I will update condition here

VladLasitsa · 2022-09-12T14:12:37Z

A new bunch of comments.

I didn't find the right place in the code, but time shift and normalize by time unit don't get translated anymore:

TSVB

Lens

Fixed

…-ref HEAD~1..HEAD --fix'

stratoula

This looks great, some comments from my side:

cumulative sum of the value count should navigate to formula in order to work properly

In main it redirects to the cumulative sum of Records which is also wrong but let's fix it here. The logic already exists to translate this to a formula.

This also happens in main but I am trying to understand why it doesn't work. (we can do it on a followup PR).
While on the timeseries mode we are allowing nested combinations of aggregations (such as overall max of average) on topN we dont. Which is the reason for that?
I have this configuration

this should be allowed, it should go to Lens with pipeline agg (this works in 8.4) - Same for percentile_rank

Kuznietsov · 2022-09-13T10:04:10Z

@stratoula, I've fixed all the comments you left. Could you, please, review the PR again? Thanks 😃

…-ref HEAD~1..HEAD --fix'

stratoula

Thanx @Kunzetsov! I can't find more regressions, this works fine! I tested almost all aggregations and settings. Everything works fine! LGTM

mbondyra

Open TSVB [Logs] Response Codes Over Time + Annotations sample data visualization.
go to annotations tab and remove annotations
The error is thrown:

VladLasitsa · 2022-09-13T13:17:40Z

Open TSVB [Logs] Response Codes Over Time + Annotations sample data visualization.

go to annotations tab and remove annotations

The error is thrown:

Fixed

ppisljar

LGTM

kibana-ci · 2022-09-13T14:17:15Z

💚 Build Succeeded

Buildkite Build
Commit: 1625b30

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`visTypeTimeseries`	363	374	+11
`visualizations`	295	304	+9
total			+20

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`	390	582	+192

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	-794.0B
`visTypeTimeseries`	459.6KB	468.8KB	+9.2KB
`visualizations`	198.4KB	198.4KB	+1.0B
total			+8.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`	15	14	-1

Page load bundle

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

id	before	after	diff
`visTypeTimeseries`	18.8KB	19.0KB	+133.0B
`visualizations`	47.5KB	49.0KB	+1.5KB
total			+1.6KB

Unknown metric groups

API count

id	before	after	diff
`lens`	641	639	-2
`visualizations`	419	611	+192
total			+190

ESLint disabled line counts

id	before	after	diff
`visTypeTimeseries`	18	19	+1

Total ESLint disabled count

id	before	after	diff
`visTypeTimeseries`	21	22	+1

History

💚 Build #71659 succeeded a23795c
💚 Build #71355 succeeded 9ca095b
💔 Build #71325 failed 1e8ef4b
💚 Build #71132 succeeded f98f3c9
💚 Build #70796 succeeded a08ef9d
💚 Build #70007 succeeded ac69c8a

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

cc @VladLasitsa @Kunzetsov

Won’t get to check this again this week, dismissing my review as it has two approvals from the team already

Kuznietsov and others added 30 commits August 15, 2022 18:18

Added NavigateToLens types.

c698e75

Removed math/overall* columns to be replaced by Formula.

de2b27b

Removed Lens specific columns/fields.

f1a628a

Added percentile column convertion.

0733449

Fixed labels.

1749b10

Add percentile_rank converter

15ab59a

Added math formula and timeScale support.

8f1ca40

Added MovingAverage and Derivative support.

6e3cfb9

Updated formula and supported metrics.

d0e7722

Fixed pipeline aggs.

e982ac7

Another fix.

471f91a

One more fix.

537d33a

CumulativeSum added.

59db553

Fixed formula, timeScale and etc.

c19acc7

Removed console.log.

a2d51eb

Other aggs to formula convertion added.

78b715d

Filter ratio added.

0549bdd

Add XY Configuration

9d1b9c9

Added last_value convertion.

7b8786f

Merge branch 'navigate-to-lens-context-converting-imporvement' of git…

8b57f8d

…hub.com:Kunzetsov/kibana into navigate-to-lens-context-converting-imporvement

Updated ColumnWithMeta types.

6766d81

Fixed for loop.

edbeea0

Added StaticValue convertion.

5221a01

Added default agg convertion.

ecdf3e2

Add filters column

faab810

Refactored filter part.

08a0539

Added meta to all the cols.

defc3a0

Fixed problem with columns.

0da77f1

Added date_histogram column, fixed layer configuration

fcdcb41

Added terms support.

bfd1dff

Merge remote-tracking branch 'upstream/main' into navigate-to-lens-co…

a08ef9d

…ntext-converting-imporvement

flash1293 reviewed Sep 9, 2022

View reviewed changes

Merge branch 'main' into navigate-to-lens-context-converting-imporvement

f98f3c9

flash1293 previously requested changes Sep 12, 2022

View reviewed changes

Fixed pipeline and terms aggs. Added timeShift arg.

3e39342

kibanamachine and others added 2 commits September 12, 2022 14:17

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

1e8ef4b

…-ref HEAD~1..HEAD --fix'

Fixed tests

9ca095b

stratoula reviewed Sep 13, 2022

View reviewed changes

Kuznietsov added 5 commits September 13, 2022 11:35

Merge branch 'main' into navigate-to-lens-context-converting-imporvement

9908612

Fixed cumulative sum by value count.

c3a443c

Fixed percentile and percentile ranks.

89f6945

Fixed tests.

cc3fcd8

One more fix.

018ea3c

VladLasitsa and others added 2 commits September 13, 2022 13:16

Fixed timeShift

3743c4d

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

a23795c

…-ref HEAD~1..HEAD --fix'

stratoula approved these changes Sep 13, 2022

View reviewed changes

mbondyra requested changes Sep 13, 2022

View reviewed changes

Fixed getDataSourceInfo method

1625b30

mbondyra approved these changes Sep 13, 2022

View reviewed changes

VladLasitsa requested a review from flash1293 September 13, 2022 14:03

ppisljar approved these changes Sep 13, 2022

View reviewed changes

Kuznietsov mentioned this pull request Sep 14, 2022

[Lens] Convert legacy visualizations to Lens Phase I #138236

Closed

28 tasks

mbondyra mentioned this pull request Sep 14, 2022

[Lens] Tsvb to lens for annotations #140718

Merged

9 tasks

Kuznietsov merged commit 4d3e76c into elastic:main Sep 15, 2022

dej611 mentioned this pull request Sep 23, 2022

[Lens] Open in Lens from TSVB: Axis scale #140482

Closed

Conversation

Kuznietsov commented Aug 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing notes

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flash1293 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VladLasitsa commented Sep 12, 2022

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

Kuznietsov commented Sep 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stratoula left a comment

Choose a reason for hiding this comment

Uh oh!

mbondyra left a comment

Choose a reason for hiding this comment

Uh oh!

VladLasitsa commented Sep 13, 2022

Uh oh!

ppisljar left a comment

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Sep 13, 2022

💚 Build Succeeded

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Kuznietsov commented Aug 30, 2022 •

edited

Loading

flash1293 left a comment •

edited

Loading

Kuznietsov commented Sep 13, 2022 •

edited

Loading