feat(logs): add support for fieldIndexPolicies in log group L2 Construct by yashkh-amzn · Pull Request #33416 · aws/aws-cdk

yashkh-amzn · 2025-02-12T18:14:59Z

Issue # (if applicable)

Reason for this change

Field Indexing for CloudWatch Logs (CWL) was launched in Nov 2024. A lot of CWL customers are asking for indexing support in L2 construct. This feature will enable that property under FieldIndexPolicies as a JSON object in the LogGroup construct.

Description of changes

The change here is just populating the fieldIndexPolicies property of the LogGroup CFN with the list of fields provided by the user. The format of this property will be like this:

const fieldIndexPolicy = new FieldIndexPolicy({
  fields: ['Operation', 'RequestId'],
});

new LogGroup(this, 'LogGroupLambda', {
  dataProtectionPolicy: dataProtectionPolicy,
  fieldIndexPolicies: [fieldIndexPolicy],
});

Describe any new or updated permissions being added

No new permissions have been added.

Description of how you validated changes

Added unit tests. Will add integ tests after getting a confirmation from the CDK team on the implementation.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

aws-cdk-automation

(This review is outdated)

yashkh-amzn · 2025-02-12T18:42:04Z

Clarification Request

aaythapa · 2025-02-13T18:18:12Z

Clarification Request

Hi @yashkh-amzn thank you for contributing!

I see the PR is missing integ tests and read me changes. To learn more about our integ test process and how to make them you can refer to this doc.

As for README changes they will presumably go here.

Note that until the linter and build passes this PR won't show up on our radar to review. Let us know if there is anything else you need clarifying.

yashkh-amzn · 2025-02-13T18:56:30Z

Clarification Request

Hi @yashkh-amzn thank you for contributing!

I see the PR is missing integ tests and read me changes. To learn more about our integ test process and how to make them you can refer to this doc.

As for README changes they will presumably go here.

Note that until the linter and build passes this PR won't show up on our radar to review. Let us know if there is anything else you need clarifying.

Hey @aaythapa, I had a sync up with your team during the CDK office hours. I was told that the CDK team first wanted to align with the business logic and hence it was okay to submit a PR first for an initial review. Once there would be an alignment, I can update the integ tests and README.
cc: @kaizencc @shikha372 @gracelu0

aaythapa · 2025-02-13T22:04:51Z

Clarification Request

Hi @yashkh-amzn thank you for contributing!
I see the PR is missing integ tests and read me changes. To learn more about our integ test process and how to make them you can refer to this doc.
As for README changes they will presumably go here.
Note that until the linter and build passes this PR won't show up on our radar to review. Let us know if there is anything else you need clarifying.

Hey @aaythapa, I had a sync up with your team during the CDK office hours. I was told that the CDK team first wanted to align with the business logic and hence it was okay to submit a PR first for an initial review. Once there would be an alignment, I can update the integ tests and README. cc: @kaizencc @shikha372 @gracelu0

Ah ok, thanks for letting me know. I'll bring this up with the team and wait for a response

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

shikha372 · 2025-02-18T02:30:52Z

packages/aws-cdk-lib/aws-logs/test/loggroup.test.ts

+  });
+});
+
+test('set field index policy positive test', () => {


what is the difference between this test and one above

Good catch! This was a dedup. I'll remove this test case.

packages/aws-cdk-lib/aws-logs/lib/field-index-policy.ts

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

codecov · 2025-02-18T15:03:53Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.21%. Comparing base (4bc151b) to head (11ffd90).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #33416   +/-   ##
=======================================
  Coverage   82.21%   82.21%           
=======================================
  Files         119      119           
  Lines        6876     6876           
  Branches     1162     1162           
=======================================
  Hits         5653     5653           
  Misses       1120     1120           
  Partials      103      103

Flag	Coverage Δ
suite.unit	`82.21% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`∅ <ø> (∅)`
packages/aws-cdk-lib/core	`82.21% <ø> (ø)`

aws-cdk-automation · 2025-02-20T00:11:50Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

aws-cdk-automation · 2025-02-21T00:11:40Z

This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.

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

GavinZZ · 2025-02-25T20:52:32Z

Removing the reviewer-clarification-requested label as it's been clarified that README and Integ test will be added once an initial review was done by the team and the contributor has added them now.

shikha372 · 2025-03-03T22:46:10Z

packages/aws-cdk-lib/aws-logs/README.md

+import * as kinesisfirehose from '@aws-cdk/aws-kinesisfirehose';
+
+
+const logGroupDestination = new LogGroup(this, 'LogGroupLambdaAudit', {


for my understanding, could you please explain the purpose of defining log group destination and s3 destination in this code reference as i don't see it being used later while defining fieldIndexPolicy.

This wasn't needed. Fixed it by removing all the unnecessary references.

Thanks @yashkh-amzn, for field index policies you'll need to add the import in rosetta file,
https://github.com/yashkh-amzn/aws-cdk/blob/feature/loggroup-indexing/packages/aws-cdk-lib/rosetta/aws_logs/default.ts-fixture or modify it as logs.LogGroup

Yes, I'm using logs.LogGroup. Take a look at the latest commit

mergify · 2025-03-05T01:12:55Z

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).

aws-cdk-automation · 2025-03-05T02:05:49Z

AWS CodeBuild CI Report

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

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

mergify · 2025-03-05T02:06:11Z

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).

github-actions · 2025-03-05T02:06:28Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

github-actions bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 labels Feb 12, 2025

aws-cdk-automation requested a review from a team February 12, 2025 18:15

aws-cdk-automation previously requested changes Feb 12, 2025

View reviewed changes

aws-cdk-automation added pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Feb 12, 2025

yashkh-amzn changed the title ~~feat(aws-logs): Add support for fieldIndexPolicies in log group L2 Construct~~ feat(logs): add support for fieldIndexPolicies in log group L2 Construct Feb 12, 2025

aaythapa removed the pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run label Feb 13, 2025

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Feb 13, 2025

aws-cdk-automation added pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Feb 13, 2025

shikha372 reviewed Feb 18, 2025

View reviewed changes

packages/aws-cdk-lib/aws-logs/lib/log-group.ts Outdated Show resolved Hide resolved

shikha372 reviewed Feb 18, 2025

View reviewed changes

packages/aws-cdk-lib/aws-logs/lib/field-index-policy.ts Show resolved Hide resolved

shikha372 reviewed Feb 18, 2025

View reviewed changes

packages/aws-cdk-lib/aws-logs/lib/log-group.ts Show resolved Hide resolved

yashkh-amzn requested a review from a team as a code owner February 21, 2025 03:59

yashkh-amzn force-pushed the feature/loggroup-indexing branch from 1321c9d to 8fc94cc Compare February 21, 2025 05:08

GavinZZ removed the pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run label Feb 25, 2025

github-actions bot mentioned this pull request Mar 1, 2025

Monthly PR metrics report #33661

Closed

godwingrs22 assigned QuantumNeuralCoder Mar 3, 2025

shikha372 assigned shikha372 and unassigned QuantumNeuralCoder Mar 3, 2025

shikha372 reviewed Mar 3, 2025

View reviewed changes

yashkh-amzn added 3 commits March 3, 2025 22:49

Add support for fieldIndexPolicies in log group L2 Construct

927a4f2

1. Add README & Integ tests 2. Address comments

355b5d2

Update README to remove unused references

9f5ed65

yashkh-amzn force-pushed the feature/loggroup-indexing branch from 5a5a45d to 9f5ed65 Compare March 4, 2025 21:17

yashkh-amzn and others added 2 commits March 4, 2025 13:22

Remove unused imports from README section

50aa572

Merge branch 'main' into feature/loggroup-indexing

11ffd90

shikha372 approved these changes Mar 5, 2025

View reviewed changes

Merge branch 'main' into feature/loggroup-indexing

b19c32c

mergify bot merged commit 6c882e0 into aws:main Mar 5, 2025
10 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Mar 5, 2025

		import * as kinesisfirehose from '@aws-cdk/aws-kinesisfirehose';


		const logGroupDestination = new LogGroup(this, 'LogGroupLambdaAudit', {

Conversation

yashkh-amzn commented Feb 12, 2025

Issue # (if applicable)

Reason for this change

Description of changes

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

Uh oh!

aws-cdk-automation left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yashkh-amzn commented Feb 12, 2025

Uh oh!

aaythapa commented Feb 13, 2025

Uh oh!

yashkh-amzn commented Feb 13, 2025

Uh oh!

aaythapa commented Feb 13, 2025

Uh oh!

Uh oh!

shikha372 Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

yashkh-amzn Feb 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aws-cdk-automation commented Feb 20, 2025

Uh oh!

aws-cdk-automation commented Feb 21, 2025

Uh oh!

GavinZZ commented Feb 25, 2025

Uh oh!

shikha372 Mar 3, 2025

Choose a reason for hiding this comment

Uh oh!

yashkh-amzn Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

shikha372 Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yashkh-amzn Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Mar 5, 2025

Uh oh!

aws-cdk-automation commented Mar 5, 2025

AWS CodeBuild CI Report

Uh oh!

mergify bot commented Mar 5, 2025

Uh oh!

Uh oh!

github-actions bot commented Mar 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

aws-cdk-automation left a comment •

edited

Loading

codecov bot commented Feb 18, 2025 •

edited

Loading

shikha372 Mar 4, 2025 •

edited

Loading