[Lens] Expose Elasticsearch accuracy warnings to the user #116632

alexwizp · 2021-10-28T13:45:14Z

Summary

Lens should show warnings when sort by ascending results from elasticsearch include the unbounded error warning: "doc_count_error_upper_bound" : -1 or greater than 0

Screens

Testing notes:

Go to DevTools and execute the following 2 commands

PUT test
{
  "settings": {
    "number_of_shards": 4
  },
  "mappings": {
    "properties": {
      "name":{
        "type":"keyword"
      }
    }
  }
}
POST _bulk
{ "index" : { "_index" : "test"} }
{ "name" : "jim" }
{ "index" : { "_index" : "test"} }
{ "name" : "bob" }
{ "index" : { "_index" : "test"} }
{ "name" : "sue" }
{ "index" : { "_index" : "test"} }
{ "name" : "jane" }
{ "index" : { "_index" : "test", "routing": "earlier"} }
{ "name" : "tanya roberts" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "tanya roberts" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "tanya roberts" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "tanya roberts" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "tanya roberts" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "tanya roberts" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "tanya roberts" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "tanya roberts" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "tanya roberts" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z1" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z2" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z3" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z4" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z5" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z6" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z7" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z8" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z9" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z10" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z11" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z12" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z13" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z14" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z15" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z16" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z17" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z18" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z19" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z20" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z21" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z22" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z23" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z24" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z25" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z26" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z27" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z28" }
{ "index" : { "_index" : "test", "routing": "today"} }
{ "name" : "z29" }

Create new Data View based on created test index
Go to Lens and create new Hit Map visualization based on test Data View

Expected result: precision error warning message should be shown

Closes: elastic#94918

elasticmachine · 2021-11-01T13:29:50Z

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

flash1293

some smaller nits.

Could we add a Unit test for this to the terms aggconfig?

@ghudgins about the wording of the error message - not sure how we should do this. Does talking about aggregations here makes sense (as it’s not a Lens concept)? Should we make it more general? Also, Maybe we should mention a possible workaround with increasing the size. Maybe linking to a paragraph in the doc to be able to better explain what’s going on?

x-pack/plugins/lens/public/indexpattern_datasource/time_shift_utils.tsx

alexwizp · 2021-11-02T13:18:15Z

@elasticmachine merge upstream

alexwizp · 2021-11-03T08:20:42Z

@elasticmachine merge upstream

x-pack/plugins/lens/public/indexpattern_datasource/utils.tsx

ghudgins · 2021-11-08T14:21:45Z

Sorry for the delay. I think we should focus on what the user can do and not necessarily an administrator. Maybe something like

Top values for this visualization may be approximate due to how the data is indexed. Try increasing the number of Top values or use Filters instead of Top values for precise results. To learn more about this limit, visit the documentation.

MichaelMarcialis · 2021-11-08T14:38:17Z

+1 to what @ghudgins says about focusing the message on what the user can do.

Top values for this visualization may be approximate due to how the data is indexed. Try increasing the number of Top values or use Filters instead of Top values for precise results. To learn more about this limit, visit the documentation.

Regarding this messaging, would it be possible and/or desirable to also have reference to top values and filters function as links that automatically open the appropriate configuration flyout for the user (as a sort of shortcut)? For example, clicking a top values link could open that configuration and auto-focus the top value number field. Thoughts?

ghudgins · 2021-11-08T15:24:23Z

would it be possible and/or desirable to also have reference to top values and filters function as links that automatically open the appropriate configuration flyout for the user (as a sort of shortcut)

@MichaelMarcialis nice idea, I don't think that can hurt (as long as it's simple to do technically). I love "errors" that have "actions" on them

flash1293 · 2021-11-09T10:19:38Z

Let's go with just the warning for the first version - this kind of action is pretty complex to implement into our current structure. It makes a lot of sense to provide it though, I will open a separate issue for it.

# Conflicts: # test/interpreter_functional/snapshots/baseline/combined_test3.json # test/interpreter_functional/snapshots/baseline/final_output_test.json # test/interpreter_functional/snapshots/baseline/metric_all_data.json # test/interpreter_functional/snapshots/baseline/metric_empty_data.json # test/interpreter_functional/snapshots/baseline/metric_multi_metric_data.json # test/interpreter_functional/snapshots/baseline/metric_percentage_mode.json # test/interpreter_functional/snapshots/baseline/metric_single_metric_data.json # test/interpreter_functional/snapshots/baseline/partial_test_2.json # test/interpreter_functional/snapshots/baseline/step_output_test3.json # test/interpreter_functional/snapshots/session/combined_test3.json # test/interpreter_functional/snapshots/session/final_output_test.json # test/interpreter_functional/snapshots/session/metric_all_data.json # test/interpreter_functional/snapshots/session/metric_empty_data.json # test/interpreter_functional/snapshots/session/metric_multi_metric_data.json # test/interpreter_functional/snapshots/session/metric_percentage_mode.json # test/interpreter_functional/snapshots/session/metric_single_metric_data.json # test/interpreter_functional/snapshots/session/partial_test_2.json # test/interpreter_functional/snapshots/session/step_output_test3.json

alexwizp · 2021-11-09T11:46:21Z

message was updated

flash1293 · 2021-11-09T11:55:34Z

That's great @alexwizp - one small nit about the message: Could the leading "Top values" be replaced with the label of the column? Just to make it easier for the user to find it in case there is more than one top values in the current chart.

alexwizp · 2021-11-09T12:18:33Z

alexwizp · 2021-11-09T14:19:26Z

@elastic/kibana-app-services please 👀

alexwizp · 2021-11-10T10:49:43Z

@elastic/kibana-app-services please review

ppisljar · 2021-11-10T14:09:26Z

x-pack/plugins/lens/public/indexpattern_datasource/utils.tsx

+ warningMessages.push(
+ <FormattedMessage
+ id="xpack.lens.indexPattern.precisionErrorWarning"
+ defaultMessage="{docCount} values for a terms aggregation may be approximate. As a result, any sub-aggregations on the terms aggregation may also be approximate."


nit: how do we know here that it was a terms aggregation that may be approximate ? it seems that hasPrecissionError could theoretically exist on any aggregation ?

@ppisljar no, hasPrecissionError is calculated for terms only. For other kinds of aggregations, hasPrecisionError method is undefined.

ppisljar

code LGTM

dej611

Tested locally with FF and Safari 👍

alexwizp · 2021-11-11T10:13:12Z

@elasticmachine merge upstream

kibanamachine · 2021-11-11T11:30:37Z

💛 Build succeeded, but was flaky

Test Failures

[job] [logs] Default CI Group #18 / apis uptime uptime REST endpoints with generated data certs api when data is present retrieves expected cert data

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`data`	500	501	+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
`data`	2848	2855	+7

Async chunks

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

id	before	after	diff
`lens`	954.5KB	955.7KB	+1.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`	268.1KB	268.2KB	+111.0B
`data`	430.9KB	431.4KB	+599.0B
total			+710.0B

Unknown metric groups

API count

id	before	after	diff
`data`	3238	3245	+7

History

💔 Build #5822 failed d46e67f
💔 Build #5812 failed 9828967
💚 Build #5338 succeeded 210380f
💛 Build #4571 was flaky 30938d9
💚 Build #4315 succeeded d2a0c0b

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

cc @alexwizp

…6632) * [Lens] Expose Elasticsearch accuracy warnings to the user Closes: elastic#94918 * fix comments * update text Co-authored-by: Kibana Machine <[email protected]>

* [Lens] Expose Elasticsearch accuracy warnings to the user Closes: #94918 * fix comments * update text Co-authored-by: Kibana Machine <[email protected]>

…6632) * [Lens] Expose Elasticsearch accuracy warnings to the user Closes: elastic#94918 * fix comments * update text Co-authored-by: Kibana Machine <[email protected]>

alexwizp force-pushed the 94918 branch 6 times, most recently from 08a6354 to 377ccc4 Compare November 1, 2021 09:55

elastic deleted a comment from kibanamachine Nov 1, 2021

[Lens] Expose Elasticsearch accuracy warnings to the user

9ba5e7b

Closes: elastic#94918

alexwizp force-pushed the 94918 branch from 377ccc4 to 9ba5e7b Compare November 1, 2021 11:17

alexwizp changed the title ~~WIP [Lens] Expose Elasticsearch accuracy warnings to the user~~ [Lens] Expose Elasticsearch accuracy warnings to the user Nov 1, 2021

alexwizp self-assigned this Nov 1, 2021

alexwizp added Feature:Lens v8.1.0 Team:Visualizations Visualization editors, elastic-charts and infrastructure release_note:enhancement labels Nov 1, 2021

alexwizp marked this pull request as ready for review November 1, 2021 13:29

alexwizp requested review from a team as code owners November 1, 2021 13:29

flash1293 requested changes Nov 1, 2021

View reviewed changes

x-pack/plugins/lens/public/indexpattern_datasource/time_shift_utils.tsx Outdated Show resolved Hide resolved

x-pack/plugins/lens/public/indexpattern_datasource/time_shift_utils.tsx Outdated Show resolved Hide resolved

botelastic bot added the Feature:ExpressionLanguage Interpreter expression language (aka canvas pipeline) label Nov 1, 2021

Merge branch 'main' into 94918

1539110

Merge branch 'main' into 94918

49aac84

alexwizp force-pushed the 94918 branch 2 times, most recently from da7ccb5 to 1b0b96f Compare November 3, 2021 13:10

dej611 reviewed Nov 3, 2021

View reviewed changes

x-pack/plugins/lens/public/indexpattern_datasource/utils.tsx Outdated Show resolved Hide resolved

fix comments

d8ac6e0

alexwizp requested a review from dej611 November 9, 2021 12:43

ppisljar reviewed Nov 10, 2021

View reviewed changes

ppisljar approved these changes Nov 10, 2021

View reviewed changes

alexwizp requested a review from a team as a code owner November 10, 2021 14:54

alexwizp force-pushed the 94918 branch from 9828967 to d46e67f Compare November 10, 2021 15:24

mshustov approved these changes Nov 10, 2021

View reviewed changes

dej611 approved these changes Nov 10, 2021

View reviewed changes

update text

4048fd1

alexwizp force-pushed the 94918 branch from d46e67f to 4048fd1 Compare November 11, 2021 10:12

Merge branch 'main' into 94918

ef481cb

alexwizp added the backport:skip This commit does not require backporting label Nov 11, 2021

alexwizp enabled auto-merge (squash) November 11, 2021 10:13

alexwizp merged commit 4c1e629 into elastic:main Nov 11, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Expose Elasticsearch accuracy warnings to the user #116632

[Lens] Expose Elasticsearch accuracy warnings to the user #116632

alexwizp commented Oct 28, 2021 •

edited

Loading

elasticmachine commented Nov 1, 2021

flash1293 left a comment •

edited

Loading

alexwizp commented Nov 2, 2021

alexwizp commented Nov 3, 2021

ghudgins commented Nov 8, 2021 •

edited

Loading

MichaelMarcialis commented Nov 8, 2021

ghudgins commented Nov 8, 2021

flash1293 commented Nov 9, 2021

alexwizp commented Nov 9, 2021

flash1293 commented Nov 9, 2021

alexwizp commented Nov 9, 2021

alexwizp commented Nov 9, 2021

alexwizp commented Nov 10, 2021

ppisljar Nov 10, 2021

alexwizp Nov 10, 2021

ppisljar left a comment

dej611 left a comment

alexwizp commented Nov 11, 2021

kibanamachine commented Nov 11, 2021

API count

[Lens] Expose Elasticsearch accuracy warnings to the user #116632

[Lens] Expose Elasticsearch accuracy warnings to the user #116632

Conversation

alexwizp commented Oct 28, 2021 • edited Loading

Summary

Screens

Testing notes:

elasticmachine commented Nov 1, 2021

flash1293 left a comment • edited Loading

Choose a reason for hiding this comment

alexwizp commented Nov 2, 2021

alexwizp commented Nov 3, 2021

ghudgins commented Nov 8, 2021 • edited Loading

MichaelMarcialis commented Nov 8, 2021

ghudgins commented Nov 8, 2021

flash1293 commented Nov 9, 2021

alexwizp commented Nov 9, 2021

flash1293 commented Nov 9, 2021

alexwizp commented Nov 9, 2021

alexwizp commented Nov 9, 2021

alexwizp commented Nov 10, 2021

ppisljar Nov 10, 2021

Choose a reason for hiding this comment

alexwizp Nov 10, 2021

Choose a reason for hiding this comment

ppisljar left a comment

Choose a reason for hiding this comment

dej611 left a comment

Choose a reason for hiding this comment

alexwizp commented Nov 11, 2021

kibanamachine commented Nov 11, 2021

💛 Build succeeded, but was flaky

Test Failures

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Page load bundle

API count

History

alexwizp commented Oct 28, 2021 •

edited

Loading

flash1293 left a comment •

edited

Loading

ghudgins commented Nov 8, 2021 •

edited

Loading