Skip to content

Conversation

@antoinedube
Copy link
Contributor

Issue # (if applicable)

Not related to any issue.

Reason for this change

It was not possible to pass the argument 'applyOnTransformedLogs' to the class MetricFilter, in order to create MetricFilter, Filter and Alarm.

Description of changes

Since MetricFilter makes use of CfnMetricFilter, which allows the parameter 'applyOnTransformedLogs', this change adds the parameter 'applyOnTransformedLogs' to MetricFilterOptions, which is directly passed to the CfnMetric class.

Describe any new or updated permissions being added

No permission is added or updated.

Description of how you validated changes

I tested by hand.

Checklist


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

@github-actions github-actions bot added the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label Aug 29, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team August 29, 2025 00:44
@github-actions github-actions bot added the p2 label Aug 29, 2025
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This review is outdated)

@antoinedube antoinedube changed the title Aws logs add parameter to allow metric filter on transformed logs feat:aws-logs add parameter to allow metric filter on transformed logs Aug 29, 2025
@antoinedube antoinedube changed the title feat:aws-logs add parameter to allow metric filter on transformed logs fix:aws-logs add parameter to allow metric filter on transformed logs Aug 29, 2025
@antoinedube
Copy link
Contributor Author

Exemption Request

This PR does not change the existing behavior of the class; it only adds a parameter in MetricFilter, when the parameter exists in CfnMetricFilter.

@aws-cdk-automation aws-cdk-automation added the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Aug 29, 2025
@iankhou
Copy link
Contributor

iankhou commented Aug 29, 2025

Exemption Request

This PR does not change the existing behavior of the class; it only adds a parameter in MetricFilter, when the parameter exists in CfnMetricFilter.

It changes the MetricFilter class. Please add unit and integ tests for this change. We want to ensure that when this param is passed through, the change is reflected in the CFn output.

This is also not really a fix, I would consider it a feat.

Copy link
Contributor

@iankhou iankhou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment on the PR.

@iankhou iankhou self-assigned this Aug 29, 2025
@antoinedube antoinedube changed the title fix:aws-logs add parameter to allow metric filter on transformed logs feat:aws-logs add parameter to allow metric filter on transformed logs Sep 1, 2025
@antoinedube
Copy link
Contributor Author

Thanks for your comment. I will add the tests.

@antoinedube antoinedube changed the title feat:aws-logs add parameter to allow metric filter on transformed logs feat(logs): add parameter to allow metric filter on transformed logs Sep 2, 2025
@aws-cdk-automation aws-cdk-automation dismissed their stale review September 2, 2025 18:40

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

@antoinedube
Copy link
Contributor Author

I guess the 'exemption-request' label can be removed.

@mergify mergify bot dismissed iankhou’s stale review September 5, 2025 21:32

Pull request has been modified.

@iankhou iankhou removed the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Oct 8, 2025
iankhou
iankhou previously approved these changes Oct 8, 2025
Copy link
Contributor

@iankhou iankhou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes. This got lost as you did not re-request review, but looks good after adding the tests.

@iankhou iankhou had a problem deploying to deployment-integ-test October 8, 2025 18:46 — with GitHub Actions Failure
@mergify mergify bot dismissed iankhou’s stale review October 8, 2025 18:46

Pull request has been modified.

@iankhou iankhou requested review from a team and iankhou and removed request for a team October 8, 2025 18:58
@iankhou iankhou had a problem deploying to deployment-integ-test October 8, 2025 19:13 — with GitHub Actions Failure
@mergify
Copy link
Contributor

mergify bot commented Oct 8, 2025

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

@mergify mergify bot added the queued label Oct 8, 2025
@mergify
Copy link
Contributor

mergify bot commented Oct 8, 2025

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

@mergify mergify bot merged commit effa46d into aws:main Oct 8, 2025
30 of 32 checks passed
@mergify mergify bot removed the queued label Oct 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2025

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 github-actions bot locked as resolved and limited conversation to collaborators Oct 8, 2025
@iankhou iankhou had a problem deploying to deployment-integ-test October 8, 2025 20:14 — with GitHub Actions Failure
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK p2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants