-
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
[Monitoring] Fixed parsing error in shard size query #99159
Conversation
Pinging @elastic/stack-monitoring (Team:Monitoring) |
💚 Build SucceededMetrics [docs]
To update your PR or re-run it, just comment with: cc @igoristic |
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! I tested this with an extremely low value and it successfully fires as expected.
FYI... I originally tested this with Elastic Agent and it wouldn't fire because the backing indices for Agent are prefixed with .ds-*
. I think this will become a problem in the future so I filled an issue: #99396
# Conflicts: # x-pack/plugins/monitoring/server/lib/alerts/fetch_index_shard_size.ts
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
1 similar comment
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
1 similar comment
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
This PR addresses a regression caused by: #96177, since top level aggregations can't be defined without a filter (or a field):