Skip to content

feat: add LogGroup property to AWS::Serverless::Function for stack-managed CloudWatch log groups#3957

Open
GuidoNebiolo wants to merge 5 commits into
aws:developfrom
GuidoNebiolo:feat/function-log-group
Open

feat: add LogGroup property to AWS::Serverless::Function for stack-managed CloudWatch log groups#3957
GuidoNebiolo wants to merge 5 commits into
aws:developfrom
GuidoNebiolo:feat/function-log-group

Conversation

@GuidoNebiolo

Copy link
Copy Markdown

Issue #, if available

Refs #2665, #3791, #1216, #2057

Description of changes

Adds a new LogGroup property to AWS::Serverless::Function that generates a stack-managed AWS::Logs::LogGroup, so the log group is created/updated/deleted together with the stack and its retention is configurable.

Two naming mechanisms:

  • Default (no LogGroupName): the group uses the conventional /aws/lambda/<function-name> name via Fn::Sub. LoggingConfig is left untouched and no extra IAM is needed — the name coincides with Lambda's default destination and AWSLambdaBasicExecutionRole already grants log writes.
  • Custom name (LogGroupName): binds the function via its native LoggingConfig.LogGroup, merging with any existing LoggingConfig.

Supported sub-properties: RetentionInDays (pass-through), DeletionPolicy (Delete default, or Retain) applied to both DeletionPolicy and UpdateReplacePolicy, and LogGroupName. Inheritable via Globals. When the property is absent, the transform output is unchanged (backward compatible).

This makes retention configurable and deletes the log group with the stack, addressing orphaned log groups (#1216).

Description of how you validated changes

  • Added transform tests across all partitions (aws, aws-cn, aws-us-gov): default name, custom name with LoggingConfig wiring, DeletionPolicy: Retain, Condition propagation, Globals inheritance/override, and an invalid DeletionPolicy error case.
  • Added unit tests for the LogGroup resource model and _construct_log_group.
  • make pr is green: ruff, mypy --strict, format-check (schema regenerated), full test suite passing with 95.64% coverage.

AI usage disclosure

This contribution was developed with the assistance of a generative AI coding tool. All code and tests were reviewed and are fully understood by the author, who is able to answer maintainer questions during review. The generated code was held to the same quality standards as hand-written code, and the diff/tests were kept minimal to ease review.

Checklist

  • Review the generative AI contribution guidelines
  • Adheres to the development guidelines
  • Add/update transform tests
    • Using correct values
    • Using wrong values
  • Add/update integration tests

Add a LogGroup resource model backing the AWS::Logs::LogGroup CloudFormation type, with runtime attributes for Ref (name) and Fn::GetAtt Arn. This is the building block for a stack-managed CloudWatch log group on AWS::Serverless::Function.

Refs aws#2665, aws#3791, aws#1216, aws#2057
…oup property

Add a LogGroup property to AWS::Serverless::Function that generates a stack-managed AWS::Logs::LogGroup. By default the group uses the conventional /aws/lambda/<function-name> name (no LoggingConfig or IAM change needed, since the group coincides with Lambda's default destination and AWSLambdaBasicExecutionRole already grants log writes). A custom LogGroupName binds the function via its native LoggingConfig.LogGroup.

RetentionInDays is passed through and DeletionPolicy (Delete by default, or Retain) is applied to both DeletionPolicy and UpdateReplacePolicy so the group is deleted with the stack, avoiding orphaned log groups. When absent, output is unchanged for backward compatibility.

Refs aws#2665, aws#3791, aws#1216, aws#2057
Add the FunctionLogGroup model (RetentionInDays, DeletionPolicy, LogGroupName) to the Function Properties and Globals schema, document the new properties in sam-docs, and regenerate the SAM and unified JSON schemas.

Refs aws#2665, aws#3791
Add LogGroup to the Function properties supported in the Globals section so a common log group configuration (for example retention or deletion policy) can be defined once and inherited by all functions, with resource-level overrides merged on top.

Refs aws#2665, aws#3791
Cover the default log group name (Mechanism 1), custom log group name with LoggingConfig wiring (Mechanism 2), DeletionPolicy Retain, Condition propagation to the log group, Globals inheritance/override, and the invalid DeletionPolicy error case, across all partitions.

Refs aws#2665, aws#3791, aws#1216, aws#2057
@valerena

Copy link
Copy Markdown
Contributor

I don't see any check or guardrails in case a user sets LoggingConfig and this new LogGroup at the same time. What happens if the log group names are different?

My initial instinct would be to prefer adding the log group name and other information as part of LoggingConfig, something like:

MyFunction:
  Properties:
    ..
    LoggingConfig:
       CreateLogGroup: true
       LogGroup: "..."
       AdditionalProperties:
         DeletionPolicy: ...
         ...

Although it is a little more verbose, but it would prevent users from having the log group information separate in two different places.

There's also the option of just accomplishing the same by just manually defining the LogGroup outside of the function directly.

MyFunction:
  Properties:
    ..
    LoggingConfig:
       LogGroup: !Ref MyLogGroup

MyLogGroup:
  Type: AWS::Logs::LogGroup
    Properties:
      LogGroupName: ...
      RetentionInDays: ...

which is fairly straightforward.

The only case where this is not that simple is when you want to continue using Lambda's default name /aws/lambda/<function-name>, where the log group would depend on the function name, so the function would be created first and you can't just make both resources depend on each other.

The other issue when you want to keep using Lambda's default name (in your solution too, and it's mentioned in a few of the linked GitHub issues) is the fact that if for some reason the function gets invoked before the log group gets created, then Lambda will still create the log group, and the CFN deployment would fail when it tries to create the log group.
This is more noticeable if you want to "add the LogGroup property to an existing function". In this case, the function likely already has an existing log group, so the log group can't be created via CFN because it already exist (you would have to change the name of the log group, or "import the log group" to the stack, or delete the previous log group so CFN can recreate it).

The point is that there are some edge cases that makes this not "as simple" as adding a new property. We're very interested in making a good solution for customers, but we don't want to have a confusing experience that can make it harder for new customers. Let me know your opinions about the above issues, and we'll continue the discussion internally with the team too.

@GuidoNebiolo

Copy link
Copy Markdown
Author

Thanks @valerena , these are really helpful points. Going through them:

  1. No guardrail when both LoggingConfig and LogGroup are set. You're right, this is a gap. Today a custom LogGroupName silently overwrites LoggingConfig.LogGroup, and with the default name I leave LoggingConfig untouched (so a user-set LoggingConfig.LogGroup would point elsewhere and the managed group would receive no logs). I'll add validation that raises a clear error when both define the destination, so it's never set in two places.

  2. Extending LoggingConfig vs a dedicated property. I chose a dedicated property mainly because LoggingConfig is a pure pass-through to AWS::Lambda::Function, and guideline How to setup body transformation? #3 (stay close to the underlying CFN properties) pushes us to keep pass-throughs 1:1 with CloudFormation. Injecting SAM-only keys like CreateLogGroup/AdditionalProperties would force SAM to parse and strip them before passing through, and could collide if Lambda later adds real fields there. That said, the "info split in two places" concern is valid, I'm happy to go the LoggingConfig route if the team prefers that trade-off. Just wanted to flag the pass-through implication.

  3. You can already define the log group manually. Agreed, and I don't think we should reinvent the !Ref MyLogGroup path. The case it doesn't cover cleanly is keeping Lambda's default /aws/lambda/<function-name> name while managing the group in the stack (the dependency on the possibly auto-generated function name makes it awkward). That's the case the linked issues (Feature: Create CloudWatch Log groups for Functions #1216, Lambda Logs definition in template #2665) are about, so I can scope the feature primarily around it.

  4. "Already exists" on first deploy / retrofit. Agreed, this is inherent to managing the default-named group in any approach, and mostly affects retrofitting a function that already has a log group. I don't think the transform can solve it, but I can document the behavior and mitigations (rename, resource import, or delete the orphan once). New functions aren't affected.

Proposal: add the conflict validation from (1), lean the feature toward the default-name use case from (3), and document (4). On (2) I'm open, do you prefer extending LoggingConfig or a dedicated property?

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.

2 participants