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 profiling to monitor heap memory #318

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Conversation

rutul25
Copy link
Contributor

@rutul25 rutul25 commented Mar 9, 2022

No description provided.

@rutul25 rutul25 requested a review from vnktram March 9, 2022 10:17
logger.Ctx(ctx).Infow("initialising pprof profiles")
go func() {
if componentName == Web {
http.ListenAndServe("metro-web-pprof.concierge.stage.razorpay.in:8080", nil)
Copy link

Choose a reason for hiding this comment

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

Found an HTTP server without TLS. Use 'http.ListenAndServeTLS' instead. See https://golang.org/pkg/net/http/#ListenAndServeTLS for more information.

🔴 Fix or ignore this finding to merge your pull request.
🙈 From go.lang.security.audit.net.use-tls.use-tls.

if componentName == Web {
http.ListenAndServe("metro-web-pprof.concierge.stage.razorpay.in:8080", nil)
} else if componentName == Worker {
http.ListenAndServe("metro-worker-pprof.concierge.stage.razorpay.in:8080", nil)
Copy link

Choose a reason for hiding this comment

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

The profiling 'pprof' endpoint is automatically exposed on /debug/pprof. This could leak information about the server. Instead, use import "net/http/pprof". See https://www.farsightsecurity.com/blog/txt-record/go-remote-profiling-20161028/ for more information and mitigation.

🔴 Fix or ignore this finding to merge your pull request.
🙈 From go.lang.security.audit.net.pprof.pprof-debug-exposure.

if componentName == Web {
http.ListenAndServe("metro-web-pprof.concierge.stage.razorpay.in:8080", nil)
} else if componentName == Worker {
http.ListenAndServe("metro-worker-pprof.concierge.stage.razorpay.in:8080", nil)
Copy link

Choose a reason for hiding this comment

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

Found an HTTP server without TLS. Use 'http.ListenAndServeTLS' instead. See https://golang.org/pkg/net/http/#ListenAndServeTLS for more information.

🔴 Fix or ignore this finding to merge your pull request.
🙈 From go.lang.security.audit.net.use-tls.use-tls.

logger.Ctx(ctx).Infow("initialising pprof profiles")
go func() {
if componentName == Web {
http.ListenAndServe("metro-web-pprof.concierge.stage.razorpay.in:8080", nil)
Copy link

Choose a reason for hiding this comment

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

The profiling 'pprof' endpoint is automatically exposed on /debug/pprof. This could leak information about the server. Instead, use import "net/http/pprof". See https://www.farsightsecurity.com/blog/txt-record/go-remote-profiling-20161028/ for more information and mitigation.

🔴 Fix or ignore this finding to merge your pull request.
🙈 From go.lang.security.audit.net.pprof.pprof-debug-exposure.

logger.Ctx(ctx).Infow("initialising pprof profiles")
go func() {
myMux := http.DefaultServeMux
if err := http.ListenAndServe("localhost:8080", myMux); err != nil {
Copy link

Choose a reason for hiding this comment

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

Found an HTTP server without TLS. Use 'http.ListenAndServeTLS' instead. See https://golang.org/pkg/net/http/#ListenAndServeTLS for more information.

🔴 Fix or ignore this finding to merge your pull request.
🙈 From go.lang.security.audit.net.use-tls.use-tls.

@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2022

Codecov Report

Merging #318 (59a641b) into master (697f4f9) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #318      +/-   ##
==========================================
- Coverage   59.28%   59.26%   -0.03%     
==========================================
  Files         124      124              
  Lines        9506     9506              
==========================================
- Hits         5636     5634       -2     
- Misses       3515     3516       +1     
- Partials      355      356       +1     
Flag Coverage Δ
integration 45.32% <ø> (ø)
unittests 56.82% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
internal/subscriber/subscriberimpl.go 77.53% <0.00%> (-0.62%) ⬇️

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 697f4f9...59a641b. Read the comment docs.

@vnktram vnktram added wontfix This will not be worked on do-not-merge labels Jun 13, 2022
@vnktram vnktram removed their request for review June 15, 2022 11:02
"net/http"

// blank import added for testing.
_ "net/http/pprof"
Copy link

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
G108: Profiling endpoint is automatically exposed on /debug/pprof (gosec)

@@ -119,3 +126,14 @@

logger.Ctx(ctx).Infow("stopped metro")
}

// sets up pprof profile for perfomance monitoring
Copy link

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
perfomance is a misspelling of performance (misspell)


// sets up pprof profile for perfomance monitoring
func setPprofProfiles(ctx context.Context, componentName string) {
logger.Ctx(ctx).Infow("initialising pprof profiles")
Copy link

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
initialising is a misspelling of initializing (misspell)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants