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

[Stack Monitoring] improve unit tests in fetch functions in alerts #124033

Merged
merged 6 commits into from
Feb 1, 2022

Conversation

neptunian
Copy link
Contributor

@neptunian neptunian commented Jan 28, 2022

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.

  • Adds 2 assertions for each rule. That the query looks a certain way and that when CCS is disabled in config, the index pattern is updated to not perform cross cluster search.
  • Updates the Missing Monitoring Data rule to have another term in the bool query filter of node_stats (see comment in code review)
  • Update fetchClusters to use dataset: 'cluster_stats' that was missing even though we were including it in the index pattern (metrics-elasticsearch.cluster_stats-*)

@neptunian neptunian added the Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services label Jan 28, 2022
@neptunian neptunian requested a review from a team as a code owner January 28, 2022 14:32
@neptunian neptunian self-assigned this Jan 28, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@neptunian neptunian added v8.1.0 release_note:skip Skip the PR/issue when compiling release notes labels Jan 28, 2022
@@ -73,6 +74,7 @@ export async function fetchMissingMonitoringData(
cluster_uuid: clusters.map((cluster) => cluster.clusterUuid),
},
},
createDatasetFilter('node_stats', 'elasticsearch.node_stats'),
Copy link
Contributor Author

@neptunian neptunian Jan 28, 2022

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.

@neptunian
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
monitoring 35 39 +4

Total ESLint disabled count

id before after diff
monitoring 39 43 +4

History

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

cc @neptunian

@neptunian neptunian merged commit 6e7fcfe into elastic:main Feb 1, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Feb 1, 2022
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 release_note:skip Skip the PR/issue when compiling release notes Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants