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

Allow the extension layer to be applied separately from the runtime layers #131

Merged
merged 6 commits into from
Sep 17, 2024

Conversation

TophrC-dd
Copy link
Contributor

What does this PR do?

Currently there is no way to deploy with the cloudformation macro and only apply the extension.
This PR adds this ability when the user sets addLayers to false the extension will still be applied.
To override this feature a new configuration option has been made called 'addExtension. This defaults to true` ensuring the extension layer is applied.

Motivation

Currently if a customer is providing their own lambda libraries (dd-trace, datadog-lambda-js) there is no way to exclude these layers from the deployment and still retain the extension layer. This adds that functionality for this edge case.

Testing Guidelines

In addition to the unit tests i used the release tool to upload my changes to a staging bucket that hosted the macro. Which i deployed to a sandbox aws environment and then created a new function using the macro. I had set addLayers to false along with leaving addExtension to default. This produced a lambda with just the extension layer.

I then modified my template.yaml file to set addExtension to false this resulted in no lambda's being added.

Additional Notes

I did not increment the version number in the template yaml file.

One thing to consider is that users that have addLayers to false today and redeploy the macro will see a change in the expected behavior.

Types of changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog

@TophrC-dd TophrC-dd requested a review from a team as a code owner September 12, 2024 20:41
export function applyExtensionLayer(region: string, lambdas: LambdaFunction[], extensionLayerVersion?: number) {
const errors: string[] = [];
lambdas.forEach((lambda) => {
let lambdaExtensionLayerArn;

if (extensionLayerVersion !== undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like we should do here similarly to what we do above. If the extension version is not set, return an error. Otherwise, I think it will be real confusing for people.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added this functionality and tested it.

Copy link
Contributor

@purple4reina purple4reina left a comment

Choose a reason for hiding this comment

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

let's add an error if they dont set extension version. otherwise looks great!

We should also think about... does this change require a major version bump? I think the answer is yes.... but let's figure that out before we merge/release.

@purple4reina purple4reina merged commit bb5797e into DataDog:main Sep 17, 2024
6 checks passed
@sawilde
Copy link

sawilde commented Sep 19, 2024

extensionLayerVersion has always been optional as you don't need it if you are pushing your logs to cloudwatch and then using the cloudwatch forwarder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants