aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.39k stars 3.79k forks source link

(aws-logs): allow prefixing auto-generated log group names #19353

Open blimmer opened 2 years ago

blimmer commented 2 years ago

Description

It would be nice to retain the flexibility of the auto-generated log group names provided by CDK & CloudFormation while also being able to apply a prefix to work around certain restrictions, as described here.

Use Case

I've got a client who uses AWS Step Functions heavily. They ran into the resource policy size issue described here, so they needed to prefix their log groups with /aws/vendedlogs/ to work around the problem.

So, they're now providing an explicit log group name, like this:

new logs.LogGroup(this, 'LogGroup', {
  logGroupName: '/aws/vendedlogs/my-log-group-name'
});

However, like with most other constructs, it would be preferable to have the entropy of auto-generated names, instead of hard-coding a static name.

For example, this simple stack produces a log group with the name CdkTestingStack-LogGroupF5B46931-PEAj1XoIi1Q5:

export class CdkTestingStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    new logs.LogGroup(this, 'LogGroup')
  }
}

What we'd like to accomplish is easily producing a log group with the name /aws/vendedlogs/CdkTestingStack-LogGroupF5B46931-PEAj1XoIi1Q5. This way, we work around the resource policy limitation while retaining the auto-generated name.

Proposed Solution

An easy backward-compatible way to introduce this behavior would be to add an optional logGroupNamePrefix parameter to the L2 LogGroup construct.

For example:

export class CdkTestingStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    new logs.LogGroup(this, 'LogGroup', {
      logGroupNamePrefix: '/aws/vendedlogs/'
    })
  }
}

would provide a log group with the name /aws/vendedlogs/CdkTestingStack-LogGroupF5B46931-PEAj1XoIi1Q5.

It could also work with statically provided names, such as:

export class CdkTestingStack extends cdk.Stack {
  constructor(scope: cdk.Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    new logs.LogGroup(this, 'LogGroup', {
      logGroupName: 'MyLogGroup',
      logGroupNamePrefix: '/aws/vendedlogs/'
    })
  }
}

which would provide a log group with the name /aws/vendedlogs/MyLogGroup.

Other information

Possible Workarounds

L1 LogGroup Construct Modification

I considered reaching into the CfnLogGroup and using an Fn::Join to get this behavior without a CDK change. However, I think I'd also need to adjust the arn property, since it's set in the constructor as well: https://github.com/aws/aws-cdk/blob/d91b2e2259f9c1615aba2cb76ad4ab1fba945836/packages/@aws-cdk/aws-logs/lib/log-group.ts#L392-L420

That seemed a bit too fragile to be worthwhile.

Using the Physical Name Generator

I thought about using the generatePhysicalName method to recreate the autogenerated name. However, this seemed unsafe to do, since the function lives in packages/@aws-cdk/core/lib/private/physical-name-generator.ts. The private specifier makes me think twice about using it.

L2 StateMachine Construct

Since creating a StateMachine through the console automatically prefixes log groups with /aws/vendedlogs, it could be nice to also add this behavior to the L2 construct once the ability to provide a prefix is added.

Acknowledge

gshpychka commented 2 years ago

Probably makes sense to expand this feature request to all physical names.

tessro commented 2 years ago

We just ran into this too, as Datadog requires the /aws/vendedlogs/states prefix to vacuum Step Function logs.

demiurg commented 1 year ago

Just ran into the issue of CDK appending to the single resource log policy that it is re-using for all CDK stags in the region, and it ran out of max size, but did add /aws/vendedlogs/ logs. However, I could not update the code quickly enough and incurred downtime until I figured out how to delete certain resources from the policy and issue a 'Put'. Going through code and updating log groups for resources will take time, this issue would help.

biffgaut commented 1 year ago

We found a solution to generate safe physical names for a Log Group (with the /aws/vendedlogs/ prefix. You can read out our solution here.

BwL1289 commented 1 year ago

Experiencing as well

github-actions[bot] commented 8 months ago

This issue has received a significant amount of attention so we are automatically upgrading its priority. A member of the community will see the re-prioritization and provide an update on the issue.

jrombi commented 2 months ago

Experiencing as well

zweger commented 3 weeks ago

We ran into this issue as well with AWS WAF logs, which require the LogGroup name to be prefixed with aws-waf-logs-.