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: Nats Jetstream consumer #11046 add simple support for jetstream subjects #11373

Merged
merged 15 commits into from
Jul 18, 2022

Conversation

b3rtram
Copy link
Contributor

@b3rtram b3rtram commented Jun 23, 2022

Required for all PRs:

  • [ x] Updated associated README.md.
  • Wrote appropriate unit tests.
  • Pull request title or commits are in conventional commit format

resolves #11046

Changed nats_consumer input plugin. Added a new configuration element js_subjects where a user can add the subjects to subscribe with jetstream context. If the length of the js_subjects_array is zero it will ignored. js_subject is optional, the original subject is not optional, so there are no breaking changes.
Because there is no interface in go nats consumer I could not reuse nats.Conn, I need to add a new jsConn variable and copy code :(

This is my first github code contribution. Please be patient and give me feedback

@telegraf-tiger telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Jun 23, 2022
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Hey @b3rtram and thanks for the nice contribution! I have some smaller comments in the code. Can you please take a look?

plugins/inputs/nats_consumer/README.md Outdated Show resolved Hide resolved
plugins/inputs/nats_consumer/nats_consumer.go Outdated Show resolved Hide resolved
plugins/inputs/nats_consumer/nats_consumer.go Outdated Show resolved Hide resolved
plugins/inputs/nats_consumer/nats_consumer.go Outdated Show resolved Hide resolved
plugins/inputs/nats_consumer/sample.conf Outdated Show resolved Hide resolved
telegraf.config Outdated Show resolved Hide resolved
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

The code looks good to me. I'd nevertheless like to ask you for a few more words in the README to help new/inexperienced users to find their way...

plugins/inputs/nats_consumer/README.md Outdated Show resolved Hide resolved
@srebhan srebhan self-assigned this Jul 4, 2022
@srebhan srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Jul 4, 2022
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Perfect! Thanks @b3rtram!

@telegraf-tiger
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nats Jetstream consumer
4 participants