parca icon indicating copy to clipboard operation
parca copied to clipboard

Enable specifying a prefix for pprof

Open roidelapluie opened this issue 3 years ago • 12 comments

While setting up Parca for prometheus' prombench, I had to configure all the profiling configs, because we use a prefix.

Since the prefix is common to all, it would be useful to be able to provide the prefix.

instead of:

    profiling_config:                                                      
      pprof_config:
       memory_total:        
               enabled: true                                               
               path: /10543/prometheus-pr/debug/pprof/allocs               
       block_total:         
               enabled: true                                               
               path: /10543/prometheus-pr/debug/pprof/block                
       goroutine_total:              
               enabled: true                                               
               path: /10543/prometheus-pr/debug/pprof/goroutine            
       mutex_total:                                                        
               enabled: true
               path: /10543/prometheus-pr/debug/pprof/mutex                
       process_cpu:                                                        
               enabled: true
               path: /10543/prometheus-pr/debug/pprof/profile

We could have:

    profiling_config:                                                      
      path_prefix: /10543/prometheus-pr

bonus point if it can be set via relabeling :)

roidelapluie avatar Apr 12 '22 07:04 roidelapluie

This looks interesting, can I pick this up?

importhuman avatar Apr 25 '22 16:04 importhuman

Of course, @importhuman go ahead, give it a shot!

kakkoyun avatar Apr 25 '22 17:04 kakkoyun

Hey, had a couple of questions.

  • Adding the path_prefix field seems simple enough, modifying the ProfilingConfig struct. But how/where exactly do we look for the paths to add? As in, once the prefix is specified, where would the config paths be looked for?

  • What is relabeling? I understand there is a Prometheus package with the name. In the context of this issue, would it be for replacing old paths with the new prefix?

importhuman avatar Apr 30 '22 17:04 importhuman

  • Adding the path_prefix field seems simple enough, modifying the ProfilingConfig struct. But how/where exactly do we look for the paths to add? As in, once the prefix is specified, where would the config paths be looked for?

That's correct, we can add a field to ProfilingConfig. Then if we have a non-empty string, we would for range over all PprofConfig and take its path and prefix it with the given prefix. The place where we do this could be around here: https://github.com/parca-dev/parca/blob/ba113635bf4708d3a730028d4c538a8ee1d03239/pkg/config/config.go#L243-L247

I'm not 100% sure about how relabelling would work in this case right now. Maybe others can explain that one.

metalmatze avatar May 04 '22 15:05 metalmatze

Here is what I mean with relabeling:

static_configs:
- targets: [prometheus-pr, prometheus-release]
relabel_configs:
 - source_labels: [__address__]
   target_label: __pprof_prefix__
   replacement: /10543/$1/
 - source_labels: [__address__]
   target_label: instance
 - target_label: __address__
   replacement: prombench.prometheus.io

roidelapluie avatar May 09 '22 08:05 roidelapluie

Did you mean the source label of the first relabel config to be __prefix__?

brancz avatar May 09 '22 15:05 brancz

I originally wanted to scrape both the PR and RELEASE pprofs within one scrape job, which is challenging since they share the same domain.

roidelapluie avatar May 09 '22 15:05 roidelapluie

Makes sense, so you meant __path__ if that existed?

brancz avatar May 10 '22 05:05 brancz

I don't understand this yet, have to do some reading, but the PR has been updated with other suggested changes so that prefixes are supported without relabelling for now.

importhuman avatar May 10 '22 10:05 importhuman

I read the Prometheus documentation for relabelling here and here, and think I have a good enough idea of what relabelling does.

I'm unclear about its implementation here though.

Is the following example a Prometheus configuration file? Because while static_configs has been defined in Parca, according to the main parca.yaml file and tests it is supposed to be in ScrapeConfig but doesn't seem to be utilised there.

static_configs:
- targets: [prometheus-pr, prometheus-release]
relabel_configs:
 - source_labels: [__address__]
   target_label: __pprof_prefix__
   replacement: /10543/$1/
 - source_labels: [__address__]
   target_label: instance
 - target_label: __address__
   replacement: prombench.prometheus.io

Also, as this is regarding modifying the scrape configs, does it mean the path inside memory_total should be considered as a label? Or are these some other labels we're talking about?

    profiling_config:                                                      
      pprof_config:
       memory_total:        
               enabled: true                                               
               path: /10543/prometheus-pr/debug/pprof/allocs               
       block_total:         
               enabled: true                                               
               path: /10543/prometheus-pr/debug/pprof/block                

importhuman avatar May 11 '22 13:05 importhuman

Is the following example a Prometheus configuration file? Because while static_configs has been defined in Parca, according to the main parca.yaml file and tests it is supposed to be in ScrapeConfig but doesn't seem to be utilised there.

The static configs defined in the Parca repo I think is a remainder of ignoring all the other discovery mechanisms in the beginning, we can probably just use the Prometheus one now.

Also, as this is regarding modifying the scrape configs, does it mean the path inside memory_total should be considered as a label? Or are these some other labels we're talking about?

I think it the idea was to make it a meta label, meaning it starts with __ and thus doesn't end up in the final label-set of the target.

brancz avatar May 31 '22 11:05 brancz

Trying to ensure I have clarity before coding for this.

Taking the example of a Parca config with no prefixing (from the issue description).

    profiling_config:                                                      
      pprof_config:
       memory_total:        
               enabled: true                                               
               path: /10543/prometheus-pr/debug/pprof/allocs               
       block_total:         
               enabled: true                                               
               path: /10543/prometheus-pr/debug/pprof/block                
       goroutine_total:              
               enabled: true                                               
               path: /10543/prometheus-pr/debug/pprof/goroutine            
       mutex_total:                                                        
               enabled: true
               path: /10543/prometheus-pr/debug/pprof/mutex                
       process_cpu:                                                        
               enabled: true
               path: /10543/prometheus-pr/debug/pprof/profile

With the already merged PR, this can be specified as follows.

    profiling_config:
      path_prefix: /10543/prometheus-pr/debug                                   
      pprof_config:
       memory_total:        
               enabled: true                                               
               path: pprof/allocs               
       block_total:         
               enabled: true                                               
               path: pprof/block                
       goroutine_total:              
               enabled: true                                               
               path: pprof/goroutine            
       mutex_total:                                                        
               enabled: true
               path: pprof/mutex                
       process_cpu:                                                        
               enabled: true
               path: pprof/profile

Now, I have modified the example of how the intended relabeling config would look like, to have values that are present in the above config instead (my assumption is that "labels" are referring to PprofProfilingConfig).

static_configs:
- targets: [prometheus-pr]
relabel_configs:
 - source_labels: [memory_total]
   target_label: block_total
   replacement: /10543/$1/
 - source_labels: [memory_total]
   target_label: goroutine_total
 - target_label: memory_total
   replacement: prombench.prometheus.io

Is the idea to

  1. restrict any possible relabeling only to those paths that have prometheus-pr in them? In this case as a prefix is specified, it can modify any path.
  2. Using memory_total's path, replace block_total's path via the given regex (so it would be /10543/pprof/allocs).
  3. Using memory_total's path, replace goroutine_total's path completely.
  4. Replace memory_total's path to be prombench.prometheus.io.

Please let me know how to proceed. Thanks!

importhuman avatar Jun 07 '22 17:06 importhuman

Done in #1022

maxbrunet avatar Oct 21 '22 22:10 maxbrunet