Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Changes from all commits
54971ad
4e8f704
72cc091
7697ae2
fc894df
4a15684
66590a4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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:
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.
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.
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.
what was ofusage.actualCosts?
https://github.com/elastic/beats/pull/32589/files#diff-4cdd545c5b3b0cfc2223812bc91592664236894dc885646b9fad7608c1c384e9R76-R77
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?