-
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
[Dataset quality] Support failure store #199806
base: main
Are you sure you want to change the base?
[Dataset quality] Support failure store #199806
Conversation
🤖 Jobs for this PR can be triggered through checkboxes. 🚧
ℹ️ To trigger the CI, please tick the checkbox below 👇
|
/ci |
@@ -26,7 +26,7 @@ export const NONE = 'none'; | |||
export const DEFAULT_TIME_RANGE = { from: 'now-24h', to: 'now' }; | |||
export const DEFAULT_DATEPICKER_REFRESH = { value: 60000, pause: false }; | |||
|
|||
export const DEFAULT_DEGRADED_DOCS = { | |||
export const DEFAULT_QUALITY_DOC_STATS = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note
This is now reused by degradedDocs
and failedDocs
.
}; | ||
|
||
return new DataStreamStat(dataStreamStatProps); | ||
} | ||
|
||
public static fromDegradedDocStat({ | ||
public static fromQualityStats({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note
We can extend this method with more qualityStats and construct a Dataset from them
@@ -40,7 +40,7 @@ export const indexNameToDataStreamParts = (dataStreamName: string) => { | |||
}; | |||
|
|||
export const extractIndexNameFromBackingIndex = (indexString: string): string => { | |||
const pattern = /.ds-(.*?)-[0-9]{4}\.[0-9]{2}\.[0-9]{2}-[0-9]{6}/; | |||
const pattern = /.(?:ds|fs)-(.*?)-[0-9]{4}\.[0-9]{2}\.[0-9]{2}-[0-9]{6}/; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Failure store is at the moment a backing index that starts with .fs
x-pack/plugins/observability_solution/dataset_quality/common/utils/quality_helpers.ts
Show resolved
Hide resolved
@mdbirnstiehl I need some help from your side
|
const datasetsQuality = { | ||
percentages: filteredItems.map((item) => item.degradedDocs.percentage), | ||
}; | ||
const datasetsQuality = countBy(filteredItems.map((item) => item.quality)) as Record< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quality is now part of the dataset, so we don't need this calculation anymore. Instead I just created a map that will hold how many datasets fall into a quality status. e.g.
{
'poor': 1,
'degraded': 2,
'good': 1,
}
/ci |
This one looks good.
I'm not sure if this is exactly how it works, but is there some way we could be more specific here? Like, "Percentage of docs sent to failure store due to an issue during ingestion." I'm not sure "failure store" itself will be meaningful for users.
This tool tip can probably match the tool tip for the data set quality column in the table. A side note, maybe this would need to be a different issue, but this heading: Should probably be Data Set Quality, not sets. It also looks like the table column headers are in title case, and should be in sentence case to be consistent with the other pages in Stack Management. For example, "Failed docs" instead of "Failed Docs" |
That's exactly how it works 👍🏼
I'll address those in this PR as well. This is how everything looks now Screen.Recording.2024-11-12.at.16.42.32.mov |
/ci |
/ci |
I started testing quickly and got 2 issues:
Screen.Recording.2024-11-14.at.12.38.24.mov
Screen.Recording.2024-11-14.at.12.39.03.mov
This works fine you can ignore it, tried it and seems good. Only the other problem is existing |
41e4f61
to
41d249a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did first round of code review and got some small comments
x-pack/plugins/observability_solution/dataset_quality/common/utils/quality_helpers.ts
Show resolved
Hide resolved
...s/observability_solution/dataset_quality/public/components/dataset_quality/table/columns.tsx
Outdated
Show resolved
Hide resolved
...tion/dataset_quality/public/components/dataset_quality/table/failed_docs_percentage_link.tsx
Show resolved
Hide resolved
...plugins/observability_solution/dataset_quality/public/hooks/use_dataset_details_telemetry.ts
Outdated
Show resolved
Hide resolved
x-pack/plugins/observability_solution/dataset_quality/common/translations.ts
Show resolved
Hide resolved
export async function getFailedDocsPaginated(options: { | ||
esClient: ElasticsearchClient; | ||
types: DataStreamType[]; | ||
datasetQuery?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe renam datasetQuery to just datasetNames as its a bit confusing since its not really a query
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think about renaming it to indexPattern
? or maybe just datasetPattern
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me 👍
I doubt it since we have a selection component to choose how many rows per page we can view. |
That's right! I totally forgot, thank you 🤣 |
/ci |
/ci |
80f5c53
to
dd9816e
Compare
/ci |
1 similar comment
/ci |
7b32bf0
to
42db254
Compare
/ci |
42db254
to
0bb1ac6
Compare
/ci |
0bb1ac6
to
076cb6c
Compare
/ci |
/ci |
1 similar comment
/ci |
a33ca8f
to
f10abac
Compare
/ci |
45e0213
to
895110f
Compare
/ci |
💔 Build Failed
Failed CI StepsHistory
cc @yngrdyn |
Closes https://github.com/elastic/logs-dev/issues/183 and https://github.com/elastic/logs-dev/issues/184
Summary
This PR aims to show to the user failed docs % in dataset quality table. The following acceptance criteria items were resolved
Dataset quality page
Dataset details page
The percentage of docs sent to failure store due to an issue during ingestion.
🎥 Demo
Screen.Recording.2024-11-12.at.11.53.02.mov
Specific cases
0.001%
degraded docs
andfailed docs
is loaded (Quality is now calculated using both values)Screen.Recording.2024-11-18.at.15.55.05.mov
Screen.Recording.2024-11-18.at.16.01.03.mov
Quality
even when they don't have access to stats or something went wrong with that endpointScreen.Recording.2024-11-18.at.16.02.47.mov
Screen.Recording.2024-11-19.at.16.57.54.mov
Missing
The following acceptance criteria are missing
because es changes are not finalised