[ES|QL][Discover] Add support for highlighted columns

[sddonne]

• Addresses [Discover][ES|QL] Highlight the results of a search #190293

Summary

This PR adds highlighting support for ES|QL.
Highlighting in ES|QL does not work similarly than classic mode, the user needs to explicitly format the column inside the query.
As today the only way to highlight results is by using the TOP_SNIPPETS function.

FROM index | EVAL col0 = TOP_SNIPPETS("Some text", "text", { "highlight": true }) 

Implementation

The support has been added following a similar approach to how CATEGORIZE or SPARKLINE works, by adding a new DataSource profile in Discover.

When a column is created by using a function that produces highlighting results it's assigned a custom cell renderer with highlighting capabilities.

Both named col0 = TOP_SNIPPETS(..) and unnamed columns are supported EVAL TOP_SNIPPETS(..) .
The highlighted column is still tracked after being renamed.

The highlighting tags are by default <em></em> but they can be changed by using { "highlight": true, "pre_tag": "<mark>", "post_tag": "</mark>" }, this information is also bypassed to the cell rendered for its correct interpretation.

Alternative implementation contemplated

An alternative approach, closest to Classic highlighting implementation has been tried in this PoC #269970, but as ES|QL and Classic mode highlighting workings differs to much it resulted in a not so clean implementation.

[sddonne]

/ci

[infra-vault-gh-plugin-prod]

Pinging @elastic/kibana-esql (Team:ESQL)

[jughosta]

Using the contextual cell rendering in the data grid for highlights is a nice idea! Although, it would not work for UnifiedDocViewer or Summary column.

Have we considered mimicking the classic structure of highlights when constructing each record in

kibana/src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_esql.ts

Lines 110 to 114 in 46927a6

	const record: DataTableRecord = {
	id: row._index && row._id ? getDocId(row) : `${idx + 1}@${responseTime}`,
	raw: row,
	flattened: row,
	};

? That way it would be automatically picked up everywhere in the code.

[sddonne]

Using the contextual cell rendering in the data grid for highlights is a nice idea! Although, it would not work for UnifiedDocViewer or Summary column.

Have we considered mimicking the classic structure of highlights when constructing each record in

kibana/src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_esql.ts

Lines 110 to 114 in 46927a6

	const record: DataTableRecord = {
	id: row._index && row._id ? getDocId(row) : `${idx + 1}@${responseTime}`,
	raw: row,
	flattened: row,
	};

? That way it would be automatically picked up everywhere in the code.

Crossed my mind.. but did not convinced me for two reasons:

• Trying to convert a ES|QL query result into a DSL result seems a dangerous path to take to me, we are trying to tie two things that are different, later on it's difficult to come back. ( Or maybe this a solution we already apply in other cases and we are fine with it, I lack some context tbh.).
  Most important, it will involve to modify the result value to take out the markers, and create a side object with the highlighted results. Modifying the output of the result value in Kibana will probably lead to users getting confused (maybe they want to copy the results with the highlight tags that wont be there anymore!).

• Highlighting do not work the same way on ES|QL and Classic, so I think it could be ok to have different experiences.
  ES|QL do not highlight the terms on every column as Classic does, you need to be specific on which column you want highlight on, so not highlighting Summary will not be weird, and what I think is key is that ES|QL directly modifies the output of the query (Classic preserves the original output and returns a sidecar with highlights). So displaying the actual result in DocViewer might be acceptable (it's also consistent with how CATEGORIZE() works). But this last point is more of a product question, not on me to decide je.

@stratoula do you have thoughts about this 🤔 ?

[davismcphee]

@sddonne I think the main issue with using a data source profile for this is that it will override other data source profiles. So e.g. if you highlight within a query targeting logs-*, the logs profile won't activate as intended. That said, we have a similar issue with the SPARKLINE command that the ML team ran into, and I have it on my todo list to provide them a real solution (not sure the exact shape yet, but it will be some kind of core Discover renderers that always apply and are detached from profiles entirely).

That said, maybe cell renderers aren't the best way to approach this at all. @jughosta what are the chances we'd be able to incorporate this directly into field formatters similar to how DSL highlighting works (but without restructuring the record itself)?

[jughosta]

@davismcphee We could extend the field formatters so they work with a different structure but since it needs access to the query, it would be better to parse the query and prepare a more readable structure somewhere higher and only once (for example around

kibana/src/platform/plugins/shared/discover/public/application/main/data_fetching/fetch_esql.ts

Lines 110 to 114 in 46927a6

	const record: DataTableRecord = {
	id: row._index && row._id ? getDocId(row) : `${idx + 1}@${responseTime}`,
	raw: row,
	flattened: row,
	};

). This way the performance of rendering would not become slower.

[davismcphee]

@jughosta Got it and understand better now, thanks. I agree with what you're saying.

@sddonne I think what Julia suggested makes sense and ideally we should support this in field formatters so it's picked up everywhere by default. I get what you mean about not wanting to force the ES|QL structure to match DSL though.

How about instead we combine ideas and go through field formatters like your original PoC, but without reusing the DSL structure and instead adding a separate esql_highlight prop based on your work in this PR? Maybe a good compromise so we can get the field formatter benefits without needing to hack the structure to match DSL.

I opened a PoC PR of the idea here and it seems to work well: #271162. It's mostly vibe coded and could be done better with more care I'm sure, but should give an idea of how something like this could work. Wdyt?

[stratoula]

Hey! I told Sebastian to check the profile solution for 2 reasons:

• his PoC was adding a boolean at the DatatableColumn which I didnt like tbh.
• We are doing the same for sparkline and categorize so it seemed to me as we are actually trying to solve the same problem (different cell rendering if a column comes from a specific function)

I agree with Sebastian, I dont find it a good idea to copy DSL. Your PR @davismcphee doesnt mess up with the columns meta (esql.ts) so it is ok by me and it is your architectural decision at the end of the day 😄 But I dont understand why we are handling this differently than categorize or sparkline. In my mind all of them are trying to solve the same problem: When a function appears, I want the specific column to be rendered differently. I dont get why we want to follow a different pattern here but maybe I am missing something and ofc you know better. Having a different architecture for the same things seems very confusing to me. Maybe we need to unify them (sparkline, highlight, categorize, whatever the future brings .... using the same architecture? I understand there is a problem in the others too we also want to handle?)

[kibanamachine]

💛 Build succeeded, but was flaky

• Buildkite Build
• Commit: 9c95f83
• Build duration: 65 mins

Failed CI Steps

• FTR Configs #134
• FTR Configs #210
• Scout Lane #9 - stateful-classic / default
• Scout Lane #39 - serverless-observability_complete / default

Test Failures

• [job] [logs] FTR Configs #134 / integrations When in the Fleet application and on the Endpoint Integration details page should display the endpoint custom content
• [job] [logs] Scout Lane #39 - serverless-observability_complete / default / local-serverless-observability_complete - Hosts Page - Empty State - should show onboarding page when no data is present
• [job] [logs] Scout Lane #9 - stateful-classic / default / local-stateful-classic - Hosts Page - Empty State - should show onboarding page when no data is present

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
agentBuilder	1662	1663	+1
agentBuilderDashboards	596	597	+1
alertingVTwo	1140	1141	+1
apm	2245	2246	+1
cases	2050	2051	+1
cloudSecurityPosture	824	825	+1
console	389	390	+1
controls	433	434	+1
dashboard	1094	1095	+1
data	641	642	+1
datasetQuality	1121	1122	+1
dataVisualizer	933	934	+1
discover	2102	2106	+4
esql	1068	1069	+1
esqlDataGrid	493	494	+1
eventAnnotationListing	758	759	+1
fleet	2213	2214	+1
infra	1913	1914	+1
lens	1811	1812	+1
lists	473	474	+1
logsShared	485	486	+1
maps	1382	1383	+1
ml	4348	4349	+1
observability	1817	1818	+1
observabilityAIAssistantApp	853	854	+1
observabilityLogsExplorer	306	307	+1
osquery	818	819	+1
queryActivity	310	311	+1
searchPlayground	635	636	+1
securitySolution	9573	9574	+1
slo	1387	1388	+1
stackAlerts	236	237	+1
streamsApp	1937	1938	+1
transform	951	952	+1
triggersActionsUi	1464	1465	+1
unifiedDocViewer	982	983	+1
visTypeVega	1837	1838	+1
visualizations	777	778	+1
workflowsManagement	1854	1855	+1
total			+42

Async chunks

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

id	before	after	diff
discover	1.9MB	1.9MB	+2.1KB

History

• 💛 Build #446299 was flaky 29361ec
• 💔 Build #446223 failed 6e29988
• 💔 Build #446211 failed 82f30d7

[davismcphee]

@stratoula I agree with not copying DSL, better to adopt our own dedicated structure for ES|QL as my PoC proposes. And while similar, it's not the same as categorize or sparkline. In those cases we specifically want special formatting only within the table cells, but highlighting should work in a way that mirrors all the places where it shows in DSL. A good example is the screenshot from my PoC where we want to support highlighting within the summary cell and log content breakdown, but wouldn't want to render the pattern badges or sparkline chart in those places:

We do have a separate issue with the pattern and sparkline renderers since they use profiles and therefore can't be mix and matched the way we want, which I plan to address soon. But even with that enhancement, those ones will still be cell renderers and I really think field formatters are the better fit for highlighting.

@sddonne My take is unless there's hard pushback, I'd encourage us to to explore an approach similar to my PoC. But I'm also gonna step back code wise and let you take it from here so we don't have too many cooks in the kitchen 🙂

[stratoula]

Thanks for the explanation @davismcphee, highlighting has a broader rendering scope than categorize/sparkline so at the current architecture field formatters seem the right fit for that.

With that being said, what I'm really pointing at is the bigger picture: we're accumulating different extension mechanisms for function-based column transformations (profiles for some, field formatters for others, and the two can't compose), and that's going to get confusing as more functions like this land. The profile override issue you mentioned is exactly the kind of symptom I'd expect from this. I'm glad you're planning to address it — just want to make sure we end up with something unified so future contributors (and us) have a clear pattern to follow rather than case-by-case decisions. 🙏

[davismcphee]

@stratoula I hear that, but I think they serve fundamentally different purposes, and field formatters aren't for column transformations. Field formatters control default field value rendering regardless of context, while cell renderers are specific to the Discover table (and may actually be transformative like inline charts). Cell renderers can also compose field formatters (e.g. summary columns do this), but not the other way around. I think this is a clear distinction and they shouldn't be unified, but the guidance around it could probably be improved.

But tbh I think highlighting is an exception and in most cases cell renderers will be the right answer. The upcoming changes for categorize and sparkline will still use cell renderers, just not connected to profiles, and I think we'll be able to rely on this for other functions in the future.

[stratoula]

Thanks for the clarification Davis — I wasn't suggesting we merge the two mechanisms, just that we need a clear decision rule for when to use which, so the next person implementing a function-based column transformation doesn't have to reverse-engineer it from existing cases.

You said "the guidance around it could probably be improved" — I'd love to see that happen. Even a short note in the profiles README or a doc comment somewhere capturing "use field formatters when the rendering needs to work outside the grid (e.g. highlighting), cell renderers otherwise" would go a long way. Happy to help write it if useful or brainstorm together other ideas.

[sddonne]

Thank you all for getting involved and share your opinions, it shows dedication 🙏 .

Opened a new PR with the proposed idea of adding a new prop to the Discover hit, so I'm going to proceed to close this one.
#271519

[davismcphee]

@stratoula That makes sense, we should be able to add some docs to clarify! I've made a note about it since I need to get back to these sooner than later anyway.