[XY] Migrate vis type xy to new unified xy expression

[VladLasitsa]

Completes part of #127115

Summary

Migrate existing vis type xy implementation to the new unified xy expression.
As part of this the following changes was made in unified xy expression:

• Moved curveType inside data layer expression functions as we can configure different curve types for different layers.
• Expanded constants of fitting functions
• Added supporting of vis dimension for Accessors from ExtendedDataLayer (as part of that the table arg was removed)
• Added splitColumnAccessor and splitRowAccessor for layeredXyVis function
• Added toggle legend button and legend color picker
• Added enforce argument in axis extent so that ignore extents validation
• Updated logic for x-domain to use adjusted interval by using function -> getAdjustedInterval from src/plugins/charts/public/static/components/endzones.tsx
• Fixed problem with formatting for splitted charts when we use range
• Fixed problem with scrolling of legend when we use new expression in vis editor
• Fixed problem with logging datatable for inspector (added missing accessors, log only one datatable for vis type xy)
• Fixed problem with direction of splitting by row and by column: before split by row uses splitHorizontally, now it was uses splitVertically prop.

[flash1293]

The delay agg test fails because of this:
This is how the table looks like (it's using a split series column which is always empty)

But in the new renderer, we do this (basically filtering out a table if the split series is not defined):

kibana/src/plugins/chart_expressions/expression_xy/public/helpers/layers.ts

Line 102 in 6d1ba64

splitAccessors.every((splitAccessor) => row[splitAccessor] === undefined)

I think this behavior is fine and it's kind of bad luck this test breaks. The easiest way to work around it I found is to map the shard delay agg to a chart split instead of a split series (you just have to change the schema to split instead of group for the shard delay agg in

kibana/x-pack/test/functional/fixtures/kbn_archiver/dashboard_async/async_search.json

Line 53 in 6c93e3d

"visState": "{\"title\":\"Sum of Bytes by Extension (Delayed 5s)\",\"type\":\"histogram\",\"aggs\":[{\"id\":\"1\",\"enabled\":true,\"type\":\"sum\",\"params\":{\"field\":\"bytes\"},\"schema\":\"metric\"},{\"id\":\"2\",\"enabled\":true,\"type\":\"terms\",\"params\":{\"field\":\"extension.raw\",\"orderBy\":\"1\",\"order\":\"desc\",\"size\":5,\"otherBucket\":false,\"otherBucketLabel\":\"Other\",\"missingBucket\":false,\"missingBucketLabel\":\"Missing\"},\"schema\":\"segment\"},{\"id\":\"3\",\"enabled\":true,\"type\":\"shard_delay\",\"params\":{\"delay\":\"5s\"},\"schema\":\"group\"}],\"params\":{\"type\":\"histogram\",\"grid\":{\"categoryLines\":false},\"categoryAxes\":[{\"id\":\"CategoryAxis-1\",\"type\":\"category\",\"position\":\"bottom\",\"show\":true,\"style\":{},\"scale\":{\"type\":\"linear\"},\"labels\":{\"show\":true,\"filter\":true,\"truncate\":100},\"title\":{}}],\"valueAxes\":[{\"id\":\"ValueAxis-1\",\"name\":\"LeftAxis-1\",\"type\":\"value\",\"position\":\"left\",\"show\":true,\"style\":{},\"scale\":{\"type\":\"linear\",\"mode\":\"normal\"},\"labels\":{\"show\":true,\"rotate\":0,\"filter\":false,\"truncate\":100},\"title\":{\"text\":\"Sum of bytes\"}}],\"seriesParams\":[{\"show\":true,\"type\":\"histogram\",\"mode\":\"stacked\",\"data\":{\"label\":\"Sum of bytes\",\"id\":\"1\"},\"valueAxis\":\"ValueAxis-1\",\"drawLinesBetweenPoints\":true,\"lineWidth\":2,\"showCircles\":true,\"circlesRadius\":1}],\"addTooltip\":true,\"addLegend\":true,\"legendPosition\":\"right\",\"times\":[],\"addTimeMarker\":false,\"labels\":{\"show\":false},\"thresholdLine\":{\"show\":false,\"value\":10,\"width\":1,\"style\":\"full\",\"color\":\"#E7664C\"},\"row\":true,\"palette\":{\"type\":\"palette\",\"name\":\"kibana_palette\"},\"isVislibVis\":true,\"detailedTooltip\":true,\"legendSize\":\"auto\"}}"

[elasticmachine]

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

[VladLasitsa]

@elasticmachine merge upstream

[jbudz]

limits.yml

[VladLasitsa]

@elasticmachine merge upstream

[stratoula]

Thanx Vlad! I found another bug. The overwrite color doesnt work for multiple metrics

Moreover, splitting a percentiles metric works differently. (same for percentile ranks)

In main. (check the legend order in comparison with the screenshot below)

Same chart here

[stratoula]

Also the split series order seems wrong (possibly related to the percentiles bug mentioned above)

[VladLasitsa]

@stratoula I fixed issues related to percentiles metric. Could you please re-review?

[stratoula]

Thanx Vlad! I found a bug
I have 2 metrics (both vertical bar charts) and then I go to metrics and axes and I change the first series from bar to area

[stratoula]

Another one. Here I have 2 series (count and average) I set the average as line but it is still rendered as a bar

[stratoula]

If I have 2 line series changing the dots size of the second is not applied (also the line mode and line width)

[stratoula]

I can't find any other regressions! I just think we can clean up the code. Are we planning to do it on a separate PR? For example:

• the renderer is not used any more
• the vis_component is not used any more
• we are registering some functions that also are not used any more (e.g expressions.registerFunction(expressionFunctions.visTypeXyVisFn);)

[VladLasitsa]

I can't find any other regressions! I just think we can clean up the code. Are we planning to do it on a separate PR? For example:

• the renderer is not used any more
• the vis_component is not used any more
• we are registering some functions that also are not used any more (i.e expressions.registerFunction(expressionFunctions.visTypeXyVisFn);)

@stratoula Removing all unused code from vis_type xy I will do in separate PR

[stratoula]

I have tested it multiple times and I can't find any other regressions. LGTM as long as the CI is green and we clean up the code at a separate PR.

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 32b29f5

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
expressionXY	113	114	+1

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
expressionXY	140	141	+1
lens	529	530	+1
total			+2

Async chunks

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

id	before	after	diff
expressionXY	103.4KB	109.3KB	+6.0KB
lens	1.2MB	1.2MB	-19.0B
total			+5.9KB

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
expressionXY	14	12	-2

Page load bundle

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

id	before	after	diff
expressionXY	34.0KB	35.0KB	+1.0KB
visTypeXy	43.7KB	47.1KB	+3.3KB
total			+4.4KB

Unknown metric groups

API count

id	before	after	diff
expressionXY	150	151	+1
lens	611	612	+1
total			+2

History

• 💛 Build #63900 was flaky 99ae23f
• 💚 Build #63485 succeeded 4a5d053
• 💚 Build #63171 succeeded 1a1249b
• 💚 Build #62758 succeeded bddd5aa
• 💔 Build #62727 failed 4b4544d
• 💔 Build #62721 failed 8a756f8

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

cc @VladLasitsa

[flash1293]

This looks mostly fine - however I noticed one weird thing - it seems like for percentage charts the old renderer is ignoring the "mode" while the new one is respecting it:

Not 100% sure what to do about. What do you think @stratoula ? IMHO we should keep the existing behavior on the expression building level (if percentage and bar, then always use stacked)

[stratoula]

@flash1293 if I recall correctly, when we firstly used the EC library on the agg based XV visualizations, we had decided that for percentage mode it should always be stacked but we had some complains, this is the reason we decided to also allow the Normal mode on the percentage bars. #118525

[flash1293]

@stratoula OK, does this mean there is a bug in the current main as it's not respecting "normal" mode?

[stratoula]

@flash1293 I am on main and it is respected (haven't tested this PR though)

[stratoula]

@flash1293 it is not respected for percentiles on main

[flash1293]

@stratoula I tested with percentiles, forgot to mention, sorry - it works fine on main for regular series

[flash1293]

As this is an edge case and the new state is more "correct" IMHO I think we can keep this.

[stratoula]

Agreed!

[flash1293]

@VladLasitsa This looks almost perfect, I just noticed one thing - the render UI counters introduced by Alexey went from render_agg_based_vertical_bar to render_agg_based_vertical_bar_stacked (and so on). Could you take a look at that? Ideally we keep it consistent with before (otherwise telemetry will look weird for the version switch)

[alexwizp]

@VladLasitsa This looks almost perfect, I just noticed one thing - the render UI counters introduced by Alexey went from render_agg_based_vertical_bar to render_agg_based_vertical_bar_stacked (and so on). Could you take a look at that? Ideally we keep it consistent with before (otherwise telemetry will look weird for the version switch)

it was discussed offline, decided to keep as it