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

feat(comparison_alerts): Translate delta percents into comparison percents in the api layer. #28787

Merged
merged 3 commits into from
Sep 23, 2021

Conversation

wedamija
Copy link
Member

@wedamija wedamija commented Sep 23, 2021

The frontend represents percentages as delta change percents. For example:

  • 30% increase
  • 40% decrease.

This makes sense from a user point of view, but doesn't fit as well into how our system handles
thesholds. For example, with decrease, if we had threshold like:

  • critical at 40% below the comparison period
  • resolve at 30% below the comparison period

This would fail our validation, since resolve thresholds on below alerts need to be greater than the
critical/warning thresholds. It also makes comparisons in the subscription processor more difficult.

To work around this, we convert these delta percents into comparison percents. This means that:

  • 30% increase -> 130% comparison percent
  • 40% decrease -> 60% comparison percent.

This fits well into our validation, since now if we have a decrease % alert like

  • critical at 40% below the comparison period (60% of comparison period)
  • resolve at 30% below the comparison period (70% of comparison period)

The resolve threshold is now higher than the critical, and validation will succeed.

Handling this complexity at the api layer seems to work the best, so that internally we can assume
that all of these threshold are comparison percents.

I also considered making increase deltas positive and decrease deltas negative, but I felt like this
reads weirdly in terms of our data schema (we'd have a below trigger that looks like < -30%, if you
just looked at the data).

…cents in the api layer.

The frontend represents percentages as delta change percents. For example:
 - 30% increase
 - 40% decrease.

This makes sense from a user point of view, but doesn't fit as well into how our system handles
thesholds. For example, with decrease, if we had threshold like:

- critical at 40%
- resolve at 30%

This would fail our validation, since resolve thresholds on below alerts need to be greater than the
critical/warning thresholds. It also makes comparisons in the subscription processor more difficult.

To work around this, we convert these delta percents into comparison percents. This means that:

 - 30% increase -> 130% comparison percent
 - 40% decrease -> 60% comparison percent.

This fits well into our validation, since now if we have a decrease % alert like

- critical at 40% (60% comparison)
- resolve at 30% (70% comparison)

The resolve threshold is now higher than the critical, and validation will fire.

Handling this complexity at the api layer seems to work the best, so that internally we can assume
that all of these threshold are comparison percents.

I also considered making increase deltas positive and decrease deltas negative, but I felt like this
reads weirdly in terms of our data schema (we'd have a below trigger that looks like < -30%, if you
just looked at the data).
@wedamija wedamija requested review from ahmedetefy and a team September 23, 2021 01:39
Copy link
Contributor

@ahmedetefy ahmedetefy left a comment

Choose a reason for hiding this comment

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

Aside from the failing test, LGTM

@wedamija wedamija enabled auto-merge (squash) September 23, 2021 21:59
@wedamija wedamija merged commit 00fa206 into master Sep 23, 2021
@wedamija wedamija deleted the danf/cmp_alerts_non_relative_api branch September 23, 2021 22:21
vuluongj20 pushed a commit that referenced this pull request Sep 30, 2021
…cents in the api layer. (#28787)

The frontend represents percentages as delta change percents. For example:
 - 30% increase
 - 40% decrease.

This makes sense from a user point of view, but doesn't fit as well into how our system handles
thresholds. For example, with decrease, if we had threshold like:

- critical at 40%
- resolve at 30%

This would fail our validation, since resolve thresholds on below alerts need to be greater than the
critical/warning thresholds. It also makes comparisons in the subscription processor more difficult.

To work around this, we convert these delta percents into comparison percents. This means that:

 - 30% increase -> 130% comparison percent
 - 40% decrease -> 60% comparison percent.

This fits well into our validation, since now if we have a decrease % alert like

- critical at 40% (60% comparison)
- resolve at 30% (70% comparison)

The resolve threshold is now higher than the critical, and validation will fire.

Handling this complexity at the api layer seems to work the best, so that internally we can assume
that all of these threshold are comparison percents.

I also considered making increase deltas positive and decrease deltas negative, but I felt like this
reads weirdly in terms of our data schema (we'd have a below trigger that looks like < -30%, if you
just looked at the data).
@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants