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

r/burn_alert: Add support for budget rate burn alerts #391

Merged
merged 11 commits into from
Nov 15, 2023

Conversation

lafentres
Copy link
Contributor

@lafentres lafentres commented Nov 10, 2023

Short description of the changes

This PR adds support for budget rate burn alerts to the burn alert resource.

How to verify that this has the expected result

Pre-requisites:

  • You'll need Go 1.20 and the latest Terraform installed.
  • You'll need a team in Honeycomb to test against.
  • You'll need a dataset, derived column, and SLO for that team.
  • You'll need an API key for that team.

Setup:

  1. Create a new directory called providers somewhere you can easily access (but not inside the same folder as your Terraform config).
  2. Build the provider by checking out this branch and running make build
  3. Move the executable created to the providers directory you created earlier.
    mv terraform-provider-honeycombio /PATH/TO/YOUR/DIRECTORY/providers
    
  4. Add the following to your ~/.terraformrc file:
    provider_installation {
    
      # Use /PATH/TO/YOUR/DIRECTORY/providers/terraform-provider-honeycombio
      # as an overridden package directory for the honeycombio/honeycombio provider. 
      # This disables the version and checksum verifications for this provider and 
      # forces Terraform to look for the null provider plugin in the given directory.
      #dev_overrides {
      #  "honeycombio/honeycombio" = "/PATH/TO/YOUR/DIRECTORY/providers"
      #}
    
      # For all other providers, install them directly from their origin provider
      # registries as normal. If you omit this, Terraform will _only_ use
      # the dev_overrides block, and so no other providers will be available.
      direct {}
    }
    
  5. Create another new directory (all Terraform commands will be run in this directory)
  6. Download and save this file as main.tf in your new directory. You'll be modifying it in the steps below.
  7. Set the following environment variables:
    export HONEYCOMB_API_KEY=YOUR_API_KEY_HERE
    export TF_VAR_dataset=YOUR_DATASET_SLUG_HERE
    export TF_VAR_slo_id=YOUR_SLO_ID_HERE
    
  8. Run terraform init.

Provision resources for r/burn_alert:

  1. Run terraform apply. This should succeed and if you check in the UI, you should see the burn alerts you just created.
  2. Run terraform plan. There should be no changes.

Check that upgrading provider version doesn't break anything:

  1. Update your provider version to local build of this branch by editing your ~/.terraformrc file and remove the comments in front of the dev_overrides block
      dev_overrides {
        "honeycombio/honeycombio" = "/PATH/TO/YOUR/DIRECTORY/providers"
      }
    
  2. Run terraform plan. This time you should see a yellow warning block telling you that development overrides are in place. It should look like this:
    Screenshot 2023-11-13 at 3 34 46 PM

Check that validation works:

  1. Uncomment the 2nd and 3rd burn alerts in your config.
  2. Run terraform apply. This should succeed and if you check in the UI, you should see the 2 new burn alerts you just created.
  3. Try updating each of your burn alerts to be invalid. Examples include:
    • Invalid alert_type
    • Exhaustion time alert without exhaustion_minutes specified
    • Exhaustion time alert with exhaustion_minutes < 0
    • Exhaustion time alert with budget rate fields specified
    • Budget rate alert without budget rate fields specified
    • Budget rate alert with budget_rate_window_minutes < 60 or > the SLO's time period
    • Budget rate alert with budget_rate_decrease_percent < 0.0001 or > 100
    • Budget rate alert with budget_rate_decrease_percent with more than 4 non-zero digits past the decimal
    • Budget rate alert with exhaustion_minutes specified
  4. Run terraform apply. This should fail and you should see descriptive errors about what attributes have been misconfigured.
  5. Undo any changes to your config before moving on

Check that updating burn alerts works:

  1. Update each of your burn alerts by changing any valid combo of alert_type, exhaustion_minutes, budget_rate_window_minutes, or budget_rate_decrease_percent. Examples of useful scenarios to test out:
    • Exhaustion time alert to a budget rate alert
    • Budget rate alert to an exhaustion time alert
    • Budget rate burn alert to an exhaustion time burn alert without specifying the alert_type
    • Exhaustion time alert to have exhaustion_minutes = 0
  2. Run terraform apply. The output should show the changes you made in the config. After confirming, you should see these changes reflected in the UI.
  3. Run terraform plan. Your plan should show no changes.

Check that destroy works

  1. Run terraform destroy. This should succeed. You should see these changes reflected in the UI.

Check that a fresh create works

  1. Run terraform apply. This should succeed. You should see these changes reflected in the UI.

Related links

@lafentres lafentres added the feature This issue wants to add new functionality. label Nov 10, 2023
@lafentres lafentres self-assigned this Nov 10, 2023
@lafentres lafentres force-pushed the lafentres.budget-rate-burn-alerts branch 2 times, most recently from 9b3ce22 to 08d2b05 Compare November 13, 2023 20:49
@lafentres lafentres changed the title [DRAFT] Add support for budget rate burn alerts to r/burn_alerts Add support for budget rate burn alerts to r/burn_alerts Nov 13, 2023
@lafentres lafentres marked this pull request as ready for review November 13, 2023 21:41
@lafentres lafentres requested a review from a team as a code owner November 13, 2023 21:41
@lafentres lafentres changed the title Add support for budget rate burn alerts to r/burn_alerts r/burn_alerts: Add support for budget rate burn alerts Nov 13, 2023
@lafentres lafentres changed the title r/burn_alerts: Add support for budget rate burn alerts r/burn_alert: Add support for budget rate burn alerts Nov 13, 2023
@lafentres lafentres force-pushed the lafentres.budget-rate-burn-alerts branch 2 times, most recently from bf4fdbd to e66c1b7 Compare November 14, 2023 15:10
@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2023

Codecov Report

Attention: Patch coverage is 97.82609% with 5 lines in your changes missing coverage. Please review.

Project coverage is 80.82%. Comparing base (5068dc5) to head (f6ebbc7).
Report is 130 commits behind head on main.

Files with missing lines Patch % Lines
internal/provider/burn_alert_resource.go 98.39% 2 Missing and 1 partial ⚠️
internal/helper/validation/precision_at_most.go 93.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #391      +/-   ##
==========================================
+ Coverage   80.81%   80.82%   +0.01%     
==========================================
  Files          56       62       +6     
  Lines        4565     4809     +244     
==========================================
+ Hits         3689     3887     +198     
- Misses        722      768      +46     
  Partials      154      154              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lafentres lafentres force-pushed the lafentres.budget-rate-burn-alerts branch from e66c1b7 to 728c093 Compare November 14, 2023 15:27
internal/provider/burn_alert_resource.go Outdated Show resolved Hide resolved
internal/provider/burn_alert_resource.go Outdated Show resolved Hide resolved
internal/provider/burn_alert_resource.go Show resolved Hide resolved
@lafentres lafentres marked this pull request as draft November 14, 2023 17:00
@lafentres lafentres changed the title r/burn_alert: Add support for budget rate burn alerts [DRAFT] r/burn_alert: Add support for budget rate burn alerts Nov 14, 2023
@lafentres lafentres force-pushed the lafentres.budget-rate-burn-alerts branch 3 times, most recently from beb2340 to fa7b8bc Compare November 14, 2023 22:16
Copy link
Collaborator

@jharley jharley left a comment

Choose a reason for hiding this comment

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

Just took another quick pass, but this is coming together great. Thanks for the care in crafting such thorough tests

internal/helper/type_float.go Outdated Show resolved Hide resolved
docs/resources/burn_alert.md Outdated Show resolved Hide resolved
internal/helper/type_int.go Outdated Show resolved Hide resolved
@lafentres lafentres force-pushed the lafentres.budget-rate-burn-alerts branch 4 times, most recently from e3aa6ad to 256ee69 Compare November 15, 2023 16:12
@lafentres lafentres force-pushed the lafentres.budget-rate-burn-alerts branch 2 times, most recently from e57ebf1 to 8209221 Compare November 15, 2023 16:18
@lafentres lafentres changed the title [DRAFT] r/burn_alert: Add support for budget rate burn alerts r/burn_alert: Add support for budget rate burn alerts Nov 15, 2023
@lafentres lafentres marked this pull request as ready for review November 15, 2023 16:21
@lafentres lafentres requested a review from jharley November 15, 2023 16:22
@lafentres lafentres force-pushed the lafentres.budget-rate-burn-alerts branch from 8209221 to 215ddc0 Compare November 15, 2023 17:09
value: types.Float64Value(123),
maxPrecision: 2,
},
// trailing zeros won't impact the conversion from percent to ppm so there's no need to fail
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

@jharley jharley left a comment

Choose a reason for hiding this comment

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

🚀

@lafentres lafentres force-pushed the lafentres.budget-rate-burn-alerts branch from 215ddc0 to f6ebbc7 Compare November 15, 2023 17:19
@lafentres lafentres merged commit 0d3a6b2 into main Nov 15, 2023
3 checks passed
@lafentres lafentres deleted the lafentres.budget-rate-burn-alerts branch November 15, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue wants to add new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants