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

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

Open
1 of 2 tasks
blimmer opened this issue Mar 11, 2022 · 8 comments
Open
1 of 2 tasks

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

blimmer opened this issue Mar 11, 2022 · 8 comments
Labels
@aws-cdk/aws-logs Related to Amazon CloudWatch Logs effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1

Comments

@blimmer
Copy link
Contributor

blimmer commented Mar 11, 2022

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:

constructor(scope: Construct, id: string, props: LogGroupProps = {}) {
super(scope, id, {
physicalName: props.logGroupName,
});
let retentionInDays = props.retention;
if (retentionInDays === undefined) { retentionInDays = RetentionDays.TWO_YEARS; }
if (retentionInDays === Infinity || retentionInDays === RetentionDays.INFINITE) { retentionInDays = undefined; }
if (retentionInDays !== undefined && !Token.isUnresolved(retentionInDays) && retentionInDays <= 0) {
throw new Error(`retentionInDays must be positive, got ${retentionInDays}`);
}
const resource = new CfnLogGroup(this, 'Resource', {
kmsKeyId: props.encryptionKey?.keyArn,
logGroupName: this.physicalName,
retentionInDays,
});
resource.applyRemovalPolicy(props.removalPolicy);
this.logGroupArn = this.getResourceArnAttribute(resource.attrArn, {
service: 'logs',
resource: 'log-group',
resourceName: this.physicalName,
arnFormat: ArnFormat.COLON_RESOURCE_NAME,
});
this.logGroupName = this.getResourceNameAttribute(resource.ref);
}

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

  • I may be able to implement this feature request
  • This feature might incur a breaking change
@blimmer blimmer added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 11, 2022
@github-actions github-actions bot added the @aws-cdk/aws-logs Related to Amazon CloudWatch Logs label Mar 11, 2022
@gshpychka
Copy link
Contributor

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

@tessro
Copy link

tessro commented May 18, 2022

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

@peterwoodworth peterwoodworth added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jun 4, 2022
@comcalvi comcalvi removed their assignment Jun 20, 2022
@demiurg
Copy link

demiurg commented Sep 26, 2022

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
Copy link
Contributor

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.

@vinayak-kukreja vinayak-kukreja added p1.5 and removed p1 labels May 15, 2023
@otaviomacedo otaviomacedo added p2 and removed p1.5 labels May 22, 2023
@BwL1289
Copy link

BwL1289 commented Jun 14, 2023

Experiencing as well

@github-actions github-actions bot removed the p2 label Nov 12, 2023
Copy link

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

jrombi commented May 13, 2024

Experiencing as well

@zweger
Copy link

zweger commented Jun 21, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-logs Related to Amazon CloudWatch Logs effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

No branches or pull requests