-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ES|QL] - only suggest active integration data streams #187247
Comments
Pinging @elastic/kibana-esql (Team:ESQL) |
@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?) |
@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? |
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 |
@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. |
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) |
Right now it just shows all the datasets, even if they don't have backing indices. So it has the same issue.
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. |
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 (?) |
Happy to, any concerns @ninoslavmiskovic ? |
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
In summary, I think the approach to this would be to extend the 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. |
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: #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. |
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 |
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
@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. |
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. |
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. 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) |
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!" It's telling me, it's unexpected, when I click on View details, I get some information that it happened at 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.
I also think we should have more data / feedback for this. |
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. |
I agree that the UI should have the information which integrations have data and which are merely "installed".
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.
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.
I agree with that, there should be a central API that provides this information. |
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 |
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) |
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? |
We discussed this in the @elastic/kibana-data-discovery sync and concluded that we currently lack the resources to create and maintain a dedicated API to filter out indices provided by integrations, and related to that, filter out integrations that have no indices containing data. Generally, it makes sense to filter out conditions that lead to errors in a UI editor. One measure to mitigate the error is to show a friendlier message in Discover, but this solution only applies to Discover. Since ES|QL usage is extending, an application-specific solution may not be sufficient. The ES|QL editor is currently calling the /api/fleet/epm/packages/installed endpoint. Ideally, for this use case, an additional parameter like From the ES|QL Elasticsearch side, it's worth considering an option to not treat the "No data state" as an error, similar to DSL. When integrations providin dashboards containing ES|QL visualizations waiting for data to ingest, we'll likely encounter similar discussions. There are cases where the a no data error is useful and cases where it's not, so why not support both needs? For now, we should wait for more input and feedback to determine the extent of the problem. The issue is not new and is also present in the Logs explorer. We need to monitor user usage since the feature was added in 8.15. |
We synced and we decided that this is something that could make sense in the fleet API. Issue to track: #191990 |
Nice @drewdaemon - thanks! |
## Summary Close #187247 https://github.com/user-attachments/assets/9f56d941-016a-442a-a2cb-b2240b280b54 Co-authored-by: Stratoula Kalafateli <[email protected]>
…#192953) ## Summary Close elastic#187247 https://github.com/user-attachments/assets/9f56d941-016a-442a-a2cb-b2240b280b54 Co-authored-by: Stratoula Kalafateli <[email protected]> (cherry picked from commit 462e437)
…192953) (#192970) # Backport This will backport the following commits from `main` to `8.x`: - [[ES|QL] exclude inactive integration data stream suggestions (#192953)](#192953) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Drew Tate","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-16T08:24:55Z","message":"[ES|QL] exclude inactive integration data stream suggestions (#192953)\n\n## Summary\r\n\r\nClose https://github.com/elastic/kibana/issues/187247\r\n\r\n\r\nhttps://github.com/user-attachments/assets/9f56d941-016a-442a-a2cb-b2240b280b54\r\n\r\nCo-authored-by: Stratoula Kalafateli <[email protected]>","sha":"462e4378100e8d5233e7379f9244da5609b7dd2c","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","backport:prev-minor","Feature:ES|QL","Team:ESQL"],"title":"[ES|QL] exclude inactive integration data stream suggestions","number":192953,"url":"https://github.com/elastic/kibana/pull/192953","mergeCommit":{"message":"[ES|QL] exclude inactive integration data stream suggestions (#192953)\n\n## Summary\r\n\r\nClose https://github.com/elastic/kibana/issues/187247\r\n\r\n\r\nhttps://github.com/user-attachments/assets/9f56d941-016a-442a-a2cb-b2240b280b54\r\n\r\nCo-authored-by: Stratoula Kalafateli <[email protected]>","sha":"462e4378100e8d5233e7379f9244da5609b7dd2c"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192953","number":192953,"mergeCommit":{"message":"[ES|QL] exclude inactive integration data stream suggestions (#192953)\n\n## Summary\r\n\r\nClose https://github.com/elastic/kibana/issues/187247\r\n\r\n\r\nhttps://github.com/user-attachments/assets/9f56d941-016a-442a-a2cb-b2240b280b54\r\n\r\nCo-authored-by: Stratoula Kalafateli <[email protected]>","sha":"462e4378100e8d5233e7379f9244da5609b7dd2c"}}]}] BACKPORT--> Co-authored-by: Drew Tate <[email protected]>
…#192953) ## Summary Close elastic#187247 https://github.com/user-attachments/assets/9f56d941-016a-442a-a2cb-b2240b280b54 Co-authored-by: Stratoula Kalafateli <[email protected]>
Describe the feature:
Right now, we suggest integration data streams which don't exist. We should only suggest active data streams.
Screen.Recording.2024-09-03.at.8.52.10.AM.mov
User Problems:
The text was updated successfully, but these errors were encountered: