parca icon indicating copy to clipboard operation
parca copied to clipboard

pkg/config: support seconds in pprof configuration

Open alperkokmen opened this issue 1 year ago • 1 comments

this updates the definition of PprofProfilingConfig such that a new integer field Seconds is added which can be set for profiles, in an attempt to resolve https://github.com/parca-dev/parca/issues/2818.

additionally, it changes the behavior where setting any profiling configurations will end up overriding the default so merge will now become a replace.

seconds is accepted as a query parameter and with this change, when provided, it will be used as scraping duration rather than the scraping interval value. this allows having scraping where process being profiled isn't under constant profiling. this only applies to configurations that have delta enabled.

https://pkg.go.dev/net/http/pprof

alperkokmen avatar May 14 '24 06:05 alperkokmen

@brancz i have taken a stab at implementing the proposed solution in #2818. please take a look.

alperkokmen avatar May 14 '24 16:05 alperkokmen

@brancz, i just wanted to bump this up to get some eyes on it; could you please take a look for add folks who would be able to review? thank you.

alperkokmen avatar May 29 '24 03:05 alperkokmen

Apologies for missing this, this looks great!

brancz avatar May 29 '24 09:05 brancz

I'm really sorry, this needs a rebase, but otherwise lgtm! And sorry again for this slipping through the cracks, so thank you for pinging me again. In the future also feel free to ping me on the Parca Discord server if you want to make sure to get a faster review! :)

brancz avatar May 29 '24 09:05 brancz

thanks for taking a look, @brancz. i rebased and addressed the feedback. PTAL when you get a chance.

alperkokmen avatar Jun 03 '24 03:06 alperkokmen

@brancz i think i need one last approval before checks are executed?

alperkokmen avatar Jun 04 '24 17:06 alperkokmen

Thank you for the awesome contribution!

brancz avatar Jun 04 '24 20:06 brancz