[Regression] Histogram aggregation always shows an error message

[sulemanof]

Kibana version: latest

Elasticsearch version: latest

Describe the bug:

The Histogram agg always shows an error message:

Steps to reproduce:

1. Open an area chart
2. Select a histogram bucket, fill the fields, press apply
3. An error with text "Unable to retrieve max and min values ..." is shown (file location : src/plugins/data/public/search/aggs/buckets/histogram.ts)

Any additional context:

The problem is related to getter/setter functionality. The actual error is :

Error: UiSettings was not set. at get (http://localhost:5620/bundles/plugin/visualizations/visualizations.plugin.js:106047:23) at SearchSource

which comes from : https://github.com/elastic/kibana/blob/master/src/plugins/data/public/search/search_source/search_source.ts#L374

[elasticmachine]

Pinging @elastic/kibana-app-arch (Team:AppArch)

[timroes]

@alexwizp Could we first verify this is not happening in 7.7, otherwise we'll need to mark this as a blocker for 7.7.

[alexwizp]

@timroes not a blocker for 7.7. Cannot reproduce it on that branch

[lukeelmers]

Just caught up with @alexwizp -- He has identified the root cause of this bug, which is that the SearchSource class is exported statically, however it still relies on runtime services. So in some cases when calling fetch there are failures because the service getters it depends on have not been set in the downstream plugin's bundle. (This is yet another reason why moving away from the createGetterSetter pattern is important, because it can create the illusion of code being static when in fact it is not).

There are three possible ways to resolve this:

1. Move SearchSource into the data.search runtime contract, perhaps under the __LEGACY namespace.
   • Could perhaps use a pattern similar to what we did with aggs where we have a createSearchSource wrapper function which returns an instantiated SearchSource class, with the runtime dependencies already injected.
   • Pro: contract of SearchSource doesn't change for downstream apps
   • Con: apps need to have an explicit dependency on data in their kibana.json and pass SearchSource down to where it is used in their app.
2. Pass the stateful dependencies to the SearchSource constructor & still export it statically
   • Pro: apps can still have the simplicity of importing SearchSource statically
   • Con: apps will still need to pass in the runtime dependencies for the services SearchSource relies on (uiSettings, injectedMetadata, search service). In the end this doesn't make Option 2 any different from Option 1.
3. Combination of 1 and 2. SearchSource is exported statically with the stateful services provided via configuration, or you can use the runtime createSearchSource as a convenience.

cc @lukasolson @lizozom in case you have any opinions on this either way.

[lukeelmers]

Thinking more about this, I think we should go with Option 1 in my list above (expose SearchSource on runtime contract). Mostly because I don't know why somebody would be needing to use SearchSource as a static import without also needing the underlying search service, so we might as well expose it on the runtime contract.

We could optionally do the refactoring in Option 2, which would improve the readability of the code, but I don't really see a use case for making SearchSource a static export (unless someone else can think of one).

[alexwizp]

@lukeelmers thank you, I also think that it should be in our runtime contract