-
Notifications
You must be signed in to change notification settings - Fork 8k
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] Warning for datasets not supporting _ignored aggregation #183183
[Dataset quality] Warning for datasets not supporting _ignored aggregation #183183
Conversation
…y-adjust-degraded-docs-query
…nored aggregation
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
…y-adjust-degraded-docs-query
x-pack/plugins/observability_solution/dataset_quality/server/routes/data_streams/routes.ts
Outdated
Show resolved
Hide resolved
...ity_solution/dataset_quality/server/routes/data_streams/get_non_aggregatable_data_streams.ts
Outdated
Show resolved
Hide resolved
...ity_solution/dataset_quality/server/routes/data_streams/get_non_aggregatable_data_streams.ts
Outdated
Show resolved
Hide resolved
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.
Looks great.
external | ||
target="_blank" | ||
data-test-subj="datasetQualityNonAggregatableHowToFixItLink" | ||
href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-rollover-index.html" |
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.
I was initially thinking ideally we could link to a UI page that can trigger a rollover with a button but seems this doesn't exist? As it is temporary for most users, this should be good enough.
Lets also have some docs around this issue explaining why users see _ignored, that is was not index on previous elasticsearch versions and a rollover makes sure it is index. Then we link to the docs and from there to rollover (or both)? Something we can still iterate on.
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.
@mdbirnstiehl can help us in that front. Should we create something like a troubleshooting section? or where should we put that type of information?
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.
Then we link to the docs and from there to rollover (or both)? Something we can still iterate on.
I'd prefer to have the How to fix it?
visible in the warning, and then some docs if I want to get to the details of why that happened, rather than having to go to the docs to see how to solve it. However don't have a strong opinion in this matter.
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.
Having the solution in the warning is good to help users that are aware of rollovers and how they work. Then we can link out to the docs for users that aren't familiar with using the rollover and explain more about why they need to do it. We can probably add a section to the docs that are in progress currently. This way we are covering all users and not making advanced users go to the docs necessarily.
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.
LGTM! Thanks for the work
… for the aggregation
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.
LGTM!
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Canvas Sharable Runtime
Page load bundle
Unknown metric groupsasync chunk count
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @yngrdyn |
Closes #179227.
📝 Summary
This PR adds a warning to main page and flyout whenever a dataStream has indices that doesn't support
_ignored
aggregation🎥 Demo
Screen.Recording.2024-05-10.at.19.15.27.mov
Flyout
Screen.Recording.2024-05-14.at.14.53.33.mov