[Discover][Meta] Issues with Discover <> UnifiedHistogram sync affecting the chart, total hits and extra fetches

[jughosta]

Previously addressed:

• [Discover] Drop the separate total hits request in ES|QL mode. Use the number of fetched records instead. #178711
• [Discover] Show the fetched Discover results even when histogram request fails on some shards #198553
• [Security Solution] Misalignment on number of hits in Timeline's ES|QL tab #169354
• [Discover] Refactor Unified Histogram refetching logic for text based languages #159438

Caused challenges when adding more features:

• [Discover] Allow to store the configured ES|QL visualization v3 #175227
• [Discover] Move total hits counter from histogram to grid area. New controls in histogram. #171638
• [Lens] Embeddable react refactor #186642

Upcoming work:

• [Discover][UnifiedHistogram][ES|QL] Time range used for histogram request is different from the one in table request #201128
• [Discover] Move total count logic from UnifiedHistogram to Discover main #177156
• [Unified histogram][ES|QL] Extra fetches in specific scenarios #165192
• [ES|QL] Should calculate true value for total number of docs for given date range #195787
• [Lens] Tech preview of Lens powered by ES|QL #197028 (Lens is switching to ES|QL and we won't be able to get total hits from it)

[elasticmachine]

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

[davismcphee]

Just leaving a couple of notes with some thoughts:

• As far as I can tell, there weren't any challenges in [Lens] Embeddable react refactor #186642 (comment) caused by Discover <> Unified Histogram syncing (unless I just can't find the comment thread since there's many). If the challenge being referred to is [Lens] Embeddable react refactor #186642 (comment), then this issue was unrelated to Discover or Unified Histogram, and was a bug in the Lens embeddable causing multiple fetches as my comment here demonstrates: [Lens] Embeddable react refactor #186642 (comment).
• Several of the other linked issues seem unrelated to Discover <> Unified Histogram syncing. For example, [Discover] Show the fetched Discover results even when histogram request fails on some shards #198553 was a lack of error handling, [ES|QL] Should calculate true value for total number of docs for given date range #195787 is related to ES|QL limitations (default limit of 1000, max limit of 10000, no track_total_hits support), and I believe some of the issues in [Unified histogram][ES|QL] Extra fetches in specific scenarios #165192 are more likely related to Lens auto fetching behaviour than anything.
• I think the Lens auto fetching behaviour contributes largely to the challenges we face here. There's no way to switch to manual fetching, which is necessary in Discover, so it forces us to do prop memoization trickery to implement it. I think (hope) the embeddable refactor will make this easier to implement for the Lens embeddable and would greatly simplify our usage.

That said, in general Unified Histogram can and should be simplified since it's become hard to manage with all the feature additions (plus we don't have a plan to make it a shared component anymore, so we don't really need to structure it as such).

[kertal]

As far as I can tell, there weren't any challenges in #186642 (comment) caused by Discover <> Unified Histogram syncing (unless I just can't find the comment thread since there's many). If the challenge being referred to is #186642 (comment), then this issue was unrelated to Discover or Unified Histogram, and was a bug in the Lens embeddable causing multiple fetches as my comment here demonstrates: #186642 (comment).

During implementation also this bug was surfacing: #184600
And the cause of it was, that when there was a change of data view id, the props given to the lens embeddable were containing old and new data view id. Then there was another similar issue, where I now cannot find the comment in the PR 🦕 , where the interaction between props that were passed in and the Lens embeddable caused another failure. So I think there were related challenges to the syncing, but it's in this case more a issue

I think we should generalize this issue, which for now is a collection of work around the Histogram which should lead to refactoring, iterate in a doc to flesh out what we gonna do.

[jughosta]

If we have to spend hours debugging Discover <> UnifiedHistogram <> Lens communication, then to me it looks like a sign that there is a problem in the current approach. It increases the risk of bugs and slows feature development in that area.

• [Lens] Embeddable react refactor #186642 (comment) days spend on debugging no matter where the actual problem turned out to be
• Failing test: Chrome UI Functional Tests.test/functional/apps/discover/group3/_lens_vis·ts - discover/group3 discover lens vis should show no histogram for non-time-based data views and recover for time-based data views #184600 days spend on debugging
• [Discover] Show the fetched Discover results even when histogram request fails on some shards #198553 a bug in UnifiedHistogram which affected the whole Discover page
• [Unified histogram][ES|QL] Extra fetches in specific scenarios #165192 again, it requires hours/days of debugging which nobody has time to do
• [ES|QL] Should calculate true value for total number of docs for given date range #195787 this has not been decided yet but I see it as an upcoming work which would depend on what approach we take regarding requests management from now on.

[davismcphee]

@kertal Thanks for confirming, that one definitely does sound like a syncing issue between Discover <> Unified Histogram then if the wrong props were being passed to Lens. I also agree with generalizing the issue and thinking broadly about what we can do to make it easier to use Unified Histogram and the Lens embeddable in Discover.

If we have to spend hours debugging Discover <> UnifiedHistogram <> Lens communication, then to me it looks like a sign that there is a problem in the current approach. It increases the risk of bugs and slows feature development in that area.

100% agree with this and your bullet points, so I think we're on the same page about needing to refactor. Mainly I'm just trying to highlight that I think the issues run deeper than Discover <> Unified Histogram syncing, and the Lens embeddable is a key component here that should be looked at too. If we can try to address the issues at each layer, I feel we'll have the best chance of simplifying all the interactions here.