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

Migrate to new Azure SDK #2209

Open
tomkerkhove opened this issue Jan 3, 2023 · 10 comments
Open

Migrate to new Azure SDK #2209

tomkerkhove opened this issue Jan 3, 2023 · 10 comments
Assignees
Labels
dependencies All issues related to dependencies & Renovate

Comments

@tomkerkhove
Copy link
Owner

Migrate to new Azure SDK as the older ones are deprecated (see azure-deprecation/dashboard#217)

@hkfgo
Copy link
Contributor

hkfgo commented Mar 23, 2024

Hey Tom, could you confirm that this is the new SDK to migrate to? I see a few other "Monitor" packages here but they seem to serve different purposes(like managing alerts.

I am willing to contribute code for this issue :)

@tomkerkhove
Copy link
Owner Author

tomkerkhove commented Mar 26, 2024

That is the SDK of the new data-plane API which is tracked in #2284

We'd need to figure out which one provides the same capabilities to fetch metrics through ARM for this issue to not cause breaking change (because new API is billed)

@hkfgo
Copy link
Contributor

hkfgo commented Mar 26, 2024

I believe both single resource and batch queries are supported under the same package. See this section of the README. It should provide the same capability, outside of the billing issue you mentioned. I found the pricing model for the new SDK, presumably billed under Platform and custom metric queries (preview). How does the billing for the old SDK work?

Do the old and new SDKs go through separate Azure Monitor APIs? I thought there's only one REST API for single-resource metric scraping

@hkfgo
Copy link
Contributor

hkfgo commented Mar 26, 2024

The mental roadmap I had before for batch scraping was

  1. Migrate to that new SDK, and make sure all existing features still work
  2. Implement batch scraping using the same SDK.

However, I wonder if things should change if we think it doesn't make sense to use the new SDK for single-resource scraping due to billing issues. Perhaps just use the SDK for batch scraping since that wouldn't be a breaking change? Wdyt?

@hkfgo
Copy link
Contributor

hkfgo commented Mar 27, 2024

(because new API is billed)

Thinking more about it, why would billing be changed exactly? I'd think the new SDK is, most likely, purely a client side change that does not affect which Azure Monitor API gets used and how those API calls are billed right? There is only one REST API for single-resource scraping anyways.

In any case, I'm reaching out to our Microsoft rep for an answer

@tomkerkhove
Copy link
Owner Author

Sorry for the slow reply.

How does the billing for the old SDK work?

It is simple, it was for free as the old SDK queries metrics through ARM. This new package, however, is specific to Azure Monitor and most probably uses the new data-plane API which is billed though.

@tomkerkhove
Copy link
Owner Author

So how I see it is:

  • Batch scraping - Using new charged API is a must, we can't support it on old API and is new capability
  • Single-resource scraping - This should be feature flagged and use the old API by default, but allow end-users to use new charged API when they want to

Once this is in place, we deprecate the old way and start flipping the feature flag to use new charged API in a next version given this is better for Promitor but has cost implications.

@hkfgo
Copy link
Contributor

hkfgo commented Apr 9, 2024

Single-resource scraping - This should be feature flagged and use the old API by default, but allow end-users to use new charged API when they want to
Agreed on the feature flag. However, I did find that only the batch client gores through the new path. The new client for single-resource scraping still goes through ARM. See their source code. There should be no change in billing. So once single-resource scraping under the new SDK is stable, we should be able to completely deprecate the old API. I'm already in this mental space now so I will contribute a PR for it first, then head back to batch scraping

@tomkerkhove
Copy link
Owner Author

@hkfgo Do you know if you'll have time next week to complete the other PRs? I'd like to cut a release next week.

@hkfgo
Copy link
Contributor

hkfgo commented Jun 1, 2024

Hey I should be able get the SDK migration PRs over the hump. To put feature flag under azureMonitor. would require a bit of code change I believe, so be on the look out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies All issues related to dependencies & Renovate
Projects
Status: Todo
Development

No branches or pull requests

2 participants