Feature: Do not overwrite metric labels by server group labels if present

[FUSAKLA]

Hi, I'd like to propose a feature to disable overwriting labels of metric by the specified labels of server group.

The reason for this is I use it as an alternative to external_labels of Prometheus. BUT those do not overwrite the label if metrics already has it.

Off course, I don't expect the behavior as is to change, that would be a breaking change, instead I'm thinking about adding some option like labels_do_not_overwrite: <bool> with default to false

The particular code is here if I'm not mistaken, where it should be sufficient to check if the label is there and if so to continue

promxy/pkg/promhttputil/merge.go

Line 47 in a956fcb

func ValueAddLabelSet(a model.Value, l model.LabelSet) error {

I'd be happy to send a PR if we agree on the implementation.

Thanks for consideration in advance!
Also for the project at all :)

[FUSAKLA]

Hm.. looking more into the code I see the Lables is also used to filter queries send to the server group which would not be possible anymore this way

maybe instad add option NonOverwritingLabels map[string]string or ExternalLabels which would not be considered in the macthers

ten could be used in the dynamic label filters if user would like to? 🤔

[FUSAKLA]

Just for the record… Thanos has the same issue thanos-io/thanos#1579

Apparently I did not think this through from the first glance, this may be way more tricky :/

[FUSAKLA]

Just for context, we use Mimir as a primary solution for global view etc., but we want to have a backup solution for cases Mimir is down or remote_write is falling behind, and we don't have fresh data.

The idea is to have a second datasource in grafana which we can use as drop-in replacement for the Mimir one, meaning we need it to have identical label schema ideally, and this causes differences in the labels (since remote write adds the external labels but does not overwrite the existing)

[jacksontj]

This is an interesting use-case; thanks for bringing it up. I'm not sure what the correct approach here is either on first glance.

Upstream thanos seems to have closed the issue saying they won't implement -- which seems "safe".

For your PR the idea of adding another config option for this IMO makes sense; although the approach for the label filter doesn't seem correct-- as in this case we can't definitively scope out any downstreams from querying -- so I think we'd need to not touch filters that match the external labels (so they are always passed downstream) -- then we'd have to ensure they get filtered after the merge step (which I don't think we do today).

So this is definitely possible; although will require some more work on the PR to get there.