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 tcp/udp check connection input plugin #650

Closed
wants to merge 1 commit into from

Conversation

titilambert
Copy link
Contributor

Rebased PR of #476

@titilambert titilambert force-pushed the connections branch 2 times, most recently from cbff449 to 72b160f Compare February 5, 2016 05:06
@titilambert
Copy link
Contributor Author

@sparrc Thanks ! I fixed the sample config ;)

@sparrc
Copy link
Contributor

sparrc commented Feb 5, 2016

could you write a README in the format of https://github.com/influxdata/telegraf/blob/master/plugins/inputs/EXAMPLE_README.md?

@sparrc
Copy link
Contributor

sparrc commented Feb 5, 2016

since we can specify multiple [[input.connection]] entries, can you change config to look more like this:

[[inputs.connection]]
  # must be either "tcp" or "udp"
  type = "tcp"
  # Server address (default IP localhost)
  address = "github.com:80"
  # Set timeout (default 1.0)
  timeout = 1.0
  # Set read timeout (default 1.0)
  read_timeout = 1.0
  # String sent to the server
  send = "ssh"
  # Expected string in answer
  expect = "ssh"

@titilambert titilambert force-pushed the connections branch 3 times, most recently from af1a478 to 27d22e1 Compare February 6, 2016 19:11
@titilambert titilambert changed the title Add tcp/udp check connection plugin Add tcp/udp check connection input plugin Feb 6, 2016
@titilambert
Copy link
Contributor Author

@sparrc Done !

@gianarb
Copy link

gianarb commented Feb 8, 2016

Awesome!! A good way to decrease the latency.. 👍 Thanks

@sparrc
Copy link
Contributor

sparrc commented Feb 13, 2016

This looks good @titilambert, the only problem I have is that I feel like the name connection is a bit vague.

But, I'm also having trouble coming up with a good name, maybe just response_time would be a better name? or net_response?

@titilambert
Copy link
Contributor Author

@sparrc I had the same feeling when I choose the name ...
Nagios equivalent plugins are check_tcp and check_udp...
I think I like net_response, but I would suggest some others: net_connection, tcpudp_connection, port_check, tcpudp_check.
What do you think ?

@sparrc
Copy link
Contributor

sparrc commented Feb 13, 2016

I like net_response or net_connection, with a slight preference for net_response since it describes the data that you get out of the plugin (response time)

I'm leaning away from tcpudp because at some point I think we could add http response times to this plugin as well.

@titilambert
Copy link
Contributor Author

@sparrc let's go for net_response !
I'm currently doing a input plugin for kubernetes, once finished, I will make this change.

@sparrc
Copy link
Contributor

sparrc commented Feb 13, 2016

OK, but someone has already tried to do that: #665

I rejected the PR because it seems that the kubernetes client library is very tied up with their entire repo, and adding it requires adding 10-12 huge dependencies to Telegraf that almost doubled the binary size.

Although there's also the option of requiring kubernetes-cli or some external executable as a dependency of the plugin

@titilambert
Copy link
Contributor Author

@sparrc I do not use kubernetes client lib :)
I use "/metrics" endpoints which show data with prometheus format :) So I just use prometheus client lib.
I'm currently adding a new parser in plugins/parsers folder.
And for other metrics, I would recommend https://github.com/kubernetes/heapster which can send data to influxDB :)

@titilambert
Copy link
Contributor Author

@sparrc changes done !

@sparrc
Copy link
Contributor

sparrc commented Feb 15, 2016

great, thanks for another contribution @titilambert

@sparrc
Copy link
Contributor

sparrc commented Feb 15, 2016

@titilambert I sent you an email for some swag ~2 weeks ago, did you receive it?

@titilambert
Copy link
Contributor Author

@sparrc no I didn't anything. Could you PM me on twitter ?

@sparrc
Copy link
Contributor

sparrc commented Feb 15, 2016

@titilambert sure, will do

@titilambert titilambert deleted the connections branch February 20, 2016 05:19
geodimm pushed a commit to miketonks/telegraf that referenced this pull request Mar 10, 2016
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