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

[Azure Billing] Update forecast api metrics interval #36142

Merged
merged 8 commits into from
Jul 27, 2023

Conversation

muthu-mps
Copy link
Contributor

@muthu-mps muthu-mps commented Jul 24, 2023

  • Bug

What does this PR do?

Fixes #35633

Why is it important?

The proposed change fetches the forecast data for the next 30 days from the current day(-2). As of today, the forecast metrics is fetched from the beginning of the month to the end of the month. Considering the fact that this API returns the actual data to consolidate the cost we are adjusting the start date to current day - 2 days.

How to test this PR locally

  • Enable the Azure billing metrics module.
  • Perform the configuration
  • Verify the billing metrics forecast API fetches metrics from yesterday and for next 30 days.
  • Ensure there the other metrics are getting loaded in the dashboards.

Related issues

Screenshots

Screenshot 2023-07-24 at 1 00 30 PM

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 24, 2023
@botelastic
Copy link

botelastic bot commented Jul 24, 2023

This pull request doesn't have a Team:<team> label.

@mergify
Copy link
Contributor

mergify bot commented Jul 24, 2023

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @muthu-mps? 🙏.
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-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jul 24, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-07-26T05:57:41.496+0000

  • Duration: 50 min 22 sec

Test stats 🧪

Test Results
Failed 0
Passed 1478
Skipped 90
Total 1568

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

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

@muthu-mps muthu-mps requested a review from zmoog July 24, 2023 14:50
@muthu-mps muthu-mps marked this pull request as ready for review July 24, 2023 14:51
@muthu-mps muthu-mps requested a review from a team as a code owner July 24, 2023 14:51
Copy link
Contributor

@zmoog zmoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Forecast API returns two kinds of data in the same response:

  • actual data, the consolidated past costs.
  • forecast data, the forecast of future costs.

By changing the interval to -1 day and +30 days, we are getting the forecast data but missing the actual data. This happens because actual data are past costs that require some time to consolidate.

Here's the actual response using the PR code:

CleanShot 2023-07-25 at 22 35 57@2x

Here's a quick test with -2 days instead of -1 days, so we give actual data to consolidate:

CleanShot 2023-07-25 at 22 37 26@2x

@zmoog
Copy link
Contributor

zmoog commented Jul 25, 2023

We also need to update the CHANGELOG.next.asciidoc with and entry for this bug fix.

@muthu-mps muthu-mps requested a review from zmoog July 26, 2023 07:33
Copy link
Contributor

@zmoog zmoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this change, and it looks good.

On each collection, we're now getting:

  • one day of actual costs
  • 30 days of forecast costs

So we expect the integration to accumulate actual costs over time and have a 30 days outlook of forecasted costs.

@muthu-mps
Copy link
Contributor Author

I tested this change, and it looks good.

On each collection, we're now getting:

  • one day of actual costs
  • 30 days of forecast costs

So we expect the integration to accumulate actual costs over time and have a 30 days outlook of forecasted costs.

Thanks @zmoog !

@muthu-mps muthu-mps merged commit 7fd27bf into elastic:main Jul 27, 2023
21 checks passed
Scholar-Li pushed a commit to Scholar-Li/beats that referenced this pull request Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs_team Indicates that the issue/PR needs a Team:* label
Projects
None yet
3 participants