Skip to content

Allow expiry of all logs#123

Open
elruwen wants to merge 1 commit into
DataDog:mainfrom
elruwen:main
Open

Allow expiry of all logs#123
elruwen wants to merge 1 commit into
DataDog:mainfrom
elruwen:main

Conversation

@elruwen
Copy link
Copy Markdown

@elruwen elruwen commented May 22, 2024

Before this change, the copy lambda creates a log group which never expires. When you manage lots of AWS accounts and you want to make sure that all cloudwatch log groups expire at some point, this can get very annoying.

I renamed the logical id of the function, so that a new function is created and therefore we get a chance to create the log group. Otherwise the stack would fail since there would be already an existing log group.

What does this PR do?

Expire the log group of the copy function.

Motivation

We centrally running analysis to make sure that all cloud watch log groups have an expiry set. This one always sticks out. That is annoying.

Testing Guidelines

I deployed the stack to avoid timing issues.

Additional Notes

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

Before this change, the copy lambda creates a log group which never
expires. When you manage lots of AWS accounts and you want to make sure
that all cloudwatch log groups expire at some point, this can get very
annoying.

I renamed the logical id of the function, so that a new function is
created and therefore we get a chance to create the log group. Otherwise
the stack would fail since there would be already an existing log group.
@elruwen elruwen requested a review from a team as a code owner May 22, 2024 01:32
@duncanista duncanista self-assigned this May 24, 2024
@duncanista duncanista added the enhancement New feature or request label May 24, 2024
@duncanista
Copy link
Copy Markdown
Contributor

Hey @elruwen, thanks for contributing. Sorry for taking so long.

I think if we were to do this change, without adding the V2 it would require us to do a breaking change. Let me get back to you on what our best option is, but I guess we'll just add your change without the suffix and make it breaking.

@duncanista
Copy link
Copy Markdown
Contributor

I think it would be ideal to have the policy as a conditional, if the log group already exists, then don't create it. That way we can ensure that there are no breaking changes, what do you think about this?

@elruwen
Copy link
Copy Markdown
Author

elruwen commented Jun 18, 2024

Sounds good, let me give that a try

@duncanista
Copy link
Copy Markdown
Contributor

Hey @elruwen,

Let me know if you get to an alternative, I tried but fail multiple times. I'm afraid we might have to end up adding a breaking change at the end 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants