Add flag to enable profiling with HTTP pprof

[jsoriano]

Add flag to enable profiling with HTTP pprof.

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2021-08-13T07:56:00.034+0000

• Duration: 5 min 12 sec

• Commit: 124870e

Test stats 🧪

Test	Results
Failed	0
Passed	92
Skipped	0
Total	92

Trends 🧪

[ruflin]

Should we enable it by default but only expose it on localhost? This has the advantage that also in prod you can exec into the container and check potential issues.

[jsoriano]

Should we enable it by default but only expose it on localhost? This has the advantage that also in prod you can exec into the container and check potential issues.

@ruflin I don't have a strong opinion, in general pprof uses to be optional in Go applications that support it, but I don't see a special problem on starting it by default, apart or opening an additional port, even if it is only locally. An alternative could be to don't enable it by default, but add the flag in the Docker images, where opening additional ports is surely not going to cause conflicts.

I have added it as default, as well as some docs. Error starting the server for http prof is handled now, so it explicitly fails if it cannot be opened, I think this is important to avoid cases where one ends-up profiling the wrong process :)

Let me know what you think.

[mtojek]

I think it's safe unless we expose it on public port, which I see it isn't the case here.

[ruflin]

Already merged but will still comment. If run outside the docker container, it might cause some conflicts with other go httpprof (agent?) on the same port. An alternative would have been to only enabled it by default if run inside the container (container passes the flag) but if run directly as binary it is not exposed by default. But that might also be surprising to have 2 different behaviours. One more option is just to enable it for us for the prod environments that are running.

[jsoriano]

Already merged but will still comment. If run outside the docker container, it might cause some conflicts with other go httpprof (agent?) on the same port. An alternative would have been to only enabled it by default if run inside the container (container passes the flag) but if run directly as binary it is not exposed by default. But that might also be surprising to have 2 different behaviours. One more option is just to enable it for us for the prod environments that are running.

Yes, these were exactly my original thoughts when I didn't enable it by default 🙂 I agree that enabling it only when used is cleaner, I will open a PR to go back to this, and will take this into account for the production deployment.

[jsoriano]

Disabling by default in #710