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 kafka backend (#1) #527

Merged
merged 4 commits into from
Oct 9, 2024
Merged

Add kafka backend (#1) #527

merged 4 commits into from
Oct 9, 2024

Conversation

saryani
Copy link
Contributor

@saryani saryani commented Oct 4, 2024

Hello @javuto. This PR adds the kakfa support to osctrl, I would appreciate if you can go through the changes and give me your feedbacks.

  • add kafka as a new logging backend
  • update configuration to allow defining a kafka logger
  • updated the docker-compose to allow to run different backends

* add kafka as a new logging backend
* update configuration to allow defining a kafka logger
* updated the docker-compose to allow to run different backends
@javuto javuto self-requested a review October 4, 2024 14:09
@javuto javuto added the 🗄️ logging Logging related issues label Oct 4, 2024
* add kafka as a new logging backend
* added required configuration definition to allow setting up a kafka logger
* updated the docker-compose to allow to run different backends
* update code to use the new logger
* fix conflicts
@saryani
Copy link
Contributor Author

saryani commented Oct 8, 2024

I fixed the conflict, also used the new "github.com/rs/zerolog/log" package for logging

@javuto
Copy link
Collaborator

javuto commented Oct 8, 2024

PR looks good, thank you! 🙏

IIRC in the other PR you opened there were some changes to the Makefile to bring up the backend but I don't see it here. Verify that and if it is not necessary, feel free to merge the PR.

Thanks again!

@javuto
Copy link
Collaborator

javuto commented Oct 8, 2024

Nevermind my comment about the Makefile target, it is up-backend from #522, I was mixing up the two PRs 😅

Good to merge! 🚢

@saryani
Copy link
Contributor Author

saryani commented Oct 9, 2024

Awesome, thanks for the review. @javuto 🙏

@javuto javuto merged commit 6d47cca into jmpsec:main Oct 9, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄️ logging Logging related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants