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

Added pprof tool #2512

Merged
merged 6 commits into from
Mar 30, 2017
Merged

Added pprof tool #2512

merged 6 commits into from
Mar 30, 2017

Conversation

d-ulyanov
Copy link
Contributor

Added standard GO pprof tool to Telegraf. This tool is useful for developers: from time to time we need to understand, which plugin or subsystem of Telegraf consumes too much resources.

By default, pprof is disabled.
To enable pprof run Telegraf with pprofaddr parameter:
telegraf --config etc/telegraf.conf --pprofaddr localhost:6060

@sparrc
Copy link
Contributor

sparrc commented Mar 8, 2017

thanks for adding this, can you add it to the --help doc? also put in the doc the format of the address, (ie, localhost:6060)

I would also call the flag --pprof-addr (with a dash)

@d-ulyanov
Copy link
Contributor Author

@sparrc I've just added pprof-addr to help with example and renamed pprofaddr to pprof-addr - I agree, it seems better.

Reason why we've added pprof - high CPU consumption by Telegraf: up to 20% of total CPU on modern servers.
It was caused by built-in NETSTAT plugin - it makes too much system calls inside github.com/shirou/gopsutil to calculate network statistics. Please, consider replacement of this plugin.
As temporary solution - we've replaced netstat plugin by this: https://github.com/monitoring-tools/telegraf-plugins/tree/master/netstat

@d-ulyanov
Copy link
Contributor Author

@sparrc please, run CI tests again: https://circleci.com/gh/influxdata/telegraf/6260 - its timed out

@d-ulyanov
Copy link
Contributor Author

@sparrc Let's include this into 1.3.0?

@danielnelson danielnelson added this to the 1.3.0 milestone Mar 16, 2017
Copy link
Contributor

@danielnelson danielnelson left a comment

Choose a reason for hiding this comment

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

Lets update the log message and add this to the CHANGELOG and we should be good to go.

if *pprofAddr != "" {
go func() {
log.Printf(
"I! Starting pprof on %s. Open profiling tools page in browser: /debug/pprof",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we have all the information we need to print the full url to this page, let have it write something like:
I! Starting pprof HTTP server at http://localhost:6060/debug/pprof This way I can click on it or copy paste it more easily.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danielnelson, thanks for review. Changes done.

Could you also move forward our contributions:
#2387
#2594

We hope to see them in 1.3 too

@danielnelson
Copy link
Contributor

Can you rebase?

@danielnelson danielnelson merged commit c980c92 into influxdata:master Mar 30, 2017
@danielnelson
Copy link
Contributor

Nevermind, It's merged!

calerogers pushed a commit to calerogers/telegraf that referenced this pull request Apr 5, 2017
vlamug pushed a commit to vlamug/telegraf that referenced this pull request May 30, 2017
maxunt pushed a commit that referenced this pull request Jun 26, 2018
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