[One Discover] Fix document comparison mode and the field statistics tab when using Smart Fields

[Kerry350]

Summary

This addresses the bugs listed in #181674 (#181621 and #178970).

Smart Fields are compound fields, as such there is no "real" backing field for them. In these scenarios where we can't use them directly we fall back to the fallbackFields configured for them.

@amyjtechwriter please could you check the copy. I think derived from... would have sounded better, but it felt at odds with the scenario where you might specifically pick a field that is also a fallback field for a smart field.

Reviewer notes

• There were quite a lot of cross cutting concerns here between the unified-field-list, unified-data-table, core Discover code, discover-utils, and the data_vizualizer plugin for the field statistics table. Where I'd have liked to potentially put more things in the discover-utils it wasn't possible due to circular project references.

• I've not added any functional tests for now as we'd be adding them to the Observability Logs Explorer which is going to be deprecated in due course. This ties in with part three of [Discover] Smart Fields #181674.

• A renderFieldName prop has been added to the field statistics table, this seemed to be the least intrusive way forward rather than a new / comprehensive cell rendering change (similar to the external cell renderers).

• I've used the original fallbackFields configurations but the virtual column components actually render a bit extra. Am I missing anything with regards to potentially extending the fallbackFields definitions?

Screenshots

[apmmachine]

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

• /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
• run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

[Kerry350]

/ci

[Kerry350]

/ci

[elasticmachine]

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

[elasticmachine]

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

[Kerry350]

Thanks for reviewing @qn895, I think I've actioned all your feedback 🙏

[Kerry350]

@elasticmachine merge upstream

[qn895]

Tested and field stats change LGTM 🎉 I recommend investigating why the async chunks increase so much (+17.1KB overall and +7.2KB for Discover) to verify if that's normal/expected before merging.

[davismcphee]

The changes look good to me overall! I left a couple of minor points of feedback, but nothing major. I agree with @qn895 that we should investigate the bundle size increase a bit since at a glance it doesn't feel like it should have increased by as much as it did 🤔

Also for some reason I'm still getting no results when switching to the field statistics tab in Logs Explorer, but I see that it was working for you. Any idea why?

tabs.mp4

[davismcphee]

I see what you mean about the cross cutting concerns. It would be great it we didn't have to pass info like this around so much, but I unfortunately don't have a better suggestion either atm.

[Kerry350]

Yeah, I couldn't really think of anything without introducing some sort of new package / plugin and that seemed rather heavy. I think by at least having the type source of truth in one place it makes future changes pretty easy.

[qn895]

Also for some reason I'm still getting no results when switching to the field statistics tab in Logs Explorer, but I see that it was working for you. Any idea why?

@davismcphee I have a theory why and the fix for it. It should be fixed with this PR #180849. I tested this PR with the changes there and verified it works correctly 👍

[Kerry350]

@qn895 @davismcphee Thank you both for looking at this 🙏 I've actioned all of your feedback, and this should be good to go.

I'm on PTO next week (and back on the 12th June), I'd love to get this approved and merged on Monday if possible (as I fly on Tuesday). But, if there's any big blockers I can look when I'm back.

The only thing I haven't been able to look at in detail is the async chunk size increase, but I'll add a task to #181674 for when I return.

[qn895]

Field stats change LGTM 🎉

[davismcphee]

Data Discovery changes LGTM 👍 The bundle size increase isn't too significant, so we can definitely follow up on that later to avoid blocking this PR. Thanks for the great work on this, and enjoy your PTO!

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: b2836bc

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
cloudSecurityPosture	450	678	+228
discover	907	911	+4
eventAnnotationListing	606	610	+4
lens	1471	1475	+4
logsExplorer	594	597	+3
securitySolution	5491	5494	+3
slo	843	846	+3
total			+249

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
@kbn/unified-field-list	269	284	+15

Async chunks

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

id	before	after	diff
cloudSecurityPosture	450.5KB	455.5KB	+5.0KB
dataVisualizer	757.6KB	757.7KB	+61.0B
discover	811.0KB	818.2KB	+7.2KB
securitySolution	15.2MB	15.2MB	+2.4KB
slo	866.2KB	868.4KB	+2.2KB
total			+16.8KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
@kbn/unified-field-list	9	8	-1

Unknown metric groups

API count

id	before	after	diff
@kbn/unified-data-table	151	152	+1
@kbn/unified-field-list	293	313	+20
total			+21

async chunk count

id	before	after	diff
cloudSecurityPosture	6	7	+1
discover	23	24	+1
securitySolution	97	98	+1
slo	26	27	+1
total			+4

ESLint disabled line counts

id	before	after	diff
@kbn/unified-field-list	16	17	+1

Total ESLint disabled count

id	before	after	diff
@kbn/unified-field-list	16	17	+1

History

• 💚 Build #213389 succeeded 816dff4
• 💔 Build #213298 failed f9e66a2
• 💔 Build #213236 failed d6c1028
• 💛 Build #212899 was flaky 5acb3cf
• 💔 Build #212837 failed 3399c26
• 💔 Build #212772 failed 9f7ba15

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

cc @Kerry350