Skip to content
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

Merged

Conversation

yngrdyn
Copy link
Contributor

@yngrdyn yngrdyn commented May 10, 2024

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

@apmmachine
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@yngrdyn yngrdyn self-assigned this May 10, 2024
@yngrdyn yngrdyn marked this pull request as ready for review May 14, 2024 12:51
@yngrdyn yngrdyn requested a review from a team as a code owner May 14, 2024 12:51
@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label May 14, 2024
@yngrdyn yngrdyn added release_note:skip Skip the PR/issue when compiling release notes and removed ci:project-deploy-observability Create an Observability project labels May 14, 2024
@botelastic botelastic bot added the ci:project-deploy-observability Create an Observability project label May 14, 2024
Copy link
Member

@ruflin ruflin left a 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"
Copy link
Member

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.

Copy link
Contributor Author

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

@mohamedhamed-ahmed mohamedhamed-ahmed left a 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

Copy link
Contributor

@mdbirnstiehl mdbirnstiehl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kibana-ci
Copy link
Collaborator

kibana-ci commented May 14, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
datasetQuality 191 193 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
datasetQuality 166.1KB 180.9KB +14.8KB

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id before after diff
module count - 5407 +5407
total size - 8.8MB +8.8MB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
datasetQuality 36.3KB 36.8KB +545.0B
Unknown metric groups

async chunk count

id before after diff
datasetQuality 8 9 +1

ESLint disabled line counts

id before after diff
datasetQuality 10 11 +1

Total ESLint disabled count

id before after diff
datasetQuality 12 13 +1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @yngrdyn

@yngrdyn yngrdyn merged commit 3caf266 into elastic:main May 14, 2024
19 checks passed
@kibanamachine kibanamachine added v8.15.0 backport:skip This commit does not require backporting labels May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:project-deploy-observability Create an Observability project release_note:skip Skip the PR/issue when compiling release notes v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dataset quality] Adjust degraded docs query
7 participants