discourse / prometheus_exporter

A framework for collecting and aggregating prometheus metrics
MIT License
538 stars 155 forks source link

NoMethodError when processing metrics #283

Open j05h opened 1 year ago

j05h commented 1 year ago

In PrometheusExporter::Server::Collector, the register_metric_unsafe sometimes returns a nil. Since the metric is used later used, we very commonly receive a NoMethodError. The metrics collection does work, but our logs are filled with exceptions like the following:

#<NoMethodError: undefined method `observe' for nil:NilClass> - /opt/ruby3.0/lib/ruby/gems/3.0.0/gems/prometheus_exporter-0.8.1/lib/prometheus_exporter/server/collector.rb:55:in `block in process_hash'
/opt/ruby3.0/lib/ruby/gems/3.0.0/gems/prometheus_exporter-0.8.1/lib/prometheus_exporter/server/collector.rb:35:in `synchronize'
/opt/ruby3.0/lib/ruby/gems/3.0.0/gems/prometheus_exporter-0.8.1/lib/prometheus_exporter/server/collector.rb:35:in `process_hash'
/opt/ruby3.0/lib/ruby/gems/3.0.0/gems/prometheus_exporter-0.8.1/lib/prometheus_exporter/server/collector.rb:31:in `process'

Is it reasonable to not attempt to even call these methods when the metric is nil? Or raise a different exception which could be handled specifically in that case the metric isn't found so that we can disambiguate from other NoMethodErrors?

https://github.com/discourse/prometheus_exporter/blob/8d77c53333b5793f26d8e90bbeecdab850acc3f8/lib/prometheus_exporter/server/collector.rb#LL44C23-L44C23