Kibana: New "search alert" type based on ES dsl query and hit count

[mikecote]

[elasticmachine]

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

[pmuellr]

If this alert is going to have a complex input element - eg, query DSL, or perhaps even KQL - I wonder if we'd want to "hide" it in the places we allow customers to create connectors. Only allow it to be created in an app like discover, where we will won't have to deal with validating a complex input object.

[dsztykman]

I think we should only allow saved search for this

[gmmorris]

I think we should only allow saved search for this

@dsztykman Am I right in thinking that by _saved search_ you actually mean _saved query_?
Unless I'm misunderstanding something I can't see how a saved search would be useful for an alert.
I was misunderstanding, as I thought saved searches save the results as well, which isn't the case - it only saves the underlying Discover configurations.

As for limiting this to Saved Queries - what do you see as the value in limiting alerts to a saved query? 🤔
Supporting _saved queries_ makes sense to me, as that could provide a much simpler alert creation/editing UI, but limiting to saved queries them has some down sides.

For example, it would force the users to create noise in their saved queries list if they need to define a bunch of alerts. It also means we would create a coupling between a saved query and its underlying alert, which means a user might change the behaviour of an alert when they change a query without even knowing they're having that effect (unless we can somehow make this link explicit in Discover, but not sure how that would be done).

Anyway, would love to hear your thoughts. :)

[gmmorris]

Looking at how Discover supports both Lucene and KQL I don't see an obvious reason not to support both in the Alert Type.

I'm even considering how we can best have the Alert Type mirror Discover (perhaps by building off of the types exposed by Discover so that they are truly _linked_), though that creates a coupling I'm not yet 100% sure about... bottom line, I think the Alert could, and probably should, support both Lucene and KQL. :)

[dsztykman]

KQL is a bit limited in certain usage, and saved queries only apply for KQL, that's why I talked about saved search.

[gmmorris]

Had a chat with @dsztykman and here are some notes from his feedback that are definitely worth keeping in mind:

David's main concern is that if you don't use a saved search/query as the basis users can't verify that their search actually returns what they want. I explained the plan is to expose this as an action in Discover where you create an Alert based off of the current search, which addresses this. This does bring to light the need to have a chart (like in Index Threshold) that gives the user an idea of how many results their search _currently_ returns.

This raises a question worth asking: Do we want to offer the option in the flyout of creating an alert based on a saved search / query?
If so, what do we do if the user wants to create a new search?
We could take some _free text_ search and rely on the chart for validating the criteria, but perhaps instead, we could redirect the user to Discover where they have a much better experience in doing so?

[AlonaNadler]

I think we should only allow saved search for this

Having alerts only on saved searches will be very limited, it is a struggle we have today with export to CSV. This will hide the alerting feature and will create many saved search objects that are not needed.

Regarding saved queries, this is rather a new feature that has low usage.

Ideally, users have an explicitly create an alert in Discover which is not tied to saved search or saved query. If it helps in the implementation you can maybe create a saved search or query in the background in user transparent way, for the sake of the alert. If saved query/search will be a mandatory step to create an alert I'm concerned we are going through the challenge we have with CSV

If I had to choose I would focus on KQL first, since this is the default and close to 90% of the clusters are using KQL

[pmuellr]

If I had to choose I would focus on KQL first, since this is the default and close to 90% of the clusters are using KQL

+1

Seems like this would be much easier for customers to deal with than query DSL anyway. I have an ESSQL based alert I was playing with a while back also - https://github.com/pmuellr/kbn-sample-plugins/blob/master/plugins/alert_type_examples/server/alert_types/essql.ts

[gmmorris]

I'm looking into how we can use the underlying data plugin that Discover uses.

The advantage to this approach is that we would get a 1-to-1 match with how Discover runs it's queries, which means that whatever the user sees in the Discover UI, should match what the query in the Alert sees. This means that when the user hits this theoretical "Create Alert from Query" button, they should get a starting point which matches what Discover displays and can then do things like change the time range etc.

It also means that changes that are made in Discover in the future, such as changes to the default params, would apply to the underlying Alerts as well and won't go out of sync.

In terms of work needed this would mean:

1. We would have to expose an api on the data plugin that would allow the Alerting plugin to create a search API like we have on _context_. You can see an example of this in the PR below.
2. We would need to figure out which fields populated by Discover should be modified at Alert creation (such as stripping out the date range and replacing with a date range relative to when the Alert runs) and what triggering options we need to expose (do we trigger an instance _per document_? Do we do so from the moment a document appears or does it require a threshold? etc.)

Would love to hear thoughts from @elastic/kibana-alerting-services and @elastic/kibana-app-arch (as I think you own the data plugin?)

Example PR of the changes we'd need to make in data and what the Alert Type would then have to do: https://github.com/elastic/kibana/pull/62932

[pmuellr]

Exposing a scoped data plugin accessor like we do callCluster and savedObjectClient for alertTypes seems like it makes a lot of sense - though I'm not familiar with the data plugin.

[gmmorris]

Exposing a scoped data plugin accessor like we do callCluster and savedObjectClient for alertTypes seems like it makes a lot of sense - though I'm not familiar with the data plugin.

It's the plugin used by Discover to query.

[lukeelmers]

@gmmorris Looking at your PR, what you are describing sounds like what we are already doing here, where we register route handler context to provide scoped search: https://github.com/elastic/kibana/blob/a3ae1042deb3fc222596a88a42a7d404f133ce1d/src/plugins/data/server/search/search_service.ts#L60-L65

However, it looks like you need the adminClient instead of dataClient, correct?

I wonder if it would be worth us providing an admin-scoped API as well via the router handler context? Or alternatively if we exported createApi statically and made the search strategies available on the runtime contract, you could probably construct the scoped api directly in your plugin.

Anyway, @lukasolson knows more on our long-term plans for the data search service on the server so he might have a better answer to this question.

[gmmorris]

@gmmorris Looking at your PR, what you are describing sounds like what we are already doing here, where we register route handler context to provide scoped search:

https://github.com/elastic/kibana/blob/a3ae1042deb3fc222596a88a42a7d404f133ce1d/src/plugins/data/server/search/search_service.ts#L60-L65

However, it looks like you need the adminClient instead of dataClient, correct?

Thanks @lukeelmers, I might not have explained this well enough, let me expand a bit:

It's not that we're using the adminClient, rather what we're doing is creating a scoped dataClient by using the adminClient such that the scope is of a specific user - specifically the user that created the Alert.
The reason we can't use the context created in the snippet you reference, is that we do not actually come through a router, but rather this is a scheduled task executed by Task Manager. RouteHandlerContext simply isn't available to us when we're running a scheduled task.

I wonder if it would be worth us providing an admin-scoped API as well via the router handler context?

As we're not using the admin scoped client, but rather a user scoped client that wasn't created via a route handler, this wouldn't help in this case. 😬

Or alternatively if we exported createApi statically and made the search strategies available on the runtime contract, you could probably construct the scoped api directly in your plugin.

The thinking behind exposing the API that I added was that it provides the ability to create a scoped search using precisely the same code that creates it for the context. Using the same code means that even though these are two different entry points, they will always provide the same API and we shouldn't have any divergence in the future.

While we could expose createApi and searchStrategies, it felt cleaner to me to keep the domain of "creating a search api" scoped to the Search plugin, rather than leak that into other plugins. 🤔
Part of the goal here is to have parity (and alignment) with the API that Discover uses, so this change made the most sense to me.

Would you still prefer to expose createApi and searchStrategies over exposing just the handler that takes a dataClient same way the contextHandler does? It seems like a bigger (and less maintainable) change to me... Happy to chat over zoom, as I might just be missing some context.

I hope that all makes sense. 😄

Anyway, @lukasolson knows more on our long-term plans for the data search service on the server so he might have a better answer to this question.

I spoke to @lukasolson last week and he generally seemed to support the direction I took, but there's always a chance I missed something.

[lukeelmers]

Thanks for clarifying @gmmorris! I misunderstood your use case. If you already talked with @lukasolson and he's on board, then no worries on my end 😄

[gmmorris]

I've spent the day trying to figure out how the pieces can fit together and I think I now have a mental model of what might work well.

The direction I've been experimenting with is thus: The ES Query AlertType will essentially be parameterised with the following:

1. A Query which is formatted as a SavedQuery - this would mean that anything we can save as a saved query can also back the ES Query AlertType.
   This would include the query itself (KQL initially), filter, time filter etc., as seen in this type:

https://github.com/elastic/kibana/blob/d0b4ae7e29e4600edb289dd229b8619054ccba02/src/plugins/data/public/query/saved_query/types.ts#L31)

1. The Alert Type can then use the data plugin's search api in the same manner that Discover uses it, which means we can keep this Alert aligned with Discover in terms of behavior. This, though, will entail mimicking the process that happens in discover where the Saved Query is converted into the query that is passed to the search api. This is tricky as much of that implementation is encapsulated in the data plugin. I have managed to expose these required helpers locally on my machine (getTime for timeFilters, buildEsQuery to turn the KQL query into a DSL Query etc.), but it looks like we'll also need access to the IndexPatterns service which, currently, is only exposed on the client side (public plugin, not the server plugin) and so, this will require additional changes in the data plugin. This would need to be done with guidance from the @elastic/kibana-app-arch team.

2. The Alert Type could then get back the results of this search and decide whether to trigger AlertInstances based on whatever parameters are specified by the alert creator - presumably a Minimal Hit Count above which we trigger the instances.

The benefit of this approach is that it should be straight forward to create an Alert directly from within Discover, as the format will match that of Saved Queries, and the matching documents seen in Discover should always match the ones seen in the alert. Imagine going from Discover to Alert and back to Discover, we'd be able to maintain a unified experience there, which I think is how we envisioned the original requirement.
The challenge will be that quite a few of the data plugin's internals (APIs & behaviour) would need to be exposed/mimicked in a maintainable manner, that will allow data to keep its encapsulation, allow alerting to avoid unnecessary coupling with data and will still provide us with the consistent behaviour we expect.

I think making additional work on this alert should wait for us to confirm this is the right direction and align with App-Arch to make sure we don't step on their toes with changes we'd need to make in the data plugin.

[gmmorris]

I'm going to try and get my local prototype PR to a stable enough point that it can express, more or less, how this might work.

[AlonaNadler]

Thanks @gmmorris for sharing your approach here!
@timroes @kertal not sure if you see that. You might have thoughts on Gidi's approach

[timroes]

@gmmorris That sounds all very reasonable. That part with server side index patterns is really important, since I assume that's a bit larger change (one that has been on the list for a long time), and I agree that we can't proceed with this alert, before we don't have that. So I think it's worth talking to App Arch and clarify with them about planning around this.

[AlonaNadler]

cc: @alexh97 @ppisljar it seems that this needs index pattern on server side

[gmmorris]

Had a discussion with @arisonl about how we can move forward with this in terms of priorities.
For now this is marked as blocked, updates to come.

[mikecote]

As discussed with @peterschretlen, @gmmorris, @bmcconaghy and @arisonl, there is value for an alert to be purely working with ES DSL. We'll use this issue to develop an ES DSL alert type that takes in a query and alerts on data returned.

Things to investigate:

• Alert per document, per count, above/below count, etc (most of these can be roadmap items but we can start with one)
• UI experience (none at the beginning, JSON editor or expression style field selector "x is y")
• Using the --examples flag or POC to integrate with console app
• Using the --examples flag or POC to integrate with discover app (see #61314)

[arisonl]

Reiterating here for the record that there is a lot of demand for a term/phrase matching alert type. So, a simple UX implementation, consistent with what we are building so far and very similar to what Logs is making available for alerting, built on top of the more generic and powerful DSL alert that is described here, would likely go a very long way. Since UX is key and raw query building/syntax are close to Watcher territory, I am wondering whether this could be an MVP for the DSL/search alert. cc @alexfrancoeur

[mikecote]

Splitting out the UI portion of this issue into #69829. This would make this issue scoped to server side / HTTP API support for search alerts and later on build a UI with #69829.

[111andre111]

It would be really neat if this one can support painless scripts as well as if it would add a certain possibility to cover certain edge cases as well.

[ppisljar]

short update from our side, IndexPatterns and SearchSource are now available on the server.

[pmuellr]

It would be really neat if this one can support painless scripts as well as if it would add a certain possibility to cover certain edge cases as well.

That seems like that could be nice as a way to boil down the query result into some "simple" variables that could then be used in action parameters. We should probably look at all the scripting options available in Watcher as potential candidates for this kind of capability. Eg, https://www.elastic.co/guide/en/elasticsearch/reference/current/transform.html

[gmmorris]

short update from our side, IndexPatterns and SearchSource are now available on the server.

🎉 🎉 🎉 🎉

That's awesome, thanks @ppisljar
IIRC IndexPatterns was the main blocker on our end. 🤔

[pmuellr]

We get requests from customers asking for more data in the context variables from the existing alerts. In some cases, this isn't really practical because of the way the alert builds it's queries - and is likely going to be difficult to allow the customer a nice interface to specify what other data they want.

The dsl query alert is of course pretty nicely setup for this, since the customer presumably will be able to create open-ended queries. Or at least that's the end goal. The next question is then - how do we make this data available to customers such that it can easily be used as context variables in actions. I'd guess we may need something like issue https://github.com/elastic/kibana/issues/77793 in some use cases, as it's hard to imagine doing much more than providing the raw query response in the context, which I'm guessing won't be easily usable within a mustache template.

[gmmorris]

Please note Infra's needs over here: https://github.com/elastic/infra/issues/19949#issuecomment-735726282
Sounds like they broadly need "the full power of the Elasticsearch Query DSL combined with Painless scripting.", so definitely worth syncing with them when this issue is picked up to ensure we're addressing their needs.