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

feature: add the ua-parser plugin to the filter plugins #88

Merged
merged 2 commits into from
Nov 28, 2023

Conversation

csabatuz-chess
Copy link
Contributor

This changeset adds the https://github.com/bungoume/fluent-plugin-ua-parser Fluent plugin to the filters.

Required for: kube-logging/logging-operator#1594

@pepov
Copy link
Member

pepov commented Nov 27, 2023

@csabatuz-chess thanks for your contribution! I just realized we haven't added a check to make sure the lock file is in sync with the dependencies so I've just updated it manually and will add a follow up issue to make sure the CI is checking it

@pepov
Copy link
Member

pepov commented Nov 27, 2023

follow up: #89

@csabatuz-chess
Copy link
Contributor Author

@pepov Thanks for the mopping-up, the lockfile evaded my attention.
Happy to contribute.

I am following a somewhat selfish path here, we are using kube-logging widely on our infra, and this plugin was the only thing that's missing to be able to satisfy our output requirements.
So here I tried to find the shortest path to get us what we need, please do point out if there's a bigger picture that I'm missing.

Thanks again, looking forward to discuss this and the other PR, let me know if I took a wrong turn in either, and I'll correct.

@pepov
Copy link
Member

pepov commented Nov 27, 2023

All good, thanks for the context! Today we have capacity to support a single image with three layers (base,filters,full), so if there is a legitimate use of a plugin we can add it to one of these layers, just as you did.

There is a chance that a new plugin introduces a security vulnerability or some regression and in that case we might have to pull it out and then you will have to add it and build it on your own on top of our base. In this case it doesn't make any changes in the existing dependencies and if the plugin is not used I don't see it doing any harm.

@pepov pepov requested a review from OverOrion November 27, 2023 17:21
Copy link
Contributor

@OverOrion OverOrion 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 your contribution, much appreciated! 🙌

@OverOrion OverOrion merged commit 24502f8 into kube-logging:main Nov 28, 2023
4 checks passed
@csabatuz-chess csabatuz-chess deleted the plugin/ua-parser branch November 28, 2023 09:40
@csabatuz-chess
Copy link
Contributor Author

Thank you, please also take a look at the PR that attemps to use this new plugin:
kube-logging/logging-operator#1594

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.

3 participants