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 expiry of all logs #123

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Allow expiry of all logs #123

wants to merge 1 commit into from

Conversation

elruwen
Copy link

@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
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
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
Author

elruwen commented Jun 18, 2024

Sounds good, let me give that a try

@duncanista
Copy link
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