-
Notifications
You must be signed in to change notification settings - Fork 3
Create Terraform module for Datadog log forwarder lambda #1
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
Create Terraform module for Datadog log forwarder lambda #1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the complexity of the module, it can worth having some terraform test to ensure that minimal or usual use cases are covered as expected
] : [] | ||
]) | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 thought: It can be convenient to isolate/separate IAM resources from the rest to sub-modules, and keep it transparent for the regular user.
Given my experience with Cloud account vending machine, we might not be the only ones to isolate IAM stuff (due to their sensitivity) and having a more granular option can help us to use the provider internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good suggestion! moved IAM resources into modules/iam
and made it optional 👍
main.tf
Outdated
var.dd_step_functions_trace_enabled ? { DD_STEP_FUNCTIONS_TRACE_ENABLED = tostring(var.dd_step_functions_trace_enabled) } : {}, | ||
var.dd_use_compression == false ? { DD_USE_COMPRESSION = tostring(var.dd_use_compression) } : {}, | ||
var.dd_compression_level != 6 ? { DD_COMPRESSION_LEVEL = tostring(var.dd_compression_level) } : {}, | ||
var.dd_max_workers != 20 ? { DD_MAX_WORKERS = tostring(var.dd_max_workers) } : {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❓ question: Is there a way to not hardcode theses values in this giant map? By using a locals
for example and use it both here and in the default value section.
I'm afraid of the moment we'll change the default value and forgot to change this tiny line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, this was Claude trying to mirror what the Cloudformation template is doing 😅 thanks for catching that!
I've updated this to make all the optional env vars null
by default, meaning they are unset, so they fall back to the defaults set in code. This makes it so that we only have to maintain the default values in one place (https://github.com/DataDog/datadog-serverless-functions/blob/master/aws/logs_monitoring/settings.py)
…nset env vars (defaults set in code)
043101c
to
00092ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is here on purpose :) it's needed for the lambda source code ref when using a layer https://github.com/DataDog/terraform-aws-log-lambda-forwarder-datadog/pull/1/files
31a55c4
to
1473d9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, some nits
Follow ups discussed IRL:
- it would be useful to get the s3 bucket into a module as well.
AWSCORE-345
Based on the config options available in the CloudFormation template: https://github.com/DataDog/datadog-serverless-functions/blob/master/aws/logs_monitoring/template.yaml
Removed support for the following deprecated features:
DD_USE_TCP
- this is marked deprecated in the codeDD_USE_PRIVATE_LINK
- marked deprecated in the codesouce_code_version
/ the option to makeinstall_as_layer=false
- the module only supports installing the function code using the recommended method withDatadog-Forwarder
lambda layer. The layer version can be specified vialayer_version
var. Custom source code version or link is not supported.