Linux tcp stats by maatthc · Pull Request #2070 · DataDog/dd-agent

maatthc · 2015-11-12T04:13:49Z

Added metrics from Linux stats:
/proc/net/sockstat
/proc/sys/net/ipv4/tcp_mem
/proc/sys/net/ipv4/tcp_max_orphans
/proc/sys/net/ipv4/netfilter/ip_conntrack_count
/proc/sys/net/ipv4/ip_conntrack_max Also from the command : /usr/bin/getconf PAGESIZE Those are critical metrics for high performance applications.

- /proc/net/sockstat - /proc/sys/net/ipv4/tcp_mem - /proc/sys/net/ipv4/tcp_max_orphans - /proc/sys/net/ipv4/netfilter/ip_conntrack_count - /proc/sys/net/ipv4/ip_conntrack_max Also from the command : /usr/bin/getconf PAGESIZE Those are critical metrics for high performance applications.

…rnel versions.

…rotocol - this way you don't need a WebServer running in front of the PHP_FPM process.

remh · 2015-11-18T21:21:54Z

Thanks a lot @maatthc !

Can you make sure to respect the contributing guidelines please?
https://github.com/DataDog/dd-agent/blob/master/CONTRIBUTING.md

JohnLZeller · 2015-12-29T20:05:53Z

Call this with self., rather than with Network.

JohnLZeller · 2015-12-29T20:18:52Z

@maatthc I think these metrics will be really useful once the check is cleaned up. Thanks for contributing!

Here is a summary of the notes I've made, that occur multiple times:

When calling the functions you've written, use self., rather than Network.
When opening a file, using with open(proc_name, 'r') as proc: instead is neater
If you're going to use self in a function, you need to either remove the decorator @staticmethod, or use @classmethod.
Nitpick: variable names shouldn't be camel case :)
I'm not entirely sure we want to return report a 0 value when we encounter an IOError. This seems to have the potential to misguide

Finally, many of your functions do the same thing, and could be condensed into a single function taking a string proc_name, and returning a value.

@maatthc would you also rebase this to be able to merge with master? There were some changes made to the network check that seem to be conflicting with your changes.

JohnLZeller · 2015-12-29T20:19:41Z

Don't use print, if you need to output a string, use self.log.*

irabinovitch · 2016-02-05T19:56:57Z

@maatthc Thank you for your contribution to the datadog agent. These look like great additions to the existing set of metrics we collect. Have you had a chance to review the feedback in code review here? We'd love to include this in an upcoming release.

maatthc · 2016-02-25T14:10:06Z

Hi guys,
I've fixed all these items on my Repo. What should I do now? Close this pull request and raise a new one?

Cheers

JohnLZeller · 2016-02-25T14:23:31Z

Thanks! No, you should squash your commits into one and then force push it up to your branch.

maatthc · 2016-02-25T16:15:59Z

Okay,
Done!

JohnLZeller · 2016-02-25T16:31:28Z

Thanks @maatthc! It looks like there are still merge conflicts with the master branch. Could you please try to resolve these conflicts, and then update this PR with the necessary changes?

maatthc · 2016-02-25T17:17:37Z

Fixed conflict.

JohnLZeller · 2016-03-11T07:21:53Z

@maatthc Could you please break out commit ba5a2c5 into a separate pr?

And, please squash the remaining commits here once you've done that :)

JohnLZeller · 2016-03-11T07:31:52Z


        return metrics_by_interface
+
+    def _linux_sockstat_get_conntrack_max(self):


This function body repeats itself in two others. Let's condense the three into one, passing in the proc_name, instead.

JohnLZeller · 2016-03-29T18:20:08Z

Hey @maatthc :) Just pinging you again to see if you have time to make the changes I pointed out.

JohnLZeller · 2016-04-28T18:14:57Z

Hey @maatthc just pinging again to see if you've had a chance to check this out

irabinovitch · 2016-05-03T00:08:57Z

@maatthc We're going to move this out to 5.9 since there are still a few pending issues, and a 5.8 release is imminent.

truthbk · 2017-02-02T20:49:42Z

It seems like this PR has been inactive for a while, I think it should be re-opened in https://github.com/DataDog/integrations-core where the active development for the checks is now taking place. Thank you @maatthc!

irabinovitch · 2017-02-04T08:27:50Z

@maatthc Thank you again for your efforts here. The new metrics are indeed valuable and we would love to merge them. Unfortunately due to the age of the PR there are quite a few conflicts that would need to be addressed in addition the feedback from Remi, John and Jaime.

As we've not heard back in sometime I'm going to close out this PR, but I do hope you will open a a new one against our integrations repo at: https://github.com/DataDog/integrations-core

If you have any questions, we are happy to discuss here in this PR or on Slack in #integrations.

Thank you for being a member of our community and helping to make Datadog awesome.

maatthc added 5 commits September 10, 2015 19:40

Better parsing of the file /proc/net/sockstat to support different ke…

5ba1dfa

…rnel versions.

Fixed Flake8 style issues.

9637f0a

Added support to Talk to the PHP process using FCGI instead of HTTP p…

ba5a2c5

…rotocol - this way you don't need a WebServer running in front of the PHP_FPM process.

Fixed Flake8 style issues and Unix domain socket location.

7932887

maatthc mentioned this pull request Nov 16, 2015

Added metrics from Linux /proc stats as sockstat and others #1904

Closed

remh added this to the 5.7.0 milestone Nov 18, 2015

remh modified the milestones: To be determined, 5.7.0 Dec 22, 2015

remh assigned irabinovitch Dec 22, 2015

JohnLZeller mentioned this pull request Dec 28, 2015

Rebased: Linux tcp stats #2170

Closed

JohnLZeller closed this Dec 28, 2015

JohnLZeller reopened this Dec 29, 2015

JohnLZeller reviewed Dec 29, 2015
View reviewed changes

Comment thread checks.d/network.py Outdated

JohnLZeller Dec 29, 2015

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

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

Call this with self., rather than with Network.

JohnLZeller reviewed Dec 29, 2015
View reviewed changes

Comment thread checks.d/network.py Outdated

JohnLZeller Dec 29, 2015

Copy link
Copy Markdown

Contributor

Choose a reason for hiding this comment

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

Don't use print, if you need to output a string, use self.log.*

remh modified the milestones: Triage, To be determined Jan 7, 2016

Applied JohnLZeller requests

40441e0

Fixed conflict on checks.d/network.py

2e46e33

remh modified the milestones: 5.8.0, Triage Feb 25, 2016

JohnLZeller reviewed Mar 11, 2016
View reviewed changes

olivielpeau modified the milestones: 5.9.0, 5.8.0 Apr 29, 2016

gmmeyer modified the milestones: 5.10.0, 5.9.0 Aug 12, 2016

truthbk modified the milestones: Integration SDK - New Integrations, 5.10.0 Aug 12, 2016

gmmeyer added the sdk-later label Jan 30, 2017

truthbk self-assigned this Jan 31, 2017

truthbk removed their assignment Feb 2, 2017

irabinovitch closed this Feb 4, 2017


		return metrics_by_interface

		def _linux_sockstat_get_conntrack_max(self):

Uh oh!

Conversation

maatthc commented Nov 12, 2015

Uh oh!

remh commented Nov 18, 2015

Uh oh!

JohnLZeller Dec 29, 2015

Choose a reason for hiding this comment

Uh oh!

JohnLZeller commented Dec 29, 2015

Uh oh!

JohnLZeller Dec 29, 2015

Choose a reason for hiding this comment

Uh oh!

irabinovitch commented Feb 5, 2016

Uh oh!

maatthc commented Feb 25, 2016

Uh oh!

JohnLZeller commented Feb 25, 2016

Uh oh!

maatthc commented Feb 25, 2016

Uh oh!

JohnLZeller commented Feb 25, 2016

Uh oh!

maatthc commented Feb 25, 2016

Uh oh!

JohnLZeller commented Mar 11, 2016

Uh oh!

JohnLZeller Mar 11, 2016

Choose a reason for hiding this comment

Uh oh!

JohnLZeller commented Mar 29, 2016

Uh oh!

JohnLZeller commented Apr 28, 2016

Uh oh!

irabinovitch commented May 3, 2016

Uh oh!

truthbk commented Feb 2, 2017

Uh oh!

irabinovitch commented Feb 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

irabinovitch commented Feb 4, 2017 •

edited

Loading