Skip to content

Ensure burn rate factors are accurate, and add exhaustion label#775

Open
saswatamcode wants to merge 4 commits intopyrra-dev:mainfrom
saswatamcode:burn-rate-factor
Open

Ensure burn rate factors are accurate, and add exhaustion label#775
saswatamcode wants to merge 4 commits intopyrra-dev:mainfrom
saswatamcode:burn-rate-factor

Conversation

@saswatamcode
Copy link
Copy Markdown
Contributor

This PR does the following,

  • Updates the burn rate factors in Windows() function from 14, 7, 2, 1 to 14, 5.6, 2.8, 1 to more accurately represent 28 day burn rate in the following cases (as per conversation offline with @metalmatze),
    • 50% error budget depleted in a day / 100% in 2 days
    • 20% error budget depleted in a day / 100% in 5 days
    • 10% error budget depleted in a day / 100% in 10 days
    • 100% error budget depleted in 28 days
  • It also adds goDoc comments to Windows which describe its functionality for posterity.
  • Adds a new exhaustion label to all budget burn alerts, which represents time till the error budget is fully exhausted.
  • Adds new test cases to Test_windows to account for various common SLO windows like 90d, 84d, 30d, 28d, 14d & 7d

Most of this large diff is to account for changes in tests. Let me know what you think! 🙂

… SLO windows

Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
Copy link
Copy Markdown

@moadz moadz left a comment

Choose a reason for hiding this comment

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

LGTM :)

Copy link
Copy Markdown
Member

@metalmatze metalmatze left a comment

Choose a reason for hiding this comment

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

Super cool! Thanks.
Especially adding more tests all over the place! 👍

I'm still not 100% sure about the exhaustion label, since the budget might be gone way quicker depending on the current budget that's left and the potentially uneven amount of events happening over time. For now, let's see how it goes with that label.

Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
@saswatamcode saswatamcode requested a review from metalmatze June 16, 2023 06:54
@metalmatze
Copy link
Copy Markdown
Member

Long time no see.
Would you still like to get this in?

@metalmatze
Copy link
Copy Markdown
Member

Hey @saswatamcode, long time! Are you still interested in getting this merged? The burn rate factor corrections and the additional tests looked good.

If so, could you either rebase this PR onto current main or open a fresh PR with the same changes? We'd be happy to review and finally get it in. Let us know!

@saswatamcode
Copy link
Copy Markdown
Contributor Author

Hey! Sorry this dropped off my radar. Let me try to update these this week!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants