-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
[Stack Monitoring] improve unit tests in fetch functions in alerts #124033
[Stack Monitoring] improve unit tests in fetch functions in alerts #124033
Conversation
Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI) |
@@ -73,6 +74,7 @@ export async function fetchMissingMonitoringData( | |||
cluster_uuid: clusters.map((cluster) => cluster.clusterUuid), | |||
}, | |||
}, | |||
createDatasetFilter('node_stats', 'elasticsearch.node_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.
Since we are querying for metrics-elasticsearch.node_stats
as part of the index pattern added here https://github.com/elastic/kibana/pull/119112/files#diff-666dd698bc0e64e0a8265f7f08b758b5d73921ea321e0312f32474e3f834ac9bR60, the term filter should have been added. I don't know why it didn't exist previously but probably because it used to look across many products before being changed to only look at ES. I presume adding the node_stats
into the index pattern to begin with is correct and tested that the alert is still working.
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @neptunian |
After modifying all the ES queries in support of agent/integrations, we should have unit tests for how we except the query params to look. This PR adds them to the the alerts' fetch queries.
node_stats
(see comment in code review)