pkg/config: support seconds in pprof configuration #4623
Conversation
this updates the definition of `PprofProfilingConfig` such that a new integer field `Seconds` is added which can be set for profiles. 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
|
@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. |
|
Apologies for missing this, this looks great! |
| @@ -0,0 +1,149 @@ | |||
| package scrape | |||
There was a problem hiding this comment.
apologies, our linting will insist to add a license header here
| @@ -388,7 +388,11 @@ func targetsFromGroup(tg *targetgroup.Group, cfg *config.ScrapeConfig, targets [ | |||
| } | |||
|
|
|||
| if pcfg, found := cfg.ProfilingConfig.PprofConfig[profType]; found && pcfg.Delta { | |||
| params.Add("seconds", strconv.Itoa(int(time.Duration(cfg.ScrapeInterval)/time.Second))) | |||
| seconds := int(time.Duration(cfg.ScrapeInterval) / time.Second) | |||
There was a problem hiding this comment.
do you mind just leaving a few code comments here, I had to read it twice to understand it right, just mention that we default to the scrape interval but if the seconds are explicitly set, we use that
|
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! :) |
this updates the definition of
PprofProfilingConfigsuch that a new integer fieldSecondsis added which can be set for profiles, in an attempt to resolve #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