-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Security Solution] Data Quality Dashboard new historical data retrieval API routes #181945
Comments
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
Pinging @elastic/security-threat-hunting-explore (Team:Threat Hunting:Explore) |
Looking good! Thanks for writing these down. Some questions from my side:
|
|
@semd can you clarify
Current |
@semd can you clarify are |
No, the current To clarify, this route returns a list of results, one (latest) result per index that matches the pattern, it is used by the existing page of the DQD, to display the latest check on each index, the other 2 routes will be needed for the new Historical results page. The
About the nested list, I don't think the search API should make any decision on the field in which the search results need to be grouped, that decision is on the UI, we may need to show results grouped by a field other than the index name, so I think it's better to return all the results in a flat list and delegate the grouping to the UI. Hope it makes sense. |
@semd thanks for your thorough response. Sorry for confusing you with my initial question, after rereading it I noticed it didn't express what I had in mind. My question was actually not about Got 2 questions and 1 suggestion:
But of course this might not be applicable and I might miss a giant piece of picture, so take this with a jar of salt :D |
We still don't know what we want to be able to filter. I don't think right now we should assume we will have a multi-valuated index pattern input in the UI, and that we'll need to send those patterns to be filtered on the API side, that's a lot of assumptions. But, if we need to do it, I think the index pattern can be treated as any other field in the mapping, right?
Well, I don't discard paginating on the API as a possibility, but I won't do it from the start. Simply because we are querying DQD checks, they are done manually, and even if we implement the automatic check executions someday, customers probably won't have a significant number of executions/documents. It's much easier and faster to implement pagination on the UI for now, and if someday we see the need to improve performance we can consider API-side pagination.
There are interesting ideas, but I don't think this issue is the correct place to discuss each one of them, let's discuss them synchronously with the team and come back here with the conclusions. |
Close in favor of #182868 |
Description
Create new API routes for historical results so they can be retrieved in the UI:
We need 2 new routes:
createdAt
field (confirm if other filters likepattern
,index
,createdBy
are also needed). To render the results table.id
. To render the "check result" detail page.Current state
The
GET /results
route already exists, right now it returns the latest result of each index in a givenpattern
received in the request query parameters:GET /internal/ecs_data_quality_dashboard/results?pattern=logs-*
This is needed to render the last "check result" on the main Data Quality Dashboard page. So, we'll need to cover the 3 types of historical "check result" retrieval.
Proposal
Add:
GET /results/id/:id
-> Retrieves only one result by idGET /results/search?startDate=[date]&endDate=[date]
-> Retrieves multiple results. The time range parameters are required. If we have extra filters or the KQL bar on the historical result search screen, then we'll need to add optional parameters to this route.Keep:
GET /results?pattern=[string]
-> Retrieves the latest result of each index in the "pattern" parameter. Since this is an internal route, we can update the path to/results/latest?pattern=[string]
to make it more consistent with the other two new routes.The text was updated successfully, but these errors were encountered: