Reapply "Fix: Handle duplicate validation correctly when sanitizing (#4238)"

[simonswine]

This introduces #4238 again, it does break the OTLP validation integration tests.

@marcsanmi if you got the chance, please take a look at it.

Fixes https://github.com/grafana/pyroscope-squad/issues/480

[marcsanmi]

@simonswine Please take a look when you're back. It seems the we weren't keeping the labels properly ordered after updating them.

[simonswine]

Thanks for picking that up, I do think it fairly hard to read and not entirely sure if its correct. Let's simplfy this as I mentioned in the comments.

[marcsanmi]

@simonswine I addressed all the concerns, lmkwyt

[simonswine]

Can't approve as it's my PR, but I would approve if I could.

Thanks for revisiting this ❤️

[alsoba13]

LGTM,

I'm wondering how would be the performance impact of using map[string]string to handle duplicates, instead of doing slice tricks such as creating new slices and binary search them. I guess it's just fine in both ways as probably we are dealing with a relatively small amount of labels. Edit: also most common scenario would be 0 slice tricks (no renaming labels), so introducing maps here would be punishing

[marcsanmi]

I'm wondering how would be the performance impact of using map[string]string to handle duplicates, instead of doing slice tricks such as creating new slices and binary search them. I guess it's just fine in both ways as probably we are dealing with a relatively small amount of labels. Edit: also most common scenario would be 0 slice tricks (no renaming labels), so introducing maps here would be punishing

I think labels must stay sorted for consistent fingerprints (Labels.Hash() processes sequentially), so even with a map for duplicates, we'd still need binary search for correct insertion positioning. The current approach uses the sorted slice for both duplicate detection and positioning, eliminating an additional data structure, lmkwyt.