-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Metricbeat][Azure]Check for metrics dimensions before matching two metrics together #42591
[Metricbeat][Azure]Check for metrics dimensions before matching two metrics together #42591
Conversation
Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services) |
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
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.
Comparing metric definitions without taking dimensions into account seems incorrect.
Here's an example where, running the code on main
, the matchMetrics()
function returns true
even when the two definitions have different dimensions:
While testing, I noticed that the docs with <empty>
dimension sometimes contain the sum of the non-<empty>
dimension values.
However, I also found documents where the value for <empty>
dimensions seemed not to have any relation with others. Possibly a random assignment due to the incomplete comparison.
I would only suggest adding a test case similar to https://github.com/lastic/beats/blob/71900c4d89fa3b29f8709bb46a61a9ad78a1e9c2/x-pack/metricbeat/module/azure/client_utils_test.go#L61-L85
So we can prove and document the behavior using different dimensions.
LGTM, thank you for taking the time to dive deeper in this issue!
/test |
…lKatsoulis/beats into bugfix/matchMetrics-azure-monitor
…etrics together (#42591) (#42620) * Check for metrics dimensions before matching two metrics together * Add test case for metrics with different dimensions (cherry picked from commit d9f5498) Co-authored-by: Michalis Katsoulis <[email protected]>
…etrics together (#42591) (#42621) * Check for metrics dimensions before matching two metrics together * Add test case for metrics with different dimensions (cherry picked from commit d9f5498) Co-authored-by: Michalis Katsoulis <[email protected]>
…etrics together (#42591) (#42622) * Check for metrics dimensions before matching two metrics together * Add test case for metrics with different dimensions (cherry picked from commit d9f5498) Co-authored-by: Michalis Katsoulis <[email protected]>
Proposed commit message
<empty>
value in ESChecklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Disruptive User Impact
There will no longer be documents with
empty
value in its dimensions in ES. But this was a bug.How to test this PR locally
Deploy metricbeat with azure module before and after the PR and set the following configuration for
azure.yaml
.Set the right client_id, client_secret, tenant_id and subscription_id.
Related issues
Use cases
Screenshots
Before the PR. Version 8.17.0. Storage accounts.
After the PR:
Before the PR. Values of
azure.dimensions.authentication
After the PR. Values of
azure.dimensions.authentication
Before the PR. Values of
azure.dimensions.tier
After the PR. Values of
azure.dimensions.tier