feat(logs): configure custom subscription filter name

[tam0ri]

Currently, we can't set the subscription filter name as a prop for L2 SubscriptionFilter construct. This PR introduces the new prop filterName. This will let us set a specific name without requiring escape hatches.

Closes #26485

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

[kaizencc]

Couple comments @tam0ri. Thanks for taking this on!

[kaizencc]

need a default here

[tam0ri]

Thank you for your comment! Sorry, I don't understand your intention. What do you mean need a default? As I described in the below conversation, I think it's better that filterName is optional because this can still let CDK/CFn generate physical id (filter name) automatically when this prop isn't specified.

[kaizencc]

For optional properties, we supply a default value in the documentation so our users know what it means when they don't supply anything:

/**
 * The name of the subscription filter.
 *
 * @default - a generated name is created for you
 */
readonly filterName?: string;

[tam0ri]

Makes sense! I aligned with logGroupName and logStreamName.

aws-cdk/packages/aws-cdk-lib/aws-logs/lib/log-group.ts

Lines 381 to 386 in e53e8dc

	/**
	* Name of the log group.
	*
	* @default Automatically generated
	*/
	readonly logGroupName?: string;

aws-cdk/packages/aws-cdk-lib/aws-logs/lib/log-group.ts

Lines 502 to 509 in e53e8dc

	/**
	* The name of the log stream to create.
	*
	* The name must be unique within the log group.
	*
	* @default Automatically generated
	*/
	readonly logStreamName?: string;

[kaizencc]

wait, won't this change all subscription filters even if they don't supply props.filterName? Why is there a need to propagate the filterName to physicalName this way

[tam0ri]

Thank you for your comment!

---

won't this change all subscription filters even if they don't supply props.filterName?

In my understanding, if we don't specify filterName prop introduced by this PR, this.physicalName will be undefined and then filterName for L1 CfnSubscriptionFilter construct will be also undefined. As a result, FilterName porperty for AWS::Logs::SubscriptionFilter resource in generted CFn template will be omitted. I already tested with the sample app in my env.

---

Why is there a need to propagate the filterName to physicalName this way

I align this with the similar constructs such as LogGroup, LogStream or MetricFilter as below.

aws-cdk/packages/aws-cdk-lib/aws-logs/lib/log-group.ts

Lines 466 to 469 in 51b20f9

	constructor(scope: Construct, id: string, props: LogGroupProps = {}) {
	super(scope, id, {
	physicalName: props.logGroupName,
	});

aws-cdk/packages/aws-cdk-lib/aws-logs/lib/log-stream.ts

Lines 67 to 70 in 9a3399f

	constructor(scope: Construct, id: string, props: LogStreamProps) {
	super(scope, id, {
	physicalName: props.logStreamName,
	});

aws-cdk/packages/aws-cdk-lib/aws-logs/lib/metric-filter.ts

Lines 25 to 28 in 9a3399f

	constructor(scope: Construct, id: string, props: MetricFilterProps) {
	super(scope, id, {
	physicalName: props.filterName,
	});

From CFn perspective, if we specify FilterName property, this is used as physical id. Shouldn't we propagate the filterName to physicalName?

[kaizencc]

We can't, because that would be a breaking change for users who don't specify filterName. The behavior that existed before needs to be maintained, unfortunately.

In practice, if we really want to change the behavior we hide it behind a feature flag. But this ins't important enough to go down that route, imo.

[kaizencc]

Oh wait you're saying this.physicalName will be undefined if no filterName is given... hmm maybe thats the case but I would be surprised.

[kaizencc]

Nevermind, you are right :)

Pull request has been modified.

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 6e876f1
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).