KQL expression string parsing is slow. We've seen a complex KQL string take up to 250ms to parse. https://github.com/elastic/kibana/issues/76811 exists to discuss the KQL expression string parsing being slow and determine whether there are any performance improvements we can make. Assuming we're not able to get KQL expression string parsing performance where we need it to be, we'll instead need to minimize where it's performed.
The SavedObjectsClient#find method currently allow a filter to be specified that is either a KQL expression string or a KueryNode: https://github.com/elastic/kibana/blob/d666038c8f6767f6790cb78f29e0027ff73d83ee/src/core/server/saved_objects/types.ts#L90
I think that we should change this to only accept a KueryNode. Specifying a KQL expression string is easier for the developer, so they will be prone to defaulting to using a string. If we only allow them to specify a KueryNode, it will hopefully prompt them to constructing the KueryNode manually, which is much more performant.
However, There are some situations where we need to use a KQL expression string:
1) When the end-user specifies the KQL expression string
2) In the SavedObjects find object API
When the end-user is specifying the KQL expression string, developers can use esKuery.fromQueryExpression directly to get the KueryNode which can then be passed to SavedObjectsClient#find.
The find route-handler will need to be changed to use esKuery.fromQueryExpression, before calling SavedObjectsClient#find.
Pinging @elastic/kibana-platform (Team:Platform)
I think that we should change this to only accept a KueryNode
However, There are some situations where we need to use a KQL expression string:
Any idea on how we should change the public API if there some use cases where it should be able to call it with a string filter?
Maybe we could split filter into two mutually exclusive fields, filter?: KueryNode; rawFilter?:string, while documenting that using rawFilter is discouraged, but I don't see how we can do better than that?
Any idea on how we should change the public API if there some use cases where it should be able to call it with a string filter?
I think @kobelb suggested removing string completely from the SavedObjectsClient API, if there is an API which needs to accept strings (like the HTTP saved objects find API) then creating the KeuryNode from a string needs to happen inside the route handler (or any level above the saved objects client).
Would it be worth spending the time to address https://github.com/elastic/kibana/issues/55485 first, and then make these changes to require a KueryNode from the data plugin instead?
Otherwise y'all will be implementing the changes described here, only to have them removed again when we address that other issue.
To address https://github.com/elastic/kibana/issues/55485, I thought we were just going to pull the "es_query" code out into a place where both core and the data plugin can import it. Wouldn't this just change the location where we're importing these types and classes? Is there some intricacy when I'm overlooking?
Ah, sorry @kobelb, my comment above was assuming the original plan we had outlined in that issue, however I had missed your subsequent comment describing the challenges of that approach.
The only issue with re-creating the es_query package is that we will run into the same issue that drove us to merge it into the data plugin in the first place: It depends on type interfaces from data like IIndexPattern, IFieldType, Query, plus also now some from the kibana_utils plugin.
I don't want to distract from the main topic of this issue, so I'll go ahead and comment on #55485
Most helpful comment
I think @kobelb suggested removing string completely from the SavedObjectsClient API, if there is an API which needs to accept strings (like the HTTP saved objects find API) then creating the KeuryNode from a string needs to happen inside the route handler (or any level above the saved objects client).