Skip to content

Commit

Permalink
review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
cyriltovena authored and thampiotr committed Oct 19, 2023
1 parent c9ff6e0 commit 7789619
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 15 deletions.
9 changes: 0 additions & 9 deletions component/pyroscope/scrape/scrape.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,21 +97,12 @@ type ProfilingConfig struct {
Custom []CustomProfilingTarget `river:"profile.custom,block,optional"`

PprofPrefix string `river:"path_prefix,attr,optional"`

allTargets map[string]ProfilingTarget
}

// AllTargets returns the set of all standard and custom profiling targets,
// regardless of whether they're enabled. The key in the map indicates the name
// of the target.
func (cfg *ProfilingConfig) AllTargets() map[string]ProfilingTarget {
if cfg.allTargets == nil {
cfg.allTargets = cfg.buildAllTargets()
}
return cfg.allTargets
}

func (cfg ProfilingConfig) buildAllTargets() map[string]ProfilingTarget {
targets := map[string]ProfilingTarget{
pprofMemory: cfg.Memory,
pprofBlock: cfg.Block,
Expand Down
4 changes: 2 additions & 2 deletions component/pyroscope/scrape/scrape_loop.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ func newScrapePool(cfg Arguments, appendable pyroscope.Appendable, logger log.Lo
func (tg *scrapePool) sync(groups []*targetgroup.Group) {
tg.mtx.Lock()
defer tg.mtx.Unlock()

allTarget := tg.config.ProfilingConfig.AllTargets()
level.Info(tg.logger).Log("msg", "syncing target groups", "job", tg.config.JobName)
var actives []*Target
tg.droppedTargets = tg.droppedTargets[:0]
for _, group := range groups {
targets, dropped, err := targetsFromGroup(group, tg.config)
targets, dropped, err := targetsFromGroup(group, tg.config, allTarget)
if err != nil {
level.Error(tg.logger).Log("msg", "creating targets failed", "err", err)
continue
Expand Down
4 changes: 2 additions & 2 deletions component/pyroscope/scrape/target.go
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ func populateLabels(lset labels.Labels, cfg Arguments) (res, orig labels.Labels,
}

// targetsFromGroup builds targets based on the given TargetGroup and config.
func targetsFromGroup(group *targetgroup.Group, cfg Arguments) ([]*Target, []*Target, error) {
func targetsFromGroup(group *targetgroup.Group, cfg Arguments, targetTypes map[string]ProfilingTarget) ([]*Target, []*Target, error) {
var (
targets = make([]*Target, 0, len(group.Targets))
droppedTargets = make([]*Target, 0, len(group.Targets))
Expand Down Expand Up @@ -404,7 +404,7 @@ func targetsFromGroup(group *targetgroup.Group, cfg Arguments) ([]*Target, []*Ta
params = url.Values{}
}

if pcfg, found := cfg.ProfilingConfig.AllTargets()[profType]; found && pcfg.Delta {
if pcfg, found := targetTypes[profType]; found && pcfg.Delta {
params.Add("seconds", strconv.Itoa(int((cfg.ScrapeInterval)/time.Second)-1))
}
targets = append(targets, NewTarget(lbls, origLabels, params))
Expand Down
4 changes: 2 additions & 2 deletions component/pyroscope/scrape/target_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func Test_targetsFromGroup(t *testing.T) {
Labels: model.LabelSet{
"foo": "bar",
},
}, args)
}, args, args.ProfilingConfig.AllTargets())
expected := []*Target{
// unspecified
NewTarget(
Expand Down Expand Up @@ -68,7 +68,7 @@ func Test_targetsFromGroup(t *testing.T) {
}),
url.Values{"seconds": []string{"14"}}),

//specified
// specified
NewTarget(
labels.FromMap(map[string]string{
model.AddressLabel: "localhost:9091",
Expand Down

0 comments on commit 7789619

Please sign in to comment.