file_sd never stop update 'custom_sd.json' file

[earthdiaosi]

Proposal

Use case. Why is this important?
I want to implement custom service discovery, so i read the follow blog and wirte a demo;

• https://prometheus.io/blog/2018/07/05/implementing-custom-sd/

but i found "custom_sd.json" file was updated each time. so i debug the follow code, and find key and i is different each time:

// Parses incoming target groups updates. If the update contains changes to the target groups
// Adapter already knows about, or new target groups, we Marshal to JSON and write to file.
func (a *Adapter) generateTargetGroups(allTargetGroups map[string][]*targetgroup.Group) {
	tempGroups := make(map[string]*customSD)
	for k, sdTargetGroups := range allTargetGroups {
		for i, group := range sdTargetGroups {
			newTargets := make([]string, 0)
			newLabels := make(map[string]string)

			for _, targets := range group.Targets {
				for _, target := range targets {
					newTargets = append(newTargets, string(target))
				}
			}

			for name, value := range group.Labels {
				newLabels[string(name)] = string(value)
			}
			// Make a unique key, including the current index, in case the sd_type (map key) and group.Source is not unique.
			key := fmt.Sprintf("%s:%s:%d", k, group.Source, i)
			tempGroups[key] = &customSD{
				Targets: newTargets,
				Labels:  newLabels,
			}
		}
	}
	if !reflect.DeepEqual(a.groups, tempGroups) {
		a.groups = tempGroups
		err := a.writeOutput()
		if err != nil {
			level.Error(log.With(a.logger, "component", "sd-adapter")).Log("err", err)
		}
	}

}

then i search code, fount the reason: the tsets is a map, so the results are different for each iteration;

func (m *Manager) allGroups() map[string][]*targetgroup.Group {
	m.mtx.Lock()
	defer m.mtx.Unlock()

	tSets := map[string][]*targetgroup.Group{}
	for pkey, tsets := range m.targets {
		for _, tg := range tsets {
			// Even if the target group 'tg' is empty we still need to send it to the 'Scrape manager'
			// to signal that it needs to stop all scrape loops for this target set.
			tSets[pkey.setName] = append(tSets[pkey.setName], tg)
		}
	}
	return tSets
}

solution: sort map by key

func (m *Manager) allGroups() map[string][]*targetgroup.Group {
	m.mtx.Lock()
	defer m.mtx.Unlock()

	tSets := map[string][]*targetgroup.Group{}
	for pkey, tsets := range m.targets {
		//sort tsets
		keys := make([]string, 0, len(tsets))
		for key := range tsets {
			keys = append(keys, key)
		}
		sort.Strings(keys)
		for _, k := range keys {
			// Even if the target group 'tg' is empty we still need to send it to the 'Scrape manager'
			// to signal that it needs to stop all scrape loops for this target set.
			tSets[pkey.setName] = append(tSets[pkey.setName], tsets[k])
		}
	}
	return tSets

Bug Report

What did you do?
Write code to implement custom service discovery;

What did you expect to see?
Do not update file "custom_sd.json" when no changes;

What did you see instead? Under which circumstances?
i found "custom_sd.json" file was updated each time.

Environment

• System information:

  Windows7 64bit

• Prometheus version:

  2.3.1

[gouthamve]

/cc @cstyan

[earthdiaosi]

I would like to submit this PR

[gouthamve]

Hi, thanks a lot for offering, please go ahead. I just linked Callum as he was the one who wrote the code and has most context.

[cstyan]

Hi @earthdiaosi can you show us what your main.go looks like for your custom-sd? The adapter code is written to specifically avoid this case of rewriting custom-sd.json if there were no changes to the targets, and I'm not able to reproduce what you're seeing. The index and key you pointed out do not change.

Additionally, if there is a bug it's in the adapter code. Discovery manager should not have to be modified.

[earthdiaosi]

i write a demo to reproduce the bug:

func (d *discovery) Run(ctx context.Context, ch chan<- []*targetgroup.Group) {
	for c := time.Tick(time.Duration(d.refreshInterval) * time.Second); ; {
		tgroups := []*targetgroup.Group{}
		for i := 1; i <= 100; i++ {
			iStr := fmt.Sprintf("%d", i)
			tgroup := &targetgroup.Group{
				Targets: make([]model.LabelSet, 0),
				Labels: model.LabelSet{
					model.LabelName("product"): model.LabelValue("test" + iStr),
					model.LabelName("cluster"): model.LabelValue("test" + iStr),
				},
				Source: "source" + iStr,
			}
			for j := 1; j <= 3; j++ {
				ip := fmt.Sprintf("192.168.%d.%d", i, j)
				tgroup.Targets = append(tgroup.Targets, model.LabelSet{
					model.AddressLabel: model.LabelValue(ip + ":80"),
				})
			}
			tgroups = append(tgroups, tgroup)
		}
		ch <- tgroups
		select {
		case <-c:
			continue
		case <-ctx.Done():
			return
		}
	}
}

ouput:
first:

second:

third:

[cstyan]

Thanks for the example! I was able to find and fix the bug.

See: #4555

[earthdiaosi]

@cstyan #4555 Some checks were not successful.

reason: cannot find package "github.com/mitchellh/hashstructure"

[cstyan]

@earthdiaosi Yeah! So I'm not sure if there's a workaround for that, or if the fix is to just vendor that package. Depends what the team wants to do.

Are you able to check out my changes and try them with your custom SD implementation? Let me know if they don't work for you and we can look at it more together!

[brian-brazil]

I'd avoid using that package, it's not something we use elsewhere and we should try to keep this example reasonably standard.

[cstyan]

@earthdiaosi if you'd like, you could take my open PR and make the changes to make the hashing of the structures that contain maps consistent without using the hashstructure library (see: https://github.com/mitchellh/hashstructure/blob/master/hashstructure.go#L177-L214), or if you can find another way to check if things have changed and only send updates if they have then go for it!

Changes should be to the code in the example dir (adapter.go) rather than in the discovery package.

[earthdiaosi]

@cstyan I debug the code of hashstructure.go, but the code is too heavy. So I wrote a simplified version of the hash function for customSD。

Algorithm logic：

• 1 targetsHash = init ^ hash(targets[0]) ^ hash(targets[1]) ^ ... ^ hash(targets[n-1])
• 2 sort labels
• 3 labelsHash = init ^ hash(labels[0].(k:v)) ^ hash(labels[1].(k:v)) ^ ... ^ hash(labels[n-1].(k:v))
• 4 return targetsHash.Sum64() ^ labelsHash.Sum64(), nil

[cstyan]

@earthdiaosi awesome, can you add that functionality to adapter.go? feel free to create a new branch off mine from #4555.

[earthdiaosi]

@cstyan I have submitted the PR.

See: #4567