Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sorting API interface output data #3843

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

linuxgcc
Copy link

When the browser accesses silence, the output column Matchers is not sorted.
When the browser accesses alert, the output column Labels is not sorted.
This pr completes the above sorting
#3834

api/v2/compat.go Outdated
}
sort.Strings(keys)
for _, key := range keys {
apiLabelSet[string(key)] = string(modelLabelSet[prometheus_model.LabelName(key)])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! 👋 Thanks for opening a PR, however I'm not sure this works. apiLabelSet is a map which is not sorted, so while you have sorted the keys, when you put them back into apiLabelSet they become unsorted again.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apiLabelSet is an empty map, and the input parameter modelLabelSet is unordered. After sorting the modelLabelSet by key, then use the sorted results to build an apiLabelSet

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but when apiLabelSet is JSON encoded it will be done so in random order because in Go maps are not sorted.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tks for your prompt reply, that's great, but I don't quite understand your question.
If the apiLabelSet map is sorted, the JSON encoded results are also sorted, in the same order as the map。
As for the randomed modelLabelSet, we don't need to pay attention.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the apiLabelSet map is sorted, the JSON encoded results are also sorted, in the same order as the map

That's the issue 🙂 The apiLabelSet map is not sorted. Even though you are inserting keys in a sorted order, they get unsorted by the internal hash function within the map. You cannot sort maps in Go.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion is if you want them to appear sorted in the UI the best place to do it is in the Elm code. You can find it in ui/app.

Copy link
Author

@linuxgcc linuxgcc May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go's map is not sorted, but if the key of the map is of string type, it will be sorted by key when using json.Marshal encoded, so there is no need to sort the map

reference: https://pkg.go.dev/encoding/json#Marshal

Map values encode as JSON objects. The map's key type must either be a string, an integer type, or implement encoding.TextMarshaler. The map keys are sorted and used as JSON object keys by applying the following rules, subject to the UTF-8 coercion described for string values above:

keys of any string type are used directly
encoding.TextMarshalers are marshaled
integer keys are converted to strings

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

json.Marshal sorts maps – I learned something! 😄

Signed-off-by: heyitao <heyitao@uniontech.com>
func (m SMatcher) Len() int { return len(m) }
func (m SMatcher) Swap(i, j int) { m[i], m[j] = m[j], m[i] }
func (m SMatcher) Less(i, j int) bool {
return m[i].Name < m[j].Name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to make this sort stable as Alertmanager does not force name to be unique in silences. For example, foo="bar" and foo=~[a-zA-Z]+ can be used in the same silence. I don't think a lot of users do this, but the sort is non-deterministic when two names are the same.

@@ -52,3 +53,11 @@ func (s ByAddress) Less(i, j int) bool {
}
return bytes.Compare(net.ParseIP(ip1), net.ParseIP(ip2)) < 0
}

type SMatcher []*silencepb.Matcher
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move this to the api package since it's unrelated to the CLI?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants