pushing metrics with instance grouping_key would result: instance is reserved

[LeoQuote]

def push_metrics(registry)
  hostname = `hostname`.strip
  puts "pusing metrics"
  require 'prometheus/client/push'
  Prometheus::Client::Push.new(
    job: 'check_puppet',
    gateway: 'https://pushgateway.mydomain.com',
    grouping_key: { instance: hostname }
  ).add(registry)
end

/usr/local/lib64/ruby/gems/2.7.0/gems/prometheus-client-4.0.0/lib/prometheus/client/label_set_validator.rb:77:in `validate_reserved_key': instance is reserved (Prometheus::Client::LabelSetValidator::ReservedLabelError)
        from /usr/local/lib64/ruby/gems/2.7.0/gems/prometheus-client-4.0.0/lib/prometheus/client/label_set_validator.rb:32:in `block in validate_symbols!'
        from /usr/local/lib64/ruby/gems/2.7.0/gems/prometheus-client-4.0.0/lib/prometheus/client/label_set_validator.rb:29:in `each'
        from /usr/local/lib64/ruby/gems/2.7.0/gems/prometheus-client-4.0.0/lib/prometheus/client/label_set_validator.rb:29:in `all?'
        from /usr/local/lib64/ruby/gems/2.7.0/gems/prometheus-client-4.0.0/lib/prometheus/client/label_set_validator.rb:29:in `validate_symbols!'
        from /usr/local/lib64/ruby/gems/2.7.0/gems/prometheus-client-4.0.0/lib/prometheus/client/push.rb:35:in `initialize'

instance should be allowed , why this is not allowed even in pushgateway?

[Sinjo]

Hi @LeoQuote! Thanks for bringing this up. You're absolutely right that it should be allowed. The fact that it isn't is an oversight on my part between commits 1eb1c9ac6196b3bbb8f07277edcf32b7b32f9e59 and 2b3310d5b808080a5dd2907b5fbf28beee640a81.

My intention was to treat instance like any other optional grouping key label, rather than having special code to handle it. When I later added validation for the grouping keys, I reused our LabelSetValidator without realising that we treat instance as a reserved label for pull-based collection (which is correct).

I don't think #276 is the right fix, as we do want to treat it as a reserved label in all other contexts. I'm going to have a think about how to solve this. My initial thought is an extra parameter in the constructor of LabelSetValidator to override the base list of reserved labels, but I'm going to think about whether there's a neater option before implementing that.

[Sinjo]

Okay, I've done some reading, and I'm less sure I'm right now.

It looks like the Go and Python clients don't reserve the instance label at all. They also don't reserve job, which surprised me. I'm starting to wonder if we should drop both of them from the reserved labels list.

I'd love to make pid more situational too, as that only needs to be reserved if the metric is a gauge being stored in a DirectFileStore, but we'd have to break an abstraction there and I think it would feel really clunky for users.

I'm going to chat with the Prometheus team and see what they think. I suspect the answer is going to bring us closer to the other clients.

For reference:

• The relevant code in the Python client checks that you don't use any invalid characters and you don't use a __ prefix.
• The relevant code in the Go client does the same thing, though by importing some common code.

[LeoQuote]

I’m in favor of the implementation in go and python client , which is don’t explicitly reserve any label. It should be user who decide whether or not use those labels and these can be respected or not respected with “honor labels” field in scrape config https://prometheus.io/docs/prometheus/latest/configuration/configuration/

[Sinjo]

Totally agree.

I'm pretty confident we can stop reserving job and instance. pid will be trickier, but I'm going to see what we can do. It might not all happen at once.

[bboreham]

Note the default behaviour of Prometheus is to rename job and instance to exported_job and exported_instance.

honor_labels changes this behaviour, but was intended for cases where they have the same meaning as the internally-generated labels.

# Setting honor_labels to "true" is useful for use cases such as federation and
# scraping the Pushgateway, where all labels specified in the target should be
# preserved.

[Sinjo]

@bboreham Cheers for the clarification. I've replied on the mailing list thread with my plan, and I'm going to do the easy first part now.