fix: Improve AzureMonitorScraper Error Handling for Time Series Missing Requested Dimension Value

[hkfgo]

Fixes #2331

At Axon, we observed that Azure Monitor may, from time to time, return time series without the requested dimension. The previous try catch logic is at the metric level, ensuring that failure to scrape one metric will not affect all metrics; however, there are many time series within a metric, and failure to process one time series will result in a metric gap for that metric. I am adding a try-catch when looping over time series to prevent that from happening.

I am very new to this repo though, and couldn't find my way around how to write tests for this. Some help would be much appreciated.

[CLAassistant]

All committers have signed the CLA.

[github-actions[bot]]

Thank you for your contribution! 🙏 We will review it as soon as possible.

[hkfgo]

Added a few comments, would you mind updating the changelog please?

Thanks for the review! Besides everything that's already talked about, I'd also like to know where I should put tests. I couldn't really find unit tests for AzureMonitorScraper/Client

[tomkerkhove]

Added a few comments, would you mind updating the changelog please?

Thanks for the review! Besides everything that's already talked about, I'd also like to know where I should put tests. I couldn't really find unit tests for AzureMonitorScraper/Client

Because unfortunately we don't have any given the tight integration and mocking everything is too much work/brittle

[tomkerkhove]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

xchen seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it.

Please make sure to sign the CLA

[hkfgo]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. xchen seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it.

Please make sure to sign the CLA

Ah okay, I think I signed it with my personal account, but committed with my company's account.

[tomkerkhove]

We're good to go! Would you mind checking the merge conflict + code factor flag please?

[hkfgo]

Hey Tom, looks like the code is conflict-free now. Although build is failing on a single failing integration test. The exception looks like something transient unrelated to my changes?

System.Net.Http.HttpRequestException : Error while copying content to a stream.

Would you mind re-running the pipeline again? I don't think I have the privileges

[tomkerkhove]

/azp run Promitor CI - Scraper Agent

[azure-pipelines[bot]]

No commit pushedDate could be found for PR 2345 in repo tomkerkhove/promitor

[tomkerkhove]

/azp run Promitor CI - Scraper Agent

[azure-pipelines[bot]]

No commit pushedDate could be found for PR 2345 in repo tomkerkhove/promitor

[hkfgo]

Thanks for responding in your evening Tom :)

My most recent commit should have fixed warnings reported by R#(they were indeed real problems)

[tomkerkhove]

Happy to help a contributor out :)

Thanks!

[hkfgo]

Hey Tom, just letting you know that this has not been forgotten :) I was just a bit swamped these past few days. I'll look into how to run R# locally and also that one failing integration test when I get a chance.

Wrt to that test - seems like it timed out for whatever reason. I have to look at exactly what the code is myself to see if it's related to my changes. You have way more context though, so maybe you could share some potential insights?

[hkfgo]

Hey Tom, it's ready for another look. I see that there was a recent feature added to master that supported scraping multiple dimensions. You might wanna have a second look at if I merged correctly(I think I did). Thanks!

[tomkerkhove]

/azp run Promitor CI - Scraper Agent

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).

[tomkerkhove]

We should be good - Thanks!

[tomkerkhove]

Thank you!