-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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(outputs.quix): Add plugin #16144
Conversation
Thanks so much for the pull request! |
|
1 similar comment
|
b7dcc9f
to
f32b854
Compare
There was a problem hiding this 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 @tomas-quix! I do have some comments in the code. Furthermore, I want to encourage you to implement some unit-tests for this plugin, e.g. by mocking the config server and using a quix docker container or at least using a kafka instance mocking the replies...
First implementation - no defaults
@tomas-quix I've pushed some cleanups and also added some unit-tests. What is currently missing is an integration test (see commented code in the test file) with a Kafka container configured to use SASL-SCRAM256 which you enforce. Could you take a look please!?!? Furthermore, I do have the following questions:
|
I have fixed points 1 and 2. I do not understand point 3 however, what do you mean by that? Do you mean to just hardcode nanoseconds as a unit? |
@tomas-quix exactly. Why not send the data with the available precision? My problem is that with an option your pipeline needs to know what was configured in Telegraf and vice versa which usually is a source of trouble if both are not maintained by the same person (which is likely)... If you hard-code it, the Quix side can expect the precision/format... |
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
Co-authored-by: stereosky <[email protected]> Co-authored-by: Sven Rebhan <[email protected]>
Summary
Checklist
Related issues