[azure monitor] Address wildcard metrics names timegrain issue

[joecompute]

Proposed commit message

Main Bug Addressed: Wildcard Search Bug

Issue: #43885

Description: In the buggy scenario, a wildcard search for metrics is provided without a timegrain. A timegrain is the Azure terminology for aggregation period. This caused metrics to be pulled with an incompatible timegrain. In the buggy scenario, we incorrectly use the last leveraged timegrain to pull metric data again.

Fix: In this fix, we first grab the smallest available timegrain from the metric availabilities from the Azure API. These timegrains appear to be ordered, ascending, so we use the first one to assign the metric to a group. We then have groups of compatible metrics associated with this timegrain to prepare for the next step. This fix applies to both

• the batch metric fetch and
• the non-batch metric fetch.

Minor Side Bug Addressed: Nil Pointer Dereference

Issue: #43725

Description: In line

beats/x-pack/metricbeat/module/azure/monitor_service.go

Line 322 in 67e847c

interval = *resp.Interval

we are dereferencing the resp.Interval pointer to get the interval from the api response.

Fix: Check if the interval is not nil and not empty before continuing to process this data. If it is nil or empty, reject the data as we do when the API call errors. When this happens, we can assume the data is bad. This is because we have also handled the wildcard issue in this PR, so this API error edge case in code should not be hit unless the API is returning bad data.

Checklist

• My code follows the style guidelines of this project
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have made corresponding change to the default configuration files
• I have added tests that prove my fix is effective or that my feature works
• I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Disruptive User Impact

None - this only fixes bugs

Author's Checklist

How to test this PR locally

Testing wildcard metrics bug

Unit tests have also been added by this PR, which are automatically run by CI. To test beyond this manually, see below:

Set up the scenario/infrastructure as described in the parent issue: #43885

- module: azure  
  metricsets:  
    - monitor  
  enabled: true  
  period: 60s  
  client_id: '${AZURE_CLIENT_ID:""}'
  client_secret: '${AZURE_CLIENT_SECRET:""}'
  tenant_id: '${AZURE_TENANT_ID:""}'
  subscription_id: '${AZURE_SUBSCRIPTION_ID:""}'
  refresh_list_interval: 600s  
  resources:  
  - resource_query: "resourceType eq 'Microsoft.DBforPostgreSQL/flexibleServers'"
    metrics:  
    - name: ["*"]
      namespace: "Microsoft.DBforPostgreSQL/flexibleServers"

To test/verify that the 400 error is gone:

Check out this branch and set a breakpoint here before running the scenario in the debugger. Observe that

• the timegrains are no longer empty the first time we hit this line
• the program runs without the Azure API 400 error:

To test/verify that the number of metric definitions is unaffected:

• Note that this is also covered by an updated unit test of the TestMapMetric, so the below helper log is just an extra piece of verification
• One can compare with main and see that the number of metric definitions is 73 in both cases. Therefore, the number of metric definitions is unaffected.
  • There is a convenience debug log added in this PR to obtain this number. The log starts with unique metric definition count.

Testing minor nil pointer bug

To confirm that we handle the situation with a nil pointer, one can set up a debugger and set a breakpoint, then force the resp.Interval to nil at the first breakpoint in the screenshot. However, as noted in the comments in this code, this should not happen because we have handled the wildcard timegrain config scenario.

Related issues

• Fixes [azure monitor] wildcard metrics names (metrics.name: [*]) issues #43885
• Fixes [Azure Monitor] Check if resp.Interval for armmonitor list api can be nil #43725

[github-actions]

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

• run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

[mergify]

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @joecompute? 🙏.
For such, you'll need to label your PR with:

• The upcoming major version of the Elastic Stack
• The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

• backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit
• backport-active-all is the label that automatically backports to all active branches.
• backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
• backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

[joecompute]

@zmoog how do the backport tags look here?

[zmoog]

@zmoog how do the backport tags look here?

We can use backport-active-all to dynamically get a backport PR for each active version when we'll merge this.

[zmoog]

I see 6 API calls for both the main version and this PR. So, for the given configuration, the changes in this PR do't increase the number of calls.

[mergify]

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b fix/wildcard-metrics-timegrain-compatibility upstream/fix/wildcard-metrics-timegrain-compatibility
git merge upstream/main
git push upstream fix/wildcard-metrics-timegrain-compatibility

[MichaelKatsoulis]

Thank you @joecompute for putting this together. Excellent work!

[github-actions]

@Mergifyio backport 8.18 8.19 9.0 9.1

[mergify]

backport 8.18 8.19 9.0 9.1

✅ Backports have been created

• #46606 [8.18](backport #46145) [azure monitor] Address wildcard metrics names timegrain issue has been created for branch 8.18
• #46607 [8.19](backport #46145) [azure monitor] Address wildcard metrics names timegrain issue has been created for branch 8.19
• #46609 [9.0](backport #46145) [azure monitor] Address wildcard metrics names timegrain issue has been created for branch 9.0
• #46610 [9.1](backport #46145) [azure monitor] Address wildcard metrics names timegrain issue has been created for branch 9.1