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 connections status and UDP socket count plugin #244

Closed
wants to merge 4 commits into from

Conversation

shirou
Copy link
Contributor

@shirou shirou commented Oct 5, 2015

Add tcp connection status plugin.
This plugin counts TCP Connection status such as established, time_wait, listen and so on.

related to #235

This plugin only counts the number. gopsutil can get family, local/remote address, port and so on. But I can not imagine how to use these information from telegraf. If you have any idea, I can add.

@sparrc
Copy link
Contributor

sparrc commented Oct 5, 2015

@shirou thank you for the contribution, but it looks like you've broken some unit tests by adding a function to the PS interface. (see MockPS).

Also, can you add some simple unit tests for this plugin?

@sparrc
Copy link
Contributor

sparrc commented Oct 5, 2015

Also, can you please write a README showing the metrics this collects? See #177

@sparrc
Copy link
Contributor

sparrc commented Oct 5, 2015

you can use the cpu plugin as an example: https://github.com/influxdb/telegraf/blob/master/plugins/system/CPU_README.md

@shirou
Copy link
Contributor Author

shirou commented Oct 6, 2015

Roger. I will write a test and a README. please wait until this weekend.

@shirou
Copy link
Contributor Author

shirou commented Oct 8, 2015

@sparrc I added mockPS and README. Could you check these out?

However, I know my English is terrible, I would appreciate if you fix my English, and add more good explanation about TCP connection state.

@sparrc
Copy link
Contributor

sparrc commented Oct 10, 2015

Thanks @shirou, but looking at your plugin, doesn't this also collect UDP information as well? Maybe you should generalize the name to something like netstat

I know that the current state of the system unit tests is a bit confusing (sorry!), but you should be able to accomplish it by modifying the data structure like this: https://github.com/influxdb/telegraf/blob/master/plugins/system/system_test.go#L54-L61

And then check down below like this: https://github.com/influxdb/telegraf/blob/master/plugins/system/system_test.go#L183-L193

@shirou shirou changed the title add TCP connections status plugin add TCP connections status and UDP socket count plugin Oct 11, 2015
@shirou
Copy link
Contributor Author

shirou commented Oct 11, 2015

@sparrc Sorry for anonying you. I couldn't get you. Now I added some test to system_test.go.
And as you said, I have added UDP socket counts to measure, and renamed to netstat.

@sparrc sparrc closed this in d394003 Oct 12, 2015
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.

None yet

2 participants