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

Replace TSVB timeseries charts with elastic-charts #33558

Merged
merged 14 commits into from
Sep 6, 2019

Conversation

sulemanof
Copy link
Contributor

@sulemanof sulemanof commented Mar 20, 2019

Resolve elastic/elastic-charts#108 .

Should be checked before merging:

In progress replacements:

  • Stacked option (Stacked/Percent);
  • Line width;
  • Point size;
  • Hide in legend;
  • Annotations;

Blocker Issues:

Tests sample dashboards ⚠️

Tasks

  • Install all 3 data sample
  • check each dashboard for errors
  • change size of TSVB visualizations
  • compare them with master version or 7.3 version

Issues 🐛

  • [eCommerce] Promotion Tracking, [Flights] Delays & Cancellations, also on visualization editor, the visualization doesn't show the crosshair line (usually red)

Screenshot 2019-08-09 at 12 04 26

  • [Flights] Delays & Cancellations disable tooltip on annotation line (/Disable tooltip on annotation line elastic-charts#324)
  • [Logs] Response Codes Over Time + Annotations the order of the series is inverted, meaning that you have to reorder the way you are adding the series into the Chart component. It's reproducible when using stacked mode on a single visualization with a group by split
    @AlonaNadler please have a look.
    this PR

Screenshot 2019-08-09 at 11 38 09

on 7.3
Screenshot 2019-08-09 at 11 37 36

  • [Logs] Response Codes Over Time + Annotations the annotation header seems always showing the same timestamp. Also reproducible during tests on Annotations
    Aug-09-2019 11-41-06
  • [Logs] Response Codes Over Time + Annotations I think you are not checking the formatter for null values as in a case you can see the NaN text here

Screenshot 2019-08-09 at 11 44 26


Test visualization editor ⚠️

Tasks

Build a single series visualization

On Panel options

  • specify kibana_sample_data_flights and choose timestamp as Timestamp
  • check if Drop last bucket shows and hide the last bucket on the series
  • choose last 24h on time window, change interval to 1h, 1d, 1m, 2h.
  • choose last 24h on time window, change interval to 1h, 1d, 1m, 2h using bar chart

On Data -> Options tab of a single series:

  • check formatting of tooltip/y axis values
  • check Chart type line, bar
  • Chart type styles: fill, line width, point size, steps
  • Check the offset series, hide in legend , split color theme (only when multi series), separate axis, override index pattern using kibana_sample_data_logs
  • check for timezone change (change timezone in kibana from Advanced Settings)

On Data -> Metrics tab of a single series and with multiple series:

  • Test Average aggregation on AvgTicketPrice
  • Test Group by: Terms by: Origin
  • Check the above Data -> options options
  • Check stacked mode: percent, stacked, stacked within series: with a single series, with 2 series, with a single series splitted, with one series splitted and one not.

On Annotations tab

  • add a kibana_sample_data_flights index pattern ,use Dest : "Verona Villafranca Airport" and Cancelled :true as KQL query, select an Icon, use Dest as Fields and {{Dest}} as row template.
  • check the rendering of the annotation, positioning, colors, icons, rendered text

Issues 🐛

  • the timepicker window changed its position from the original top right position:

Screenshot 2019-08-09 at 11 55 28

  • Line width option with value 0 doesn't hide/reduce the line width size to 0. Still to 1. Works fine for point size 0:

  • [x] The point size works fine until the value of 10, after that, 11 or more there is no more fill:
    this is not an issue, but a feature existing on 7.3 also :(

point size: 10
Screenshot 2019-08-09 at 10 53 42

point size: 11 or above
Screenshot 2019-08-09 at 10 53 54

  • Fill and Line width styles are not applied to Bars
  • Hide in legend seems not working with a single series
  • Split within series doesn't seem to work.
  • ask @elastic/kibana-design about the status of the Exclamation in circle icon Exclamation mark in circle icon eui#2121

Screenshot 2019-08-09 at 13 01 29


Test PDF rendering ✅

Enable trial mode Start a 30-day trial from Settings -> Licence Manager

  • Create a set of TSVB visualization
  • Click on Share -> PNG report -> Generate PNG
  • Click on Share -> PDF report -> Generate PDF
  • Create a dashboard with 10 TSVB visualization
  • Click on Share -> PNG report -> Generate PNG
  • Click on Share -> PDF report -> Generate PDF

New issues fixed by this PR 🎉

  • When using step line, the current points shows the right value, not showing a tooltip on the same x value with two different y values

before:
Aug-09-2019 12-18-38

now:

Aug-09-2019 12-19-28

  • When we have no value on a bucket the legend now doesn't display a wrong 0 value:
    on PR

Screenshot 2019-08-09 at 12 32 30

on master/7.3
Screenshot 2019-08-09 at 12 32 36

  • when using stacked in percentage mode, it automatically apply the % sign to the data format

  • when using a different timezone than the one in the browser, on the previous version each ticks where rounded to the UTC timezone, losing the nice rounding of the hours/days 00:00, 12:00, 00:00, 12:00

Screenshot 2019-08-09 at 13 23 34

This PR fix that keeping the same rouding
Screenshot 2019-08-09 at 13 23 26


Notes for testing

Compere the visualizations against master branch or at least against 7.3.
Using docker with 7.3 is a bit more easy: these are the steps for that:
Write a docker-compose.yaml file and run it with docker-compose up:

version: "3"
services:
  elasticsearch:
    environment:
      - reindex.remote.whitelist=docker.for.mac.host.internal:9200
      - discovery.type=single-node
    image: docker.elastic.co/elasticsearch/elasticsearch:7.3.0
    container_name: elasticsearch_7_3_0
    ports:
      - 9273:9200
  kibana:
    image: docker.elastic.co/kibana/kibana:7.3.0
    container_name: kibana_7_3_0
    ports:
      - 5673:5601

To use exactly the same dataset (that will allows you to compare also the visualization output use the following from kibana dev tools) this will reindex all the data from the testing elasticsearch into the dockerized ES

# DELETE ALL SAMPLE DOCUMENTS

POST kibana_sample_data_logs/_delete_by_query
{
  "query": {
    "match_all": {}
  }
}

POST kibana_sample_data_ecommerce/_delete_by_query
{
  "query": {
    "match_all": {}
  }
}

POST kibana_sample_data_flights/_delete_by_query
{
  "query": {
    "match_all": {}
  }
}

# REINDEX ALL SAMPLE DOCUMENTS FROM TESTING ES
POST _reindex
{
  "source": {
    "remote": {
      "host": "http://docker.for.mac.host.internal:9200",
      "username": "elastic",
      "password": "changeme"
    },
    "index": "kibana_sample_data_logs"
  },
  "dest": {
    "index": "kibana_sample_data_logs"
  }
}
POST _reindex
{
  "source": {
    "remote": {
      "host": "http://docker.for.mac.host.internal:9200",
      "username": "elastic",
      "password": "changeme"
    },
    "index": "kibana_sample_data_ecommerce"
  },
  "dest": {
    "index": "kibana_sample_data_ecommerce"
  }
}
POST _reindex
{
  "source": {
    "remote": {
      "host": "http://docker.for.mac.host.internal:9200",
      "username": "elastic",
      "password": "changeme"
    },
    "index": "kibana_sample_data_flights"
  },
  "dest": {
    "index": "kibana_sample_data_flights"
  }
}

Notes for Elastic-Charts 🐛

  • check the NaN case. The chart doesn't display an interruption of data between the values, but it just connect the available points with a line: this is an issue with the fieldFormatter used in TSVB

Screenshot 2019-08-09 at 11 47 59

Screenshot 2019-08-09 at 11 25 50

**on this PR**

Screenshot 2019-08-09 at 11 25 57

Screenshot 2019-08-09 at 12 02 55

  • check the missing ticks (probably just update the ech lib to latest)

Screenshot 2019-08-09 at 12 09 29

@sulemanof sulemanof force-pushed the charts/test-lines-series branch from ed102dd to 6ae639c Compare March 22, 2019 11:27
@sulemanof sulemanof requested a review from a team as a code owner March 22, 2019 11:27
@sulemanof sulemanof added the WIP Work in progress label Mar 22, 2019
@sulemanof sulemanof force-pushed the charts/test-lines-series branch 2 times, most recently from 5d48034 to 7904847 Compare March 29, 2019 09:58
@sulemanof sulemanof changed the title [WIP] Test charts - line series [WIP] Replace TSVB timeseries charts with elastic-charts Mar 31, 2019
@sulemanof sulemanof force-pushed the charts/test-lines-series branch from 7904847 to 891200b Compare April 1, 2019 09:30
@elasticmachine
Copy link
Contributor

💔 Build Failed

@sulemanof
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@sulemanof sulemanof requested review from markov00 and alexwizp April 2, 2019 08:41
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@sulemanof sulemanof requested a review from a team April 12, 2019 08:58
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@PhilippBaranovskiy PhilippBaranovskiy left a comment

Choose a reason for hiding this comment

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

hey @sulemanof, I guess you might leave conflict marks in the code

test/functional/apps/visualize/_tsvb_chart.ts Outdated Show resolved Hide resolved
test/functional/apps/visualize/_tsvb_chart.ts Outdated Show resolved Hide resolved
test/functional/apps/visualize/_tsvb_chart.ts Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💔 Build Failed

@gospodarsky gospodarsky force-pushed the charts/test-lines-series branch from 829404c to 197491f Compare April 15, 2019 13:26
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alexwizp alexwizp added v7.5.0 v8.0.0 Team:Visualizations Visualization editors, elastic-charts and infrastructure labels Sep 3, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

…-series

# Conflicts:
#	test/functional/apps/visualize/_tsvb_time_series.ts
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@nickofthyme nickofthyme dismissed markov00’s stale review September 4, 2019 16:56

requested change made in 6c17fb8

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alexwizp alexwizp merged commit ab24359 into elastic:master Sep 6, 2019
alexwizp pushed a commit to alexwizp/kibana that referenced this pull request Sep 6, 2019
* Replace TSVB timeseries charts with elastic-charts

* Add sort index for series

* Update src/legacy/core_plugins/metrics/public/visualizations/views/timeseries/index.js

Co-Authored-By: Nick Partridge <nick.ryan.partridge@gmail.com>

* Fix PR comments

* fix issue with scaling

* fix crosshair styles for bar
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 6, 2019
…ete-for-distance_feature

* 'master' of github.com:elastic/kibana: (89 commits)
  Replace TSVB timeseries charts with elastic-charts (elastic#33558)
  [TSVB][Top N aggregation] Unable to deal with negative values (elastic#43581)
  [alerting] Adds Action Type configuration support and whitelisting (elastic#44483)
  FTR: fix WebDriver Actions calls (elastic#44605)
  [Code] add NodeRepositoriesService to watch new repositories on local node (elastic#44677)
  [skip-ci][Maps] Improve Maps intro page (elastic#44721)
  [Maps] Update titles and descriptions for data sources (elastic#44833)
  Types + Extract Integration Util (elastic#44433)
  Downgrade log level from info to debug for cases when we cannot handle authentication attempt. (elastic#44933)
  [Reporting] Remove Chome stdout/stderr observables, Add Browser Logger observable (elastic#44359)
  Update Jest script to output coverage (elastic#44447)
  [ftr] support --kibana-install-dir flag (elastic#44552)
  [WATCHER] Allow user to set a threshold value of 0 (elastic#44810)
  Remove injectI18n in dashboard plugin. (elastic#44580)
  [Graph] Save modal (elastic#44261)
  Use external script for the OIDC Implicit flow handler page. (elastic#44866)
  disable router prefixing with pluginId (elastic#44855)
  [SIEM] Fix bug on url + inspect functionality on hosts/hostDetails page (elastic#44671)
  [ML] File data viz limiting uploaded doc chunk size (elastic#44768)
  [code] Append go env variable 'GOCACHE' to go lsp spawn command. (elastic#44864)
  ...
alexwizp added a commit that referenced this pull request Sep 6, 2019
* Replace TSVB timeseries charts with elastic-charts

* Add sort index for series

* Update src/legacy/core_plugins/metrics/public/visualizations/views/timeseries/index.js

Co-Authored-By: Nick Partridge <nick.ryan.partridge@gmail.com>

* Fix PR comments

* fix issue with scaling

* fix crosshair styles for bar
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 6, 2019
…plate

* 'master' of github.com:elastic/kibana: (91 commits)
  [APM] Make number of x ticks responsive to the plot width (elastic#44870)
  [ML] Single metric viewer: Fix top nav refresh behaviour. (elastic#44860)
  Replace TSVB timeseries charts with elastic-charts (elastic#33558)
  [TSVB][Top N aggregation] Unable to deal with negative values (elastic#43581)
  [alerting] Adds Action Type configuration support and whitelisting (elastic#44483)
  FTR: fix WebDriver Actions calls (elastic#44605)
  [Code] add NodeRepositoriesService to watch new repositories on local node (elastic#44677)
  [skip-ci][Maps] Improve Maps intro page (elastic#44721)
  [Maps] Update titles and descriptions for data sources (elastic#44833)
  Types + Extract Integration Util (elastic#44433)
  Downgrade log level from info to debug for cases when we cannot handle authentication attempt. (elastic#44933)
  [Reporting] Remove Chome stdout/stderr observables, Add Browser Logger observable (elastic#44359)
  Update Jest script to output coverage (elastic#44447)
  [ftr] support --kibana-install-dir flag (elastic#44552)
  [WATCHER] Allow user to set a threshold value of 0 (elastic#44810)
  Remove injectI18n in dashboard plugin. (elastic#44580)
  [Graph] Save modal (elastic#44261)
  Use external script for the OIDC Implicit flow handler page. (elastic#44866)
  disable router prefixing with pluginId (elastic#44855)
  [SIEM] Fix bug on url + inspect functionality on hosts/hostDetails page (elastic#44671)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:TSVB TSVB (Time Series Visual Builder) release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace TSVB timeseries charts with elastic-charts