Terraform setup - #37
Conversation
7emansell
left a comment
There was a problem hiding this comment.
- Yes, we want to import the existing alarm and then modify it
- No, let's go ahead with the correct naming convention (the metric should be named SomethingError and the alarm should be named SomethingErrorAlarm)
- Yes, let's just keep it on prod for now
| import { | ||
| to = aws_cloudwatch_metric_alarm.scsbuster_error_alarm | ||
| id = "SCSBusterErrorAlarm" | ||
| } |
There was a problem hiding this comment.
Neat. I was not previously aware of import blocks. This seems maybe better than the import command since it documents that this was previously managed outside of terraform and how we imported it (because I always have to look up the right terraform import command).
|
I just saw that the original alarm (SCSBusterErrorAlarm) was deleted ~3 weeks ago but I have no recollection of doing so... I'll see if I can figure out how it happened but in the meantime, I removed the import block so that a new alarm (with updated name) will be created. I don't see any fatal error logs in Cloudwatch in the time without alarm coverage, but ideally this PR can be merged in soon to reduce further downtime |
danamansana
left a comment
There was a problem hiding this comment.
Lock files should be gitignored and not included in commit, otherwise looks good
Sets up Terraform and an alarm. Also makes some changes to the Dockerfile so that tests can run (issues with deprecations). I don't know much about Ruby/this setup so other changes may be needed
Update: the alarm was deleted somehow so I removed the import and updated the
terraform planoutputQuestions for reviewers:
SCSBusterErrorAlarm. In this PR, an new metricSCSBusterErrorand filter (identical except for name) are defined to keep with the current naming convention. Would using the original metric be preferred?terraform planoutput: