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

Add AWS regexp pattern #62

Merged
merged 14 commits into from
Dec 30, 2023
Merged

Add AWS regexp pattern #62

merged 14 commits into from
Dec 30, 2023

Conversation

flysen
Copy link

@flysen flysen commented Dec 21, 2023

Description

This PR introduces a new feature that enhances the replacement functionality in the DefaultRegexpPairs. It allows for more flexible replacements by supporting dynamic replacement of matched groups from the regular expression pattern.

  • Added checkbox for explicit AWS usage
  • Refactored LogFileFilterOutputStream to handle multiple DefaultRegexpPairs

Testing done

Test cases are provided in the project

Submitter checklist

Preview Give feedback


Screenshot from 2023-12-21 11-30-14

@amandel
Copy link
Member

amandel commented Dec 21, 2023

Hi Fredrik,
thanks a lot for your contribution. I wonder if we really should have 2 options to add default or/and AWS default patterns? If you have reasons to keep it separate I'm fine, otherwise I would prefer to have only one option which enables both sets.
Kind regards, Andreas.

@flysen
Copy link
Author

flysen commented Dec 21, 2023

Hi Andreas,
From start I was thinking in your direction, but then I ended up to separate it. My thoughts went like this:

  • Separation into clear categories and refactor LogFileFilterOutputStream to open up for further development and no breaking change for existing implementations.
  • I also thinking to add categories for other "common" implementations like DataDog; GCP, Azure.
  • Eliminate the overhead of parsing each line with a huge set of regexp and patterns not in scope, made me go this direction
  • Test cases are simpler when having smaller sets
    Viele Grüße Fredrik

@flysen flysen mentioned this pull request Dec 29, 2023
@flysen
Copy link
Author

flysen commented Dec 29, 2023

Hi @amandel and happy new year soon,
Not sure this is the path forward, but I provided another integration on top of this --> PR.
Cheers
Fredrik

@amandel amandel self-requested a review December 30, 2023 12:41
@amandel amandel merged commit b08e705 into jenkinsci:main Dec 30, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants