PLT-1773 : Implement core log-retention service - #615
jasonvinson wants to merge 3 commits into
Conversation
| @@ -0,0 +1,283 @@ | |||
| locals { | |||
| firehose_name = "${var.app}-${var.env}-long-term-log-retention" | |||
There was a problem hiding this comment.
This makes sense because our pattern is cdap-test-* for various shared services. Do you think, though, there's a better name that would mean this doesn't get confused for being just cdap related logs? This should be all service logs
There was a problem hiding this comment.
I added a new "shared" naming convention here. Wondering if "common" would fit our patterns better...but I felt "shared" was best for clarity. Would love your thoughts on this!
| policy = data.aws_iam_policy_document.log_retention_kms.json | ||
| } | ||
|
|
||
| resource "aws_kms_alias" "log_retention" { |
There was a problem hiding this comment.
This alias implies that it's not a CDAP specific key, so per above comment, match the naming or the sorting convention. If we keep this all under "CDAP" as it's a platform service, then this should probably use the cdap-$env key per convention, and not get its own dedicated key. This would mean that when we have automated rotation with that key, we get this "for free" and the logs just start rotating.
There was a problem hiding this comment.
Updated to cdap-$env-log-retention
| @@ -0,0 +1 @@ | |||
| TARGET_ENVS="cdap-test cdap-prod" | |||
There was a problem hiding this comment.
Lets make sure we firm up this pipeline fully in cdap-test before provisioning to prod with compliance mode to 6 years. Leave just as "cdap-test" until the work is complete and evaluated in cdap-test
There was a problem hiding this comment.
It might make sense, for testing and development purposes, to set up a config/.yml with variables here
There was a problem hiding this comment.
Agreed! I removed references to prod for now.
| statement { | ||
| sid = "DeliveryErrorLogging" | ||
| actions = ["logs:PutLogEvents"] | ||
| resources = ["${aws_cloudwatch_log_group.firehose.arn}:log-stream:*"] |
There was a problem hiding this comment.
Does this log group have to be individually created or can we leverage the cloudwatch log group module, noting that... those logs themselves will end up in this firehose.
There was a problem hiding this comment.
Updated to use the cloudwatch log group module.
| alarm_name = "${local.firehose_name}-s3-delivery-failure" | ||
| alarm_description = "Firehose is failing to deliver log records to the long-term retention bucket" | ||
| namespace = "AWS/Firehose" | ||
| metric_name = "DeliveryToS3.Success" | ||
| statistic = "Average" | ||
| period = 300 | ||
| evaluation_periods = 3 | ||
| threshold = 1 |
There was a problem hiding this comment.
This likely makes sense, though I'm curious if there's something in DataDog we could leverage instead.
- new shared naming convention for resources shared by multiple apps - use modules/cloudwatch_log_group - remove var.app as this pipeline is cdap managed only - remove references to prod. We want to firm this up as much as possible in test first - add cdap-test-log-retention KMS key references to 900-github-actions-role/config/cdap yml files
2 participants
🎫 Ticket
https://jira.cms.gov/browse/PLT-1773
🛠 Changes
ℹ️ Context
We currently run a script periodically to enforce long-term log retention. We want to support this HIPAA requirement natively so that all resources created through Tofu comply in a standardized fashion.
🧪 Validation
Plans successfully targeting test. Will validate further once deployed to test env.