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.

Sign up for GitHub

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

Jump to bottom

Support Riemann-Protobuff Listener #8163

Merged
merged 1 commit into from
Nov 27, 2020
Merged

Support Riemann-Protobuff Listener #8163

merged 1 commit into from
Nov 27, 2020

Conversation

vipinvkmenon
Copy link
Contributor

This commits adds an input plugin that will allow telegraf to
serve as a simple riemann-protobuff listener, that riemann clients
can connect to.

It uses the recommended protobuf structure from the recommended
riemann-go-client maintained in the riemann repository.

Closes #8160

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

@vipinvkmenon vipinvkmenon force-pushed the riemann_listener_2 branch 3 times, most recently from e90e060 to 0236431 Compare September 21, 2020 18:34
@ssoroka
Copy link
Contributor

ssoroka commented Sep 24, 2020

I've never heard of this before. It might be better off as an external plugin. Check out the shim readme on externalizing plugins, and let me know what you think.

@vipinvkmenon
Copy link
Contributor Author

vipinvkmenon commented Sep 24, 2020 •
edited
Loading

Exactly.. Riemann(http://riemann.io/).
Its an event based metric collection framework.. While it has its advantages working with push based metrics, it nevergot as big as metric capture solutions like TIG stack or prometheus..
One of the reasons I thought to bring it in as a standard input plugin rather than as a shim was becuase riemann is already present in telegraf as an output plugin. So it made sense to bring it in as an inbuilt input plugin as well..

@vipinvkmenon vipinvkmenon reopened this Sep 24, 2020
@vipinvkmenon
Copy link
Contributor Author

Apologgies... I accedentally closed the pr trying to comment from my phone

ssoroka
ssoroka reviewed Nov 3, 2020
View reviewed changes
Copy link
Contributor

@ssoroka ssoroka left a comment •
edited
Loading

Choose a reason for hiding this comment

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

looks really good. some small comments. Also there's some conflicts with the go package files that needs to be resolved.

plugins/inputs/riemann_listener/riemann_listener.go Outdated
wg := sync.WaitGroup{}

for {
c, err := rsl.Accept()
Copy link
Contributor

Choose a reason for hiding this comment

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

can this take a context to shut down cleanly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm.. I actually followed this paradigm similar to how it was implemented in socket_listener, so thought this should be ok.
Your thought :) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

it's possible socket_listener needs to be updated too. My concern here is if Telegraf shuts down cleanly when it's waiting for a connection. should be easy enough to test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... I think it should be fine. Anyhow, added a context handler to close connections safely as well.

plugins/inputs/riemann_listener/riemann_listener.go Show resolved Hide resolved
plugins/inputs/riemann_listener/README.md Outdated Show resolved Hide resolved
@vipinvkmenon
Copy link
Contributor Author

was on a break and back after a hiatus.
Happy to hear back from you guys 😃
Will update the branch to reflect the new go mods as well.

@ssoroka
Copy link
Contributor

ssoroka commented Nov 20, 2020

Thanks! happy to see this is active.

@vipinvkmenon
Copy link
Contributor Author

Have pushed with newer changes

@vipinvkmenon
Support Riemann-Protobuff Listener
4425317 
This commits adds an input plugin that will allow telegraf to
serve as a simple riemann-protobuff listener, that riemann clients
can connect to.

It uses the recommended protobuf structure from the recommended
riemann-go-client maintained in the riemann repository.

Closes influxdata#8160

Required for all PRs:
- [x] Signed [CLA](https://influxdata.com/community/cla/).
- [x] Associated README.md updated.
- [x] Has appropriate unit tests.
@ssoroka
Copy link
Contributor

ssoroka commented Nov 20, 2020

The context didn't exactly work out like I expected. I don't think it's providing the value I wanted, so if you want to remove it I'm fine with that. Let me know when you're ready to merge either way and we can.

@vipinvkmenon
Copy link
Contributor Author

vipinvkmenon commented Nov 21, 2020 •
edited
Loading

yes, let's merge it 😬

@ssoroka ssoroka merged commit d536f61 into influxdata:master Nov 27, 2020
@ssoroka
Copy link
Contributor

ssoroka commented Nov 27, 2020

Thank you!

arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
@vipinvkmenon
Support Riemann-Protobuff Listener (influxdata#8163)
2a2a777
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ssoroka ssoroka ssoroka approved these changes

Assignees
No one assigned
Labels
new plugin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Riemann-Protobuff as in telegraf's inputs
3 participants
@vipinvkmenon @ssoroka @sjwang90