elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.48k stars 8.04k forks source link

[ES|QL] - Improve Integration API to Return Only Existing Indices, when using the ES|QL Editor #187247

Open ninoslavmiskovic opened 4 days ago

ninoslavmiskovic commented 4 days ago

Describe the feature:

I discovered an issue where the integration API returns indices that do not exist, leading to a degraded user experience. Users expect to see only indices with actual data when using the es|ql editor.

User Problems:

Suggested Improvement:

Discussion:

Next Steps:

Video showing the challenge:

https://github.com/elastic/kibana/assets/108192783/8898420d-91b3-40dc-a546-0d4898394ed8

Please review and provide any additional input or adjustments if necessary. Thanks!

elasticmachine commented 4 days ago

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

stratoula commented 4 days ago

@flash1293 can you explain to me why this is an ES|QL concern? We are just using the api that you suggested. Is it expected to return non installed indices? (If I am not mistaken is the same that you are using in the logx explorer no?)

flash1293 commented 4 days ago

@stratoula I'm not sure where it belongs, definitely up for discussion.

The API works as intended - Logs Explorer also shows the data streams without any documents. You need to differentiate between

The difference between installed and data is shipped isn't something the fleet API checks today and I'm not sure it's the right place to do so - it feels like a different concern to me, but OTOH we don't need to overcomplicate things. If there is already (Kibana) server side logic the ESQL editor relies on it feels like the best place to me.

@kpollich how do you feel about some query param on the fleet API for packages to only return the datasets that actually have backing indices?

stratoula commented 4 days ago

If there is already (Kibana) server side logic the ESQL editor relies on it feels like the best place to me.

There is not, we are doing a client side api call to the fleet api.

So the packages are installed but the data is not shipped. I get it better now. I think it is ok if we leave it as it is and dont overcomplicate things, no?

There is no ES|QL server (at least now) and I would like to avoid it if possible

flash1293 commented 4 days ago

@elastic/obs-ux-logs-team any thoughts on the general issue of showing datasets that don't have backing indices (since we do this in the Logs Explorer already)?

I think there were plans to show at least an indicator for that.

flash1293 commented 4 days ago

There is no ES|QL server (at least now) and I would like to avoid it if possible

That makes sense, you could probably also fetch information about backing indices on the client and merge it with the fleet API info, but it feels a bit weird to do it that way.

stratoula commented 4 days ago

There is no ES|QL server (at least now) and I would like to avoid it if possible

That makes sense, you could probably also fetch information about backing indices on the client and merge it with the fleet API info, but it feels a bit weird to do it that way.

This is what you are doing in the Logs explorer?

How often is this to happen btw? ( the packages are installed but the data is not shipped)

flash1293 commented 4 days ago

@stratoula

This is what you are doing in the Logs explorer?

Right now it just shows all the datasets, even if they don't have backing indices. So it has the same issue.

How often is this to happen btw? ( the packages are installed but the data is not shipped)

Not sure how often it happens, but the user can also uninstall an integration if they don't need anymore. I'm not sure how big the impact of this issue is - the question is whether it's worth doing another request which has complexity and performance implications.

stratoula commented 4 days ago

the question is whether it's worth doing another request which has complexity and performance implications.

This is why I am asking. Has any customer complained to you about it? If not, let's leave it as it is and if we have complains we can re-discuss (?)

flash1293 commented 4 days ago

Happy to, any concerns @ninoslavmiskovic ?

ninoslavmiskovic commented 4 days ago

I believe it’s important for us to be proactive rather than reactive in addressing this issue. Waiting for customers to encounter this challenge could lead to unnecessary frustration.

Even on our edge-lite environment, which should be representative, we have several integrations with empty data sets. Integrations like Elastic Agent, which have multiple datasets, often end up being empty. Since we plan to show all integrations, the likelihood of some being empty is higher than we might expect. Displaying these empty integrations to users adds little to no value.

Please see the video from Edge-lite for a demonstration:

https://github.com/elastic/kibana/assets/108192783/094ae3c9-38d6-473a-941e-fc9b56e59400

Additionally, here is another example from our overview cluster, which is representative of a large-scale customer:

https://github.com/elastic/kibana/assets/108192783/c960bd77-8675-4bbb-8167-3cb93c67486f

The list can also be unnecessarily long for the user when including all empty ones.

Also a bit confusing that we are doing the check on indices and not when going throw the integrations.

E.g. the index pattern: logs-elastic_agent.osquerybeat-* does not get suggested when not going the integration route.

Skærmbillede 2024-07-01 kl  15 56 17

https://github.com/elastic/kibana/assets/108192783/5554eaae-ec39-423f-9ea0-4ee7c73389a6

flash1293 commented 4 days ago

Thanks @ninoslavmiskovic , I think your assessment of the issue makes sense.

From the technical side, I don't like putting this into the fleet code, as it's not the purpose of this API. The setup with a new, dedicated API for this purpose is the proper way forward, as it will also allow us to add more specific functionality later on to make the experience richer over time - something we probably want to do. I also don't like putting this kind of logic into the client, as this would mean we need to transfer additional information to the browser that's not required. The Kibana server is the right component for handling this kind of task.

While Logs Explorer has the same issue, we want to move towards a world where all data selection is happening through ESQL. Also, it's not an observability issue, but a general one, so I'm not sure about where this API would fit in terms of our current plugin structure. In terms of team ownership, as this is about generic data access, IMHO the data discovery would be the best long term owner of this kind of functionality - even the team name says it's about discovering your data ;) . It also fits pretty well the description of the data plugin which houses related functionality like autocomplete already:

The data plugin provides common data access services, such as search and query, for solutions and application developers

In summary, I think the approach to this would be to extend the data plugin (or introduce a new targeted plugin, no strong opinion on this) that offers an API for fetching integrations and their datasets, along with information whether there are any backing data streams, with the purpose to let the user select from them in the UI, using the ES API and the method exposed from the fleet plugin behind the scenes to gather this information.

Once that is in place, the ESQL editor can consume this API, ideally through a method exposed via the contract of the client side plugin.

I don't expect this to be a huge task - it's not trivial to do, but relatively straightforward.

flash1293 commented 4 days ago

Some additional thoughts that came up around this in Logs Explorer:

Not having any matching indices is the easiest case, but it's also possible there is data, just not in the current time range. In this case not showing the integration is probably the wrong call (we also don't do this for indices). However, just showing it could lead to the same experience Nino reported above.

There was work planned for the Logs Explorer to show a "last activity" indicator: https://github.com/elastic/kibana/issues/177713 Not sure how that would look like in the ESQL case, this might be difficult for the current Monaco-based rendering.

IMHO starting with excluding datasets without any data at all is a good start and we can make the behavior smarter over time - I still think a dedicated API would be the best approach for this here.

ninoslavmiskovic commented 4 days ago

Totally agree on that this is a generic challenge. How we solve it technically it could make sense what you are proposing. I'm interested in solving the problem and if it is the data plugin then we can get @kertal to charm in an put the right label and amend the issue once there is an alignment.

++ on preparing the data where users have a time window where the data is not yet flowing in.

I think the user challenge is clear , now let's find the right solution ☺️

elasticmachine commented 4 days ago

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

stratoula commented 4 days ago

Not having any matching indices is the easiest case, but it's also possible there is data, just not in the current time range

@flash1293 we are getting the list of indices regardless of the time span. So this is not a concern.

But I agree with you, this is not something I want to do in the ES|QL client side implementation. Is not performant and feels very wrong the ES|QL plugin to be responsible for this.

Possibly the data plugin is a good candidate but I want to see what the Discovery team thinks about it.

It still feels like this should be something the api should do though and we should not be responsible for it. And with api I mean the api we are using to get the integrations and with we I mean the consumers. I am not sure this should be the responsibility of the Discovery team 🤷‍♀️ But I dont want to talk for the team, it just feels weird to me. If we wanted to fix it in the Logs explorer would we again add this in the data plugin? The problem we are trying to solve should not drive the solution imho. But anyway, this is just my opinion, it def is wrong to be part of the ES|QL logic, for everything else I am open for discussion.

flash1293 commented 4 days ago

If we wanted to fix it in the Logs explorer would we again add this in the data plugin

If it would be exclusively for the logs explorer we would probably put it into a logs-explorer-specific server side plugin. IMHO it was never a clean solution to directly call the integrations API from the UI like that, I think now is a good time to do it right instead of buying deeper into something that will make things harder to maintain over time. Logs explorer is still beta and not intended to be kept it its current state forever, but we intend to keep the esql-focused discover for a long time.

stratoula commented 3 days ago

The problem that Nino is highlighting here is that we are suggesting indices that dont exist which means that the client side validation will fail and also ES if the users are submitting the query. This happens at the moment only with integrations.

meow

I just want to be sure that we are at the same page for this problem (and this is why I think is an integrations problem / limitation)

kertal commented 1 day ago

We can discuss if a new server side detection to sort out indices that contain no data server side would be a fit for the data plugin for sure, however, I don't see this as a quick task, and I would evaluate alternatives, that might be more beneficial near term.

What I would suggest is, to consider improving the error message when there's no index. I get it that from ES side it's an error, but from UI and UX side I wouldn't say so. Currently the error message is screaming at me, YOU DID SOMETHING WRONG, HERE'S YOUR RED CARD!"

Bildschirmfoto 2024-07-04 um 18 32 55

It's telling me, it's unexpected, when I click on View details, I get some information that it happened at discover.chunk.0.js:1:15362. if we keep it as an error state, we could improve this, be more helpful, offer some explanation, your data might not have been arrived, a reload button for this case. Or even, Sci-Fi, if I made a typo, ignoring the editors error indication, we could suggest similar index names.

What I want to say, I'm more certain that improving the error message would help, than removing indices/index pattern without data. What if no index contains data? is the integration then displayed? How does the user know it works and isn't an error state, when the integration is missing? what if no user played my new rockclicker instance and as the chief rockclicker who installed the rockclicker integration I wanna see the fist player score incoming?

Saying this, because that used to be a pattern of mine in my "youth" before joining Elastic, going to Discover, waiting that my new, freshly provisioned application sends data.

This is why I am asking. Has any customer complained to you about it? If not, let's leave it as it is and if we have complains we can re-discuss (?)

I also think we should have more data / feedback for this.

stratoula commented 1 day ago

I don't think that this is going to solve the problem Mattthias. The client side validation is in sync with the server side validation. When _query api fails we want to show an error. ES|QL works with indices, there is no index, it fails as it should be. What is the point to hide it? I can't do anything with this. I cant even author a query. The user needs to know when they are using an index that doesn't exist. The ES|QL editor mirrors the _query behavior. I see 3 options:

The latter seems as a hack to me, from the conversation we had yesterday I saw that there are other places which also want the same behavior. So it is not an ES|QL editor problem but a kibana problem.

flash1293 commented 1 day ago

I agree that the UI should have the information which integrations have data and which are merely "installed".

The obs team to check why there are integrations installed without installing the indices in many cases

Indices can't be installed really, they will created once data arrives - it's a pretty fundamental way of how Elasticsearch manages its data in the data stream naming scheme. Not installing these integrations on the first Kibana startup might be a possibility, we would need to check with the infra services team who own synthetics and APM, but I don't think this is a great approach, as the problem can still happen with other integrations where the user installs them from the integrations page, but then never collects any data.

Check with the fleet team if they could extend their api to return integrations that have indices created Create an api which will check the index existence and return only the useful integrations

It seems like we agree on the fact that we need some server side code to provide this information, the question is just where to put it and who is going to own this logic and will build and maintain it.

I chatted about extending the fleet plugin with @kpollich , but we agreed that it's an awkward fit for the API we are using today, so I think we need to introduce a new API anyway to not affect fleet in a negative way - the question is just where to put it. As the current fleet API is user facing, I think we should have an internal API for this use case we can evolve without worrying about backwards compatibility.

So it is not an ES|QL editor problem but a kibana problem

I agree with that, there should be a central API that provides this information.

kertal commented 1 day ago

Added it to the agenda of our next @elastic/kibana-data-discovery sync, then there are more people of the team around to discuss it

stratoula commented 1 day ago

No data collection means no index? If yes I agree then, my suggestion won't make the problem less important. I would still check though why this is happening with synthetics and apm though.

Thanx for discussing this with the fleet team Joe, my plan to move it to another team just vanished 😄

@kertal thanx, I had it on the ES|QL sync but I agree is better to be discussed in the discovery team level. 👍 (I will remove it from our agenda)

mattkime commented 16 hours ago

I'm just getting up to speed here but a couple of thoughts -

The Data plugin shouldn't have additional dependencies. Its unclear to me why this is a poor fit for the Fleet plugin but I'd be tempted to create a new plugin over using the data plugin.

I think the failure to match an index is a common enough error case that it deserves a custom UI. The generic error messages are a bit confusing compared to a simple statement of 'no index found'


How many integrations are there? How many might we expect to be used on a deployment?