[TSVB] Fix shard failures are not reported by alexwizp · Pull Request #123474 · elastic/kibana

alexwizp · 2022-01-20T15:37:32Z

🏁 Shard failures are not reported #122944

Describe the feature:

TSVB does not report shard failures as long as there's at least one shard successfully returning data. This can make it hard to debug issues and bears a risk of misinterpreting charts because parts of data are silently missing

Screens:

Screen.Recording.2022-01-21.at.6.45.33.PM.mov

🏁 Allow Elasticsearch request bodies to be displayed #15424

Describe the feature:

For most of the Kibana visualizations, there's a UP arrow button in the lower left corner when on the Visualize tab that allows the user to see the data in table view, the raw request + response, and query stats. This doesn't seem to exist for the Time Series Visual Builder visualization.

I know it's more complicated because the TSVB supports multiple queries being executed but it'd be useful to see the request/response like with other visualizations.

Screens:

Screen.Recording.2022-01-21.at.6.46.14.PM.mov

Closes: elastic#122944

elasticmachine · 2022-01-21T15:55:10Z

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

alexwizp · 2022-01-24T08:38:16Z

@elasticmachine merge upstream

flash1293 · 2022-01-24T17:05:11Z

Not sure whether related to this PR, but on adding an annotations source, TSVB does a lot of requests (eventually it stops though):

flash1293

Some nits, but works great - thanks for this! We should look into the annotations thing though to make sure it's not caused by this PR - I tested on 8.0-beta and it's not happening there for some reason.

Otherwise this LGTM

src/plugins/vis_types/timeseries/server/ui_settings.ts

src/plugins/vis_types/timeseries/common/types/vis_data.ts

alexwizp · 2022-01-25T08:39:25Z

@elasticmachine merge upstream

flash1293

Tested and works well. LGTM 👍

src/plugins/vis_types/timeseries/server/ui_settings.ts

# Conflicts: # src/plugins/vis_types/timeseries/public/metrics_type.ts

kibana-ci · 2022-01-26T11:42:35Z

💚 Build Succeeded

Metrics [docs]

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
`data`	2767	2771	+4

Async chunks

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

id	before	after	diff
`visTypeTimeseries`	447.8KB	447.9KB	+69.0B

Page load bundle

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

id	before	after	diff
`data`	448.9KB	449.0KB	+53.0B
`inspector`	22.3KB	22.3KB	+38.0B
`visTypeTimeseries`	14.8KB	15.5KB	+741.0B
total			+832.0B

Unknown metric groups

API count

id	before	after	diff
`data`	3364	3368	+4

History

💔 Build #19622 failed 5ac0811
💚 Build #19340 succeeded 5a16f95
💚 Build #19296 succeeded 9183b31
💚 Build #18997 succeeded 9035822
💚 Build #18904 succeeded 831f03e

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

cc @alexwizp

alexwizp · 2022-01-26T13:36:00Z

@elastic/infra-monitoring-ui @elastic/kibana-app-services please have a look

Dosant

Nice improvement 👍 From my and app services code perspective lgtm

But have some questions/suggestions for TSVB code. Maybe some of the worth an issue:

It seems like error path of TSVB is handled poorly and hides a lot of info

For example:

this is what I got:
All shards failed

And this is how much info inside the es response:

{"errBody":{"is_partial":true,"is_running":false,"start_time_in_millis":1643284656920,"expiration_time_in_millis":1643284716920,"response":{"took":3,"timed_out":false,"terminated_early":false,"num_reduce_phases":0,"_shards":{"total":1,"successful":0,"skipped":0,"failed":1,"failures":[{"shard":0,"index":"kibana_sample_data_ecommerce","node":"aTguCaz_QamVRWwzvm0LGA","reason":{"type":"query_shard_exception","reason":"failed to create query: [kibana_sample_data_ecommerce][0] Watch out!","index_uuid":"kq37gPxmRBSoaQs0xhbGnA","index":"kibana_sample_data_ecommerce","caused_by":{"type":"runtime_exception","reason":"[kibana_sample_data_ecommerce][0] Watch out!"}}}]},"hits":{"total":{"value":0,"relation":"gte"},"max_score":null,"hits":[]}},"error":{"type":"status_exception","reason":"error while executing search","caused_by":{"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"kibana_sample_data_ecommerce","node":"aTguCaz_QamVRWwzvm0LGA","reason":{"type":"query_shard_exception","reason":"failed to create query: [kibana_sample_data_ecommerce][0] Watch out!","index_uuid":"kq37gPxmRBSoaQs0xhbGnA","index":"kibana_sample_data_ecommerce","caused_by":{"type":"runtime_exception","reason":"[kibana_sample_data_ecommerce][0] Watch out!"}}}]}}},"statusCode":400}

Currently errored es responses aren't propagated through trackedEsSearches. I think this would be very useful to add.
FYI, there is a testing module in es that could help with testing errors: elastic/elasticsearch#71674

Dosant · 2022-01-27T11:59:11Z

...ins/vis_types/timeseries/server/lib/search_strategies/strategies/abstract_search_strategy.ts

          )
+          .pipe(
+            tap((data) => {
+              if (trackingEsSearchMeta?.requestId && trackedEsSearches) {


I think that would be nice if this code also executed when searching request errors out completely.

Dosant · 2022-01-27T12:00:06Z

src/plugins/vis_types/timeseries/server/ui_settings.ts

    }),
    schema: schema.boolean(),
  },
+  [UI_SETTINGS.ALLOW_CHECKING_FOR_FAILED_SHARDS]: {


Not sure I understand the reason for adding the uiSetting

flash1293 · 2022-01-27T13:10:46Z

Agreed, reporting errors in the same way would be a nice addition. However I think it's fine to split that out into a separate PR as the issue is about shard warnings which were hidden completely from the users point of view previously (in case all shards fail it's very obvious from the state of the chart).

* [TSVB] Fix shard failures are not reported elastic#122944 Closes: elastic#122944 * fix PR comments * Update ui_settings.ts Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* [TSVB] Fix shard failures are not reported elastic#122944 Closes: elastic#122944 * fix PR comments * Update ui_settings.ts Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 7e1b780) # Conflicts: # src/plugins/vis_types/timeseries/common/constants.ts # src/plugins/vis_types/timeseries/kibana.json # src/plugins/vis_types/timeseries/public/metrics_type.ts # src/plugins/vis_types/timeseries/server/ui_settings.ts # x-pack/plugins/infra/server/lib/adapters/metrics/kibana_metrics_adapter.ts

* [TSVB] Fix shard failures are not reported (#123474) * [TSVB] Fix shard failures are not reported #122944 Closes: #122944 * fix PR comments * Update ui_settings.ts Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 7e1b780) # Conflicts: # src/plugins/vis_types/timeseries/common/constants.ts # src/plugins/vis_types/timeseries/kibana.json # src/plugins/vis_types/timeseries/public/metrics_type.ts # src/plugins/vis_types/timeseries/server/ui_settings.ts # x-pack/plugins/infra/server/lib/adapters/metrics/kibana_metrics_adapter.ts * fix CI Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* [TSVB] Fix shard failures are not reported elastic#122944 Closes: elastic#122944 * fix PR comments * Update ui_settings.ts Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 7e1b780) # Conflicts: # src/plugins/vis_types/timeseries/kibana.json # src/plugins/vis_types/timeseries/public/metrics_type.ts

* [TSVB] Fix shard failures are not reported (#123474) * [TSVB] Fix shard failures are not reported #122944 Closes: #122944 * fix PR comments * Update ui_settings.ts Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 7e1b780) # Conflicts: # src/plugins/vis_types/timeseries/kibana.json # src/plugins/vis_types/timeseries/public/metrics_type.ts * fix * fix telemetry Co-authored-by: Alexey Antonov <alexwizp@gmail.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

alexwizp changed the title ~~[TSVB] Fix shard failures are not reported #122944~~ [TSVB] Fix shard failures are not reported Jan 21, 2022

[TSVB] Fix shard failures are not reported elastic#122944

831f03e

Closes: elastic#122944

alexwizp force-pushed the shards branch from 69f4e1e to 831f03e Compare January 21, 2022 14:58

alexwizp self-assigned this Jan 21, 2022

alexwizp added v8.1.0 release_note:feature Makes this part of the condensed release notes Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// labels Jan 21, 2022

alexwizp marked this pull request as ready for review January 21, 2022 15:55

alexwizp requested review from a team as code owners January 21, 2022 15:55

Merge branch 'main' into shards

9035822

flash1293 reviewed Jan 24, 2022

View reviewed changes

src/plugins/vis_types/timeseries/server/ui_settings.ts Outdated Show resolved Hide resolved

src/plugins/vis_types/timeseries/common/types/vis_data.ts Show resolved Hide resolved

kibanamachine and others added 3 commits January 25, 2022 03:39

Merge branch 'main' into shards

9183b31

fix PR comments

806237a

Merge remote-tracking branch 'upstream/main' into shards

5a16f95

flash1293 approved these changes Jan 25, 2022

View reviewed changes

src/plugins/vis_types/timeseries/server/ui_settings.ts Outdated Show resolved Hide resolved

src/plugins/vis_types/timeseries/server/ui_settings.ts Outdated Show resolved Hide resolved

alexwizp added 2 commits January 26, 2022 10:48

Update ui_settings.ts

5ac0811

Merge remote-tracking branch 'upstream/main' into shards

2963e6a

# Conflicts: # src/plugins/vis_types/timeseries/public/metrics_type.ts

alexwizp force-pushed the shards branch from f461364 to 2963e6a Compare January 26, 2022 10:19

flash1293 added release_note:fix and removed release_note:feature Makes this part of the condensed release notes labels Jan 27, 2022

Kerry350 approved these changes Jan 27, 2022

View reviewed changes

Dosant approved these changes Jan 27, 2022

View reviewed changes

alexwizp merged commit 7e1b780 into elastic:main Jan 27, 2022

kibanamachine added the backport:skip This PR does not require backporting label Jan 27, 2022

This was referenced Feb 1, 2022

[7.17] [TSVB] Fix shard failures are not reported (#123474) #124207

Merged

[7.14] [TSVB] Fix shard failures are not reported #124228

Closed

flash1293 added v7.17.1 v8.0.1 labels Feb 4, 2022

flash1293 mentioned this pull request Feb 9, 2022

[8.0] [TSVB] Fix shard failures are not reported (#123474) #125046

Merged

alexwizp mentioned this pull request Apr 13, 2022

[Step 3] Cleanup charts plugin #130132

Merged

Conversation

alexwizp commented Jan 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🏁 Shard failures are not reported #122944

🏁 Allow Elasticsearch request bodies to be displayed #15424

Uh oh!

elasticmachine commented Jan 21, 2022

Uh oh!

alexwizp commented Jan 24, 2022

Uh oh!

flash1293 commented Jan 24, 2022

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!

Uh oh!

Uh oh!

alexwizp commented Jan 25, 2022

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kibana-ci commented Jan 26, 2022

💚 Build Succeeded

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

History

Uh oh!

alexwizp commented Jan 26, 2022

Uh oh!

Dosant left a comment

Choose a reason for hiding this comment

Uh oh!

Dosant Jan 27, 2022

Choose a reason for hiding this comment

Uh oh!

Dosant Jan 27, 2022

Choose a reason for hiding this comment

Uh oh!

flash1293 commented Jan 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

alexwizp commented Jan 20, 2022 •

edited

Loading

flash1293 left a comment •

edited

Loading