Replace flot with elastic-chart in Timelion by VladLasitsa · Pull Request #81565 · elastic/kibana

VladLasitsa · 2020-10-23T16:02:24Z

Closes: #79984, #74888, #69122, #10810

Summary

Migrated timelion from jquery-flot to elastic-charts

Problems which I found described in the following issues: elastic/elastic-charts#878 and elastic/elastic-charts#893

Here points that we should do:

Should update documentation for timelion (https://www.elastic.co/guide/en/kibana/current/timelion.htm)

Below you can find some screenshots to compare timelion with jquery-flot and elastic-charts:

.es(index=metricbeat-*, timefield='@timestamp', metric='avg:system.cpu.user.pct')

timelion with jquery-flot (as now we have in master):

timelion with elastic-charts (this PR):

.es(offset=-1h, index=metricbeat-*, timefield='@timestamp', metric='avg:system.cpu.user.pct').label('last hour').lines(fill=1,width=0.5).color(gray), .es(index=metricbeat-*, timefield='@timestamp', metric='avg:system.cpu.user.pct').label('current hour').title('CPU usage over time').color(#1E90FF)

timelion with jquery-flot (as now we have in master):

timelion with elastic-charts (this PR):

.es(index=metricbeat*, timefield=@timestamp, metric=max:system.network.in.bytes).derivative().divide(1048576), .es(index=metricbeat*, timefield=@timestamp, metric=max:system.network.out.bytes).derivative().multiply(-1).divide(1048576)

timelion with jquery-flot (as now we have in master):

timelion with elastic-charts (this PR):

Checklist

Delete any items that are not applicable to this PR.

Documentation was added for features that require explanation or tutorials

For maintainers

This was checked for breaking API changes and was labeled appropriately

VladLasitsa · 2020-11-03T07:45:09Z

@elasticmachine merge upstream

src/plugins/vis_type_timelion/public/components/area_series/index.tsx

src/plugins/vis_type_timelion/public/components/bar_series/index.tsx

src/plugins/vis_type_timelion/public/helpers/panel_utils.ts

src/plugins/vis_type_timelion/public/components/timelion_vis_component.tsx

alexwizp · 2020-11-05T10:25:37Z

src/plugins/vis_type_timelion/public/components/timelion_vis_component.tsx

+        {chart[0]._global && chart[0]._global.yaxes ? (
+          chart[0]._global.yaxes.map((axis: IAxis, index: number) => {
+            return (
+              <Axis
+                key={index}
+                id={axis.position + axis.axisLabel}
+                title={axis.axisLabel}
+                position={axis.position}
+                tickFormat={axis.tickFormatter}
+                gridLine={{
+                  stroke: 'rgba(125,125,125,0.3)',
+                  visible: true,
+                }}
+                domain={axis.domain as YDomainRange}
+              />
+            );
+          })
+        ) : (
+          <Axis
+            id="left"
+            position={Position.Left}
+            gridLine={{
+              stroke: 'rgba(125,125,125,0.3)',
+              visible: true,
+            }}
+          />


it's too difficult to read. Maybe we should name it somehow or move into separate component?

I don't think it's needed. Create a separate document just for rendering Axis it's strange for me. Maybe do you want that I create s separate function for rendering Axis?

VladLasitsa · 2020-11-06T07:45:49Z

@elasticmachine merge upstream

stratoula

Thanx Vlad for this PR. Some comments from my side:

It seems that Timelion app is not working anymore

- The bar's width scales differently. And the red line that follows user's mouse is huge in some cases. Here I have set the bard width on 1 but it is definitely not 1. Moreover the red line has the same width as the bars. It shouldn't

- .multiply(-1) creates a weird chart

While in 7.10

src/plugins/vis_type_timelion/public/components/_timelion_vis.scss

src/plugins/vis_type_timelion/public/components/area_series/index.tsx

src/plugins/vis_type_timelion/public/components/timelion_vis_component.tsx

src/plugins/vis_type_timelion/public/helpers/panel_utils.ts

stratoula

So this works pretty well! I have added some comments about the new setting.
I can only see a difference between the two implementations with this expression:
.static(50).points(symbol=cross, radius=2)

On the old implementation:

On the new:

Two comments here:

The static value appears twice in the legend
In the new implementation, the y axis has values from 0 to 50, while on the old implementation the y axis starts from 49.9925. As a result the static value is being displayed in the center.

stratoula · 2021-07-20T10:28:21Z

docs/management/advanced-options.asciidoc

 of buckets to try to represent.

+[[timelion-legacyChartsLibrary]]`timelion:legacyChartsLibrary`::
+Enables legacy charts library for Timelion visualizations.


Enables the legacy charts library for timelion charts in Visualize.

stratoula · 2021-07-20T10:42:36Z

src/plugins/vis_type_timelion/public/components/series/area.tsx

+  groupId: string;
+}
+
+const isShowLines = (lines: VisSeries['lines'], points: VisSeries['points']) =>


nit: showLines

stratoula · 2021-07-20T10:45:00Z

src/plugins/vis_type_timelion/public/components/timelion_vis_component.tsx

+      fit: false,
+    })}
+    position={Position.Left}
+    groupId={`${MAIN_GROUP_ID}`}


nit: I haven't tested it but why not writing it a bit simpler groupId={MAIN_GROUP_ID}

stratoula · 2021-07-20T10:51:18Z

src/plugins/vis_type_timelion/server/ui_settings.ts

+  return {
+    [UI_SETTINGS.LEGACY_CHARTS_LIBRARY]: {
+      name: i18n.translate('timelion.uiSettings.legacyChartsLibraryLabel', {
+        defaultMessage: 'Legacy charts library',


Timelion legacy charts library

VladLasitsa · 2021-07-27T15:19:23Z

So this works pretty well! I have added some comments about the new setting.
I can only see a difference between the two implementations with this expression:
.static(50).points(symbol=cross, radius=2)

On the old implementation:

On the new:

Two comments here:

The static value appears twice in the legend

In the new implementation, the y axis has values from 0 to 50, while on the old implementation the y axis starts from 49.9925. As a result the static value is being displayed in the center.

@stratoula, I checked TSVB and Lens with static value and get the following:
Lens:

TSVB:

As you can see elastic-charts works with static value everywhere like in timelion now. I understand that is not like before but I think we can it leave this behavior so that we will be consistent. Also about legend, elastic-chart show in legend the last value of graphic if we don't have cursor. As we have name (50) and value (50) we see twice it. The similar behavior you can see in TSVB:

VladLasitsa · 2021-07-27T15:22:53Z

@elasticmachine merge upstream

stratoula

Ok I see, thanx @VladLasitsa I think we can merge it as it is.
LGTM, I have tested it multiple times and taking under consideration that we also have the fallback on the old implementation I think it is ready to be merged 🎉

stratoula · 2021-07-27T15:50:04Z

@elastic/kibana-telemetry can you please take a look? 🙇‍♀️

Bamieh

telemetry changes LGTM. mikhail already approved this so we are not blocking the PR. seems it needs a review from the kibana-design team

stratoula · 2021-07-27T16:07:30Z

Correct! @elastic/kibana-design we need your review ❤️

alexwizp

LGTM! Tested locally

alexwizp · 2021-07-29T19:40:48Z

Correct! @elastic/kibana-design we need your review ❤️

@elastic/kibana-design please have a look, we really want to merge it 🙏

ryankeairns

LGTM. It appears Michael's suggestions were addressed as well.

stratoula · 2021-08-02T06:29:19Z

@elasticmachine merge upstream

kibanamachine · 2021-08-02T08:53:00Z

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`visTypeTimelion`	48	66	+18

Async chunks

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

id	before	after	diff
`timelion`	197.4KB	197.4KB	+15.0B
`visTypeTimelion`	69.1KB	89.3KB	+20.2KB
total			+20.2KB

Page load bundle

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

id	before	after	diff
`core`	421.5KB	421.5KB	+78.0B
`visTypeTimelion`	24.4KB	24.6KB	+262.0B
total			+340.0B

Unknown metric groups

async chunk count

id	before	after	diff
`visTypeTimelion`	3	4	+1

History

💚 Build #141557 succeeded bf63108
💛 Build #141033 was flaky 56a94b4
💚 Build #139677 succeeded 757e95a
💚 Build #138370 succeeded eac6a41
💚 Build #138066 succeeded 4d5292b

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

cc @VladLasitsa

* First draft migrate timelion to elastic-charts * Some refactoring. Added brush event. * Added title. Some refactoring * Fixed some type problems. Added logic for yaxes function * Fixed some types, added missing functionality for yaxes * Fixed some types, added missing functionality for stack property * Fixed unit test * Removed unneeded code * Some refactoring * Some refactoring * Fixed some remarks. * Fixed some styles * Added themes. Removed unneeded styles in BarSeries * removed unneeded code. * Fixed some comments * Fixed vertical cursor across Timelion visualizations of a dashboad * Fix some problems with styles * Use RxJS instead of jQuery * Remove unneeded code * Fixed some problems * Fixed unit test * Fix CI * Fix eslint * Fix some gaps * Fix legend columns * Some fixes * add 2 versions of Timeline app * fix CI * cleanup code * fix CI * fix legend position * fix some cases * fix some cases * remove extra casting * cleanup code * fix issue with static * fix header formatter * fix points * fix ts error * Fix yaxis behavior * Fix some case with yaxis * Add deprecation message and update asciidoc * Fix title * some text improvements Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Alexey Antonov <alexwizp@gmail.com>

kibanamachine · 2021-08-02T08:55:38Z

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.

kibanamachine · 2021-08-04T08:58:41Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

* First draft migrate timelion to elastic-charts * Some refactoring. Added brush event. * Added title. Some refactoring * Fixed some type problems. Added logic for yaxes function * Fixed some types, added missing functionality for yaxes * Fixed some types, added missing functionality for stack property * Fixed unit test * Removed unneeded code * Some refactoring * Some refactoring * Fixed some remarks. * Fixed some styles * Added themes. Removed unneeded styles in BarSeries * removed unneeded code. * Fixed some comments * Fixed vertical cursor across Timelion visualizations of a dashboad * Fix some problems with styles * Use RxJS instead of jQuery * Remove unneeded code * Fixed some problems * Fixed unit test * Fix CI * Fix eslint * Fix some gaps * Fix legend columns * Some fixes * add 2 versions of Timeline app * fix CI * cleanup code * fix CI * fix legend position * fix some cases * fix some cases * remove extra casting * cleanup code * fix issue with static * fix header formatter * fix points * fix ts error * Fix yaxis behavior * Fix some case with yaxis * Add deprecation message and update asciidoc * Fix title * some text improvements Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Alexey Antonov <alexwizp@gmail.com> Co-authored-by: Uladzislau Lasitsa <Uladzislau_Lasitsa@epam.com> Co-authored-by: Alexey Antonov <alexwizp@gmail.com>

* First draft migrate timelion to elastic-charts * Some refactoring. Added brush event. * Added title. Some refactoring * Fixed some type problems. Added logic for yaxes function * Fixed some types, added missing functionality for yaxes * Fixed some types, added missing functionality for stack property * Fixed unit test * Removed unneeded code * Some refactoring * Some refactoring * Fixed some remarks. * Fixed some styles * Added themes. Removed unneeded styles in BarSeries * removed unneeded code. * Fixed some comments * Fixed vertical cursor across Timelion visualizations of a dashboad * Fix some problems with styles * Use RxJS instead of jQuery * Remove unneeded code * Fixed some problems * Fixed unit test * Fix CI * Fix eslint * Fix some gaps * Fix legend columns * Some fixes * add 2 versions of Timeline app * fix CI * cleanup code * fix CI * fix legend position * fix some cases * fix some cases * remove extra casting * cleanup code * fix issue with static * fix header formatter * fix points * fix ts error * Fix yaxis behavior * Fix some case with yaxis * Add deprecation message and update asciidoc * Fix title * some text improvements Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Alexey Antonov <alexwizp@gmail.com>

VladLasitsa added 6 commits October 23, 2020 18:58

First draft migrate timelion to elastic-charts

ff810c8

Some refactoring. Added brush event.

6d7f157

Merge remote-tracking branch 'origin/elastic#79984' into elastic#79984

b888e79

Added title. Some refactoring

999c2d7

Fixed some type problems. Added logic for yaxes function

f43a965

Fixed some types, added missing functionality for yaxes

af60792

VladLasitsa requested review from alexwizp and stratoula October 30, 2020 13:40

Fixed some types, added missing functionality for stack property

f429afe

kibanamachine and others added 4 commits November 3, 2020 02:45

Merge branch 'master' into elastic#79984

c94a3fc

Fixed unit test

5cd3a48

Removed unneeded code

4b3a294

Some refactoring

1e7eee7