[One Discover] Breakdown by log.level for logs sources

[tonyghiani]

📓 Summary

Closes #183498

This work sets the Breakdown selector for the Histogram to the log.level field once a logs data source is resolved.
It also applies to ES|QL queries, the change is applied/removed on each source change.

[tonyghiani]

/ci

[tonyghiani]

/ci

[tonyghiani]

/ci

[tonyghiani]

/ci

[davismcphee]

I opened a PR against this branch with fixes for the Discover issues here: tonyghiani#3.

cc @jughosta since it would be great to get your review on this after the PR merges to evaluate my changes. I also left some comments in the PR for reference.

[elasticmachine]

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

[elasticmachine]

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

[tonyghiani]

@davismcphee @jughosta the changes you applied fixed the initialization of the breakdown field 👌
It now works under a logical perspective, but (I guess) due to the internals of the discover state management, the UI flashes quite heavily when switching data view, probably cause it gets the newly resolved breakdown field and updates the state, which handles the loading for the histogram. I recorded a video, let me know if the issue is evident or we can look at it together.

Screen.Recording.2024-11-22.at.14.27.04.mov

[davismcphee]

@tonyghiani Thanks for raising this, I was surprised to see it since I didn't think there should be an impact from these changes. Turns out this is actually an issue in main related to the Lens embeddable firing an extra pair of load/complete events before fetching now (but only actually fetches once). I haven't seen this bug before though so I wonder if it's recent and just snuck it's way in this PR with a merge from main.

The good news is the Lens embeddable refactor is merging soon, so I rebased these commits on it to test locally, and confirmed it doesn't have the bug:

data_view_2.mp4

Now I just need to figure out when the bug was merged to main and see if we need a fix backported...

[davismcphee]

Ok I looked into it more and it's not an issue with the embeddable itself. It's because the embeddable is being unmounted and remounted here, which triggers an additional fetch, but it gets cancelled quickly so no request is actually sent:

kibana/src/plugins/unified_histogram/public/chart/histogram.tsx

Lines 233 to 235 in 0963369

	if (!checkValidDataViewConfig(dataView, visContext, lensProps.attributes.state.adHocDataViews)) {
	return <></>;
	}

It looks like it was merged a couple of days ago in #200687, so still unrelated to this PR. And that code was removed in the embeddable refactor which explains why I didn't see it there.

cc @kertal, looks like the flaky test fix might have had some unintended effects. This is actually a good example of how manual control over Lens fetches would make our lives easier 😄

[tonyghiani]

@davismcphee I see thanks! I'll wait for the work the lens embeddable to be merged and check for a last round if everything works correctly!

[kertal]

Ok I looked into it more and it's not an issue with the embeddable itself. It's because the embeddable is being unmounted and remounted here, which triggers an additional fetch, but it gets cancelled quickly so no request is actually sent:

kibana/src/plugins/unified_histogram/public/chart/histogram.tsx

Lines 233 to 235 in 0963369

	if (!checkValidDataViewConfig(dataView, visContext, lensProps.attributes.state.adHocDataViews)) {
	return <></>;
	}

It looks like it was merged a couple of days ago in #200687, so still unrelated to this PR. And that code was removed in the embeddable refactor which explains why I didn't see it there.

cc @kertal, looks like the flaky test fix might have had some unintended effects. This is actually a good example of how manual control over Lens fetches would make our lives easier 😄

Yes, I think we should even consider to remove this "fix" of the backport, treat the occacional race condition and the one condition that we can reliably reproduce it, as a known issue. it's resolved in with the Lens embeddable 🥳

the interaction between Discover state <> Unified Histogram <> Lens embeddable produces unwanted side effects like in this case, that are resolved with #186642, and another argument for making our lives easier by gaining manual control over Lens embeddable data fetching 👍

[tonyghiani]

@davismcphee the lens refactor is merged and I can confirm the glitch on the data fetching is gone, it looks good now! There were no additional change on my side, let me know whether we are good to go with this 👌

[jughosta]

LGTM too 👍

[davismcphee]

@tonyghiani Nice! It LGTM too, I just want to clarify a product question in Slack before merging.

[davismcphee]

It looks like we're all in agreement from the product side to move forward, so in that case I'd say let's get this merged 👍 great work @tonyghiani!

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: e73a4ad

Metrics [docs]

Async chunks

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

id	before	after	diff
discover	810.1KB	811.3KB	+1.2KB
unifiedHistogram	71.1KB	71.3KB	+234.0B
total			+1.4KB

Unknown metric groups

API count

id	before	after	diff
unifiedHistogram	70	69	-1

History

• 💚 Build #253513 succeeded 85fd4b8
• 💔 Build #252327 failed 1c8f3d9
• 💔 Build #252306 failed e596248
• 💔 Build #252272 failed d9474a0
• 💔 Build #252236 failed 7072fec
• 💔 Build #252001 failed 4101ad1

cc @tonyghiani

[kibanamachine]

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12064927627

[kibanamachine]

💔 All backports failed

Status	Branch	Result
❌	8.x	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 200584

Questions ?

Please refer to the Backport tool documentation

[tonyghiani]

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation