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 config option to enable pprof #253

Merged
merged 2 commits into from
Mar 14, 2019
Merged

Conversation

xtda
Copy link
Contributor

@xtda xtda commented Mar 13, 2019

Description

Simple PR to enable pprof for ongoing performance testing

It adds a profiler config option that is disabled by default along with some Makefile aliases for ease of access with the net/http/pprof package for heap and CPU profiling

At the moment it listens on the same address as the REST interface this may change in future

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Automated testing using go test ./.. confirms all existing tests pass
Manual testing confirms pprof is working as intended when enabled

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation and regenerated documentation via the documentation tool
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally and on Travis with my changes
  • Any dependent changes have been merged and published in downstream modules

@xtda xtda requested review from thrasher- and gloriousCode March 13, 2019 05:10
@codecov-io
Copy link

codecov-io commented Mar 13, 2019

Codecov Report

Merging #253 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #253      +/-   ##
==========================================
- Coverage   42.22%   42.21%   -0.01%     
==========================================
  Files         123      123              
  Lines       26619    26622       +3     
==========================================
  Hits        11239    11239              
- Misses      14453    14456       +3     
  Partials      927      927
Impacted Files Coverage Δ
config/config.go 64.75% <ø> (ø) ⬆️
restful_router.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6cb356c...aca43c0. Read the comment docs.

@thrasher-
Copy link
Collaborator

Note: If the webserver is disabled but the profiler config is enabled, then pprof won't load as it uses the same mux router as the webserver, as mentioned in the comment. This will be segregated in the engine branch.

Copy link
Collaborator

@gloriousCode gloriousCode left a comment

Choose a reason for hiding this comment

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

Good stuff 👍 Tested locally

@thrasher- thrasher- merged commit 58bd0a3 into thrasher-corp:master Mar 14, 2019
@xtda xtda deleted the profiler branch March 19, 2019 04:22
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.

4 participants