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

feat(logs): support regex patterns for JSON Metrics filters #30741

Merged
merged 16 commits into from
Feb 14, 2025

Conversation

jan-xyz
Copy link
Contributor

@jan-xyz jan-xyz commented Jul 3, 2024

Issue # (if applicable)

Closes #30451

Reason for this change

Support Regex in filter functions for JSON

Description of changes

Adding a new JSONPattern factory that uses % instead of " to support the regex pattern.

Description of how you validated changes

I tried it in our own CDK code.

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 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Jul 3, 2024
@aws-cdk-automation aws-cdk-automation requested a review from a team July 3, 2024 08:15
@jan-xyz jan-xyz changed the title Support regex patterns Support regex patterns for JSON Metrics filters Jul 3, 2024
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)

@jan-xyz jan-xyz changed the title Support regex patterns for JSON Metrics filters feat(logs): Support regex patterns for JSON Metrics filters Jul 3, 2024
@jan-xyz jan-xyz changed the title feat(logs): Support regex patterns for JSON Metrics filters feat(logs): support regex patterns for JSON Metrics filters Jul 3, 2024
@jan-xyz
Copy link
Contributor Author

jan-xyz commented Jul 3, 2024

Clarification Request: How should the snapshot look like? It seems that this package doesn't have any yet? According to INTEGRATION_TEST.md there should be a integ.mtericfilter.js.snapshot/ directory but I couldn't find it.

@aws-cdk-automation aws-cdk-automation added the pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run label Jul 3, 2024
@aws-cdk-automation
Copy link
Collaborator

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

@jan-xyz
Copy link
Contributor Author

jan-xyz commented Jul 25, 2024

I had a question how to do the integration test, and I am still waiting on the response

@aws-cdk-automation
Copy link
Collaborator

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

@aws-cdk-automation aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Aug 1, 2024
Copy link

github-actions bot commented Aug 1, 2024

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 Aug 1, 2024
@paulhcsun
Copy link
Contributor

paulhcsun commented Aug 7, 2024

Hi @jan-xyz, apologies for missing this clarification request. The integration tests for modules in aws-cdk-lib are located in the aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-logs/test directory: https://github.com/aws/aws-cdk/tree/main/packages/%40aws-cdk-testing/framework-integ/test/aws-logs/test

Please refer to this section of our INTEGRATION_TESTS.md.

@paulhcsun paulhcsun reopened this Aug 7, 2024
@paulhcsun paulhcsun added pr-linter/do-not-close The PR linter will not close this PR while this label is present and removed closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. pr/reviewer-clarification-requested The contributor has requested clarification on feedback, a failing build, or a failing PR Linter run labels Aug 7, 2024
@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Feb 11, 2025
@aws aws unlocked this conversation Feb 12, 2025
Copy link
Contributor

@paulhcsun paulhcsun left a comment

Choose a reason for hiding this comment

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

@jan-xyz Thanks for the contribution and for getting back to this PR! I've left a couple comments about some refactoring that will be needed since unions are not supported in public APIs/methods in the CDK. I will pass this review off to another team member to review once the changes are made.

@aaythapa aaythapa self-assigned this Feb 13, 2025
@mergify mergify bot dismissed paulhcsun’s stale review February 13, 2025 17:00

Pull request has been modified.

@jan-xyz jan-xyz requested a review from paulhcsun February 13, 2025 17:03
Copy link
Contributor

@aaythapa aaythapa 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 contributing! Small comment

aaythapa
aaythapa previously approved these changes Feb 13, 2025
Copy link
Contributor

@aaythapa aaythapa left a comment

Choose a reason for hiding this comment

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

thank you for the contribution

Copy link
Contributor

mergify bot commented Feb 13, 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).

@aws-cdk-automation 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
Copy link
Contributor

mergify bot commented Feb 13, 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).

@paulhcsun
Copy link
Contributor

@Mergifyio update

Copy link
Contributor

mergify bot commented Feb 13, 2025

update

✅ Branch has been successfully updated

Copy link

codecov bot commented Feb 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.00%. Comparing base (39151d0) to head (e2c766f).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #30741   +/-   ##
=======================================
  Coverage   81.00%   81.00%           
=======================================
  Files         238      238           
  Lines       14271    14271           
  Branches     2492     2492           
=======================================
  Hits        11560    11560           
  Misses       2425     2425           
  Partials      286      286           
Flag Coverage Δ
suite.unit 81.00% <ø> (ø)

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

Components Coverage Δ
packages/aws-cdk 79.92% <ø> (ø)
packages/aws-cdk-lib/core 82.16% <ø> (ø)

Copy link
Contributor

mergify bot commented Feb 13, 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).

Copy link
Contributor

mergify bot commented Feb 13, 2025

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

@aaythapa
Copy link
Contributor

@mergify update

Copy link
Contributor

mergify bot commented Feb 14, 2025

update

☑️ Nothing to do

  • #commits-behind > 0 [📌 update requirement]
  • -closed [📌 update requirement]
  • -conflict [📌 update requirement]
  • queue-position = -1 [📌 update requirement]

@mergify mergify bot dismissed aaythapa’s stale review February 14, 2025 00:46

Pull request has been modified.

Copy link
Contributor

mergify bot commented Feb 14, 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).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

Copy link
Contributor

mergify bot commented Feb 14, 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 c89afe3 into aws:main Feb 14, 2025
14 checks passed
Copy link

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 Feb 14, 2025
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 effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 pr-linter/do-not-close The PR linter will not close this PR while this label is present
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws-logs: Allow JSON filter patterns to use regex by removing automatic double quotes around pattern strings
4 participants