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
[Azure Billing] Switch to Cost Management API for forecast data #32589
[Azure Billing] Switch to Cost Management API for forecast data #32589
Conversation
e06bf12
to
5389aa1
Compare
// | ||
|
||
actualCosts, err := client.BillingService.GetForecast(fmt.Sprintf("properties/chargeType eq '%s'", "Actual")) |
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.
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.
@aspacca, we probably should add what actual/forecast data are, WDYT?
) | ||
if err != nil { | ||
return usage, fmt.Errorf("retrieving usage details failed in client: %w", err) | ||
} | ||
|
||
usage.UsageDetails = usageDetails.Values() | ||
for paginator.NotDone() { |
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.
you didn't change the behaviour here, but does it make sense to change and fetch forecasts even if we don't have usage details?
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.
The module collects Usage Details data and forecast data at different time intervals:
- usage details: previous day (24 hours)
- forecast: current month (30 days)
Suppose we have zero usage in the last 24h and some resource usage in the previous days. In this case, usage details would be empty, but we could have some forecasts based on the days before.
WDYT? I have no previous knowledge of this topic, I reconstructed it from the codebase and Microsoft docs, so any question like this one is more than welcome!
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.
Suppose we have zero usage in the last 24h and some resource usage in the previous days. In this case, usage details would be empty, but we could have some forecasts based on the days before.
WDYT? I have no previous knowledge of this topic, I reconstructed it from the codebase and Microsoft docs, so any question like this one is more than welcome!
I indeed didn't think about the fact that the call to the usage details could fail because there are zero usage in the last 24h :)
Mine was a more generic doubt about the fact that they are two different API calls, that can fail each for different reasons, even transient (ie: by the time we call the first we have a network outage that will be solved by the time of the next call, or just one upstream service is down while the other is up). Does it make sense to keep fetching the second metrics if fetching the first fails?
I would look the answer to this in the dashboards or the way the two metrics are related to each others.
Taking the example you made, and assuming having zero usage data is the reason for a possible failure: I see it can make sense to see in a dashboard that I have zero usage and the forecast is horizontal, rather than missing at all any information. At the same time is the usage is zero and the forecast increase this can be an hint that something is not working properly.
Probably @ravikesarwani might have an opinion here as well.
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.
Does it make sense to keep fetching the second metrics if fetching the first fails?
Yep, I think it makes sense.
In the dashboard, usage details and forecasts have independent visualizations: we don't have one visualization that uses data from both. However, the sum of the usage details matches the amount of actual data in the forecast for the same day.
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.
I'm in favor of changing the logic and continuing with the forecast even if the call for usage details fails for some reason.
@ravikesarwani, let us know what's on your mind.
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.
great work. LGTM
This pull request does not have a backport label.
|
a236100
to
ab0eb25
Compare
This pull request is now in conflicts. Could you fix it? 🙏
|
The previous endpoint from Consumption API is no longer maintained and does not support scope.
After testing with different values of includeFreshPartialCost, the best trade-off with the current design of the dashboard is to set it to false and only use consolidated/final data.
Thanks golangci-lint!
Stopping the processing of the whole set of rows when we detect a wrong row is too hard. With this change, when something is wrong with a row with it we: - log the circumstances - stop processing the current row - continue with the next ones
ab0eb25
to
66590a4
Compare
* Switch forecast to Cost Management API The previous endpoint from Consumption API is no longer maintained and does not support scope. * Add comments with findings from tests After testing with different values of includeFreshPartialCost, the best the trade-off with the current design of the dashboard is to set it to false and only use consolidated/final data. * Drop rows with an unexpected format and continue Stopping the processing of the whole set of rows when we detect the wrong row is too hard. With this change, when something is wrong with a row with it we: - log the circumstances - stop processing the current row - continue with the next ones (cherry picked from commit 86b111d) # Conflicts: # x-pack/metricbeat/module/azure/billing/billing.go # x-pack/metricbeat/module/azure/billing/billing_test.go # x-pack/metricbeat/module/azure/billing/client.go # x-pack/metricbeat/module/azure/billing/client_test.go # x-pack/metricbeat/module/azure/billing/data.go # x-pack/metricbeat/module/azure/billing/data_test.go # x-pack/metricbeat/module/azure/billing/mock_service.go # x-pack/metricbeat/module/azure/billing/service.go
* Switch forecast to Cost Management API The previous endpoint from Consumption API is no longer maintained and does not support scope. * Add comments with findings from tests After testing with different values of includeFreshPartialCost, the best the trade-off with the current design of the dashboard is to set it to false and only use consolidated/final data. * Drop rows with an unexpected format and continue Stopping the processing of the whole set of rows when we detect the wrong row is too hard. With this change, when something is wrong with a row with it we: - log the circumstances - stop processing the current row - continue with the next ones (cherry picked from commit 86b111d)
…) (#33024) * Switch forecast to Cost Management API The previous endpoint from Consumption API is no longer maintained and does not support scope. * Add comments with findings from tests After testing with different values of includeFreshPartialCost, the best the trade-off with the current design of the dashboard is to set it to false and only use consolidated/final data. * Drop rows with an unexpected format and continue Stopping the processing of the whole set of rows when we detect the wrong row is too hard. With this change, when something is wrong with a row with it we: - log the circumstances - stop processing the current row - continue with the next ones (cherry picked from commit 86b111d) Co-authored-by: Maurizio Branca <maurizio.branca@gmail.com>
* Switch forecast to Cost Management API The previous endpoint from Consumption API is no longer maintained and does not support scope. * Add comments with findings from tests After testing with different values of includeFreshPartialCost, the best the trade-off with the current design of the dashboard is to set it to false and only use consolidated/final data. * Drop rows with an unexpected format and continue Stopping the processing of the whole set of rows when we detect the wrong row is too hard. With this change, when something is wrong with a row with it we: - log the circumstances - stop processing the current row - continue with the next ones (cherry picked from commit 86b111d)
* Switch forecast to Cost Management API The previous endpoint from Consumption API is no longer maintained and does not support scope. * Add comments with findings from tests After testing with different values of includeFreshPartialCost, the best the trade-off with the current design of the dashboard is to set it to false and only use consolidated/final data. * Drop rows with an unexpected format and continue Stopping the processing of the whole set of rows when we detect the wrong row is too hard. With this change, when something is wrong with a row with it we: - log the circumstances - stop processing the current row - continue with the next ones (cherry picked from commit 86b111d)
* Switch forecast to Cost Management API The previous endpoint from Consumption API is no longer maintained and does not support scope. * Add comments with findings from tests After testing with different values of includeFreshPartialCost, the best the trade-off with the current design of the dashboard is to set it to false and only use consolidated/final data. * Drop rows with an unexpected format and continue Stopping the processing of the whole set of rows when we detect the wrong row is too hard. With this change, when something is wrong with a row with it we: - log the circumstances - stop processing the current row - continue with the next ones (cherry picked from commit 86b111d)
* Switch forecast to Cost Management API The previous endpoint from Consumption API is no longer maintained and does not support scope. * Add comments with findings from tests After testing with different values of includeFreshPartialCost, the best the trade-off with the current design of the dashboard is to set it to false and only use consolidated/final data. * Drop rows with an unexpected format and continue Stopping the processing of the whole set of rows when we detect the wrong row is too hard. With this change, when something is wrong with a row with it we: - log the circumstances - stop processing the current row - continue with the next ones (cherry picked from commit 86b111d)
* Switch forecast to Cost Management API The previous endpoint from Consumption API is no longer maintained and does not support scope. * Add comments with findings from tests After testing with different values of includeFreshPartialCost, the best the trade-off with the current design of the dashboard is to set it to false and only use consolidated/final data. * Drop rows with an unexpected format and continue Stopping the processing of the whole set of rows when we detect the wrong row is too hard. With this change, when something is wrong with a row with it we: - log the circumstances - stop processing the current row - continue with the next ones (cherry picked from commit 86b111d)
… for forecast data (#33023) * [Azure Billing] Switch to Cost Management API for forecast data (#32589) * Switch forecast to Cost Management API The previous endpoint from Consumption API is no longer maintained and does not support scope. * Add comments with findings from tests After testing with different values of includeFreshPartialCost, the best the trade-off with the current design of the dashboard is to set it to false and only use consolidated/final data. * Drop rows with an unexpected format and continue Stopping the processing of the whole set of rows when we detect the wrong row is too hard. With this change, when something is wrong with a row with it we: - log the circumstances - stop processing the current row - continue with the next ones (cherry picked from commit 86b111d) * Fix conflicts * Update NOTICE.txt * Fix linter objections * Fix NOTICE * Fix missing log initialization Co-authored-by: Maurizio Branca <maurizio.branca@gmail.com>
* Switch forecast to Cost Management API The previous endpoint from Consumption API is no longer maintained and does not support scope. * Add comments with findings from tests After testing with different values of includeFreshPartialCost, the best the trade-off with the current design of the dashboard is to set it to false and only use consolidated/final data. * Drop rows with an unexpected format and continue Stopping the processing of the whole set of rows when we detect the wrong row is too hard. With this change, when something is wrong with a row with it we: - log the circumstances - stop processing the current row - continue with the next ones
What does this PR do?
Switch the fetching of forecast data from the Azure Consumption API to the Cost Management API.
Why is it important?
The previous forecast endpoint from Consumption API is no longer maintained and does not support scope.
Additional information
The Consumption API version of the forecast endpoint was simple: it offered a simple filter with limited options. Instead, the Cost Management API version is a much richer endpoint with a dozen of options. The current implementation focuses on offering the same functionality using the new API.
The Azure Portal uses the Cost Management API (version 2021-10-01) to implement the Cost Management > Cost analysis page. We used the Azuire Portal as a reference to build the request for the Forecast data in this module.
This Azure Billing module is using an older version of the Cost Management API: 2019-10-01, it is the latest working 1 version offered in the official Azure SDK. If we need to jump to a later version, we built a fallback solution: the github.com/zmoog/azure-sdk-clients repo contains an unofficial Azure SDK client generated from the OpenAPI spec using the same toolchain Microsoft uses.
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.How to test this PR locally
On Azure:
CLIENT_ID
.CLIENT_SECRET
.On your local environment:
You may need to use
output.elasticsearch.allow_older_versions=true
if you built Metricbeat from sources and not using the latest version of ES.Related issues
Footnotes
Version 2020-06-01 does not work for Forecast. ↩