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.

Sign up for GitHub

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

Jump to bottom

feat(cmp_alerts): Show change alert status in the metric alert builder chart #29201

Merged
merged 8 commits into from
Oct 13, 2021
Merged

feat(cmp_alerts): Show change alert status in the metric alert builder chart #29201

merged 8 commits into from
Oct 13, 2021

Conversation

taylangocmen
Copy link
Contributor

@taylangocmen taylangocmen commented Oct 8, 2021 •
edited
Loading

This updates the metric alert builder chart for change alerts. It removes the threshold lines from the chart and adds the status areas based on the change percentage if a change percentage threshold and changeDelta period is selected in the alert builder, after picking Percent Change for the threshold type.

Example:
Screen Shot 2021-10-13 at 1 00 36 AM

Jira: WOR-1409
Figma: Alerts-V3

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2021 •
edited
Loading

size-limit report

Path Base Size (c386c95) Current Size Change
src/sentry/static/sentry/dist/entrypoints/app.js 52.91 KB 52.9 KB -0.02% 🔽
src/sentry/static/sentry/dist/entrypoints/sentry.css 70.9 KB 70.9 KB 0%

@taylangocmen taylangocmen marked this pull request as ready for review October 8, 2021 17:16
@taylangocmen taylangocmen requested a review from a team as a code owner October 8, 2021 17:16
@taylangocmen taylangocmen requested review from a team and removed request for a team October 8, 2021 17:16
matejminar
matejminar approved these changes Oct 11, 2021
View reviewed changes
Copy link
Member

@matejminar matejminar left a comment

Choose a reason for hiding this comment

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

Left couple of minor suggestions, other than that, lgtm.

@taylangocmen taylangocmen force-pushed the taylangocmen/metric-alert-change-graph branch from 928b537 to 38be1c8 Compare October 12, 2021 16:59
@taylangocmen taylangocmen requested a review from a team October 12, 2021 16:59
@taylangocmen taylangocmen requested a review from a team as a code owner October 12, 2021 16:59
@taylangocmen taylangocmen force-pushed the taylangocmen/metric-alert-change-graph branch from 38be1c8 to 8cc701d Compare October 12, 2021 17:32
@taylangocmen taylangocmen force-pushed the taylangocmen/metric-alert-change-graph branch from 8cc701d to 6b678e3 Compare October 13, 2021 04:07
@taylangocmen taylangocmen requested review from matejminar and a team and removed request for a team October 13, 2021 08:03
@taylangocmen taylangocmen removed the WIP label Oct 13, 2021
@taylangocmen taylangocmen force-pushed the taylangocmen/metric-alert-change-graph branch from 74592be to dfd8963 Compare October 13, 2021 21:10
@taylangocmen taylangocmen merged commit 11aa52c into master Oct 13, 2021
@taylangocmen taylangocmen deleted the taylangocmen/metric-alert-change-graph branch October 13, 2021 21:36
@github-actions github-actions bot locked and limited conversation to collaborators Oct 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@matejminar matejminar matejminar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@taylangocmen @matejminar