FEATURE: allow passing extra labels to process metrics

approved
#1

FEATURE: allow passing extra labels to process metrics

Also corrects issue where collector was not scoping on hostname causing issues with multi-host and multi-container metric collection

Fixes: #53, #54

diff --git a/CHANGELOG b/CHANGELOG
index 4e2914d..b5886f7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,8 @@
+0.4.4 - 14-02-2019
+
+- Feature: Allow process collector to ship custom labels for all process metrics
+- Fix: Always scope process metrics on hostname in collector
+
 0.4.3 - 11-02-2019
 
 - Feature: Add alias for Gauge #observe called #set, this makes it a bit easier to migrate from prom
diff --git a/README.md b/README.md
index e46af4e..c025ba7 100644
--- a/README.md
+++ b/README.md
@@ -68,7 +68,7 @@ server.start
 PrometheusExporter::Client.default = PrometheusExporter::LocalClient.new(collector: server.collector)
 
 # this ensures basic process instrumentation metrics are added such as RSS and Ruby metrics
-PrometheusExporter::Instrumentation::Process.start
+PrometheusExporter::Instrumentation::Process.start(type: "my program", labels: {my_custom: "label for all process metrics"})
 
 gauge = PrometheusExporter::Metric::Gauge.new("rss", "used RSS for process")
 counter = PrometheusExporter::Metric::Counter.new("web_requests", "number of web requests")
diff --git a/lib/prometheus_exporter/instrumentation/process.rb b/lib/prometheus_exporter/instrumentation/process.rb
index df1f93b..3d79002 100644
--- a/lib/prometheus_exporter/instrumentation/process.rb
+++ b/lib/prometheus_exporter/instrumentation/process.rb
@@ -3,8 +3,18 @@
 # collects stats from currently running process
 module PrometheusExporter::Instrumentation
   class Process
-    def self.start(client: nil, type: "ruby", frequency: 30)
-      process_collector = new(type)
+    def self.start(client: nil, type: "ruby", frequency: 30, labels: nil)
+
+      metric_labels =
+        if labels && type
+          labels.merge(type: type)
+        elsif labels
+          labels
+        else
+          { type: type }
+        end
+
+      process_collector = new(metric_labels)
       client ||= PrometheusExporter::Client.default
 
       stop if @thread
@@ -30,14 +40,26 @@ module PrometheusExporter::Instrumentation
       end
     end
 
-    def initialize(type)
-      @type = type
+    def initialize(metric_labels)
+      @metric_labels = metric_labels
+      @hostname = nil
+    end
+
+    def hostname
+      @hostname ||=
+        begin
+          `hostname`.strip
+        rescue => e
+          STDERR.puts "Unable to lookup hostname #{e}"
+          "unknown-host"
+        end
     end
 
     def collect
       metric = {}
       metric[:type] = "process"
-      metric[:process_type] = @type
+      metric[:metric_labels] = @metric_labels
+      metric[:hostname] = hostname
       collect_gc_stats(metric)
       collect_v8_stats(metric)
       collect_process_stats(metric)
diff --git a/lib/prometheus_exporter/server/process_collector.rb b/lib/prometheus_exporter/server/process_collector.rb
index b8f13c2..fd84f15 100644
--- a/lib/prometheus_exporter/server/process_collector.rb
+++ b/lib/prometheus_exporter/server/process_collector.rb
@@ -34,7 +34,7 @@ module PrometheusExporter::Server
       metrics = {}
 
       @process_metrics.map do |m|
-        metric_key = { pid: m["pid"], type: m["process_type"] }
+        metric_key = m["metric_labels"].merge("pid" => m["pid"])
 
         PROCESS_GAUGES.map do |k, help|
           k = k.to_s
@@ -62,8 +62,10 @@ module PrometheusExporter::Server
       obj["created_at"] = now
 
       @process_metrics.delete_if do |current|
-        obj["pid"] == current["pid"] || (current["created_at"] + MAX_PROCESS_METRIC_AGE < now)
+        (obj["pid"] == current["pid"] && obj["hostname"] == current["hostname"]) ||
+          (current["created_at"] + MAX_PROCESS_METRIC_AGE < now)
       end
+
       @process_metrics << obj
     end
   end
diff --git a/lib/prometheus_exporter/version.rb b/lib/prometheus_exporter/version.rb
index 47dc38f..73efc4e 100644
--- a/lib/prometheus_exporter/version.rb
+++ b/lib/prometheus_exporter/version.rb
@@ -1,3 +1,3 @@
 module PrometheusExporter
-  VERSION = "0.4.4"
+  VERSION = "0.4.5"
 end
diff --git a/test/server/collector_test.rb b/test/server/collector_test.rb
index 9a43360..c5591ee 100644
--- a/test/server/collector_test.rb
+++ b/test/server/collector_test.rb
@@ -26,7 +26,7 @@ class PrometheusCollectorTest < Minitest::Test
     collector = PrometheusExporter::Server::Collector.new
     client = PrometheusExporter::LocalClient.new(collector: collector)
 
-    PrometheusExporter::Instrumentation::Process.start(client: client)
+    PrometheusExporter::Instrumentation::Process.start(client: client, labels: { hello: "custom label" })
 
     metrics_text = ""
     TestHelper.wait_for(2) do
@@ -37,6 +37,7 @@ class PrometheusCollectorTest < Minitest::Test
     PrometheusExporter::Instrumentation::Process.stop
 
     assert(metrics_text.match?(/heap_live_slots/))
+    assert(metrics_text.match?(/hello.*custom label/))
   end
 
   def test_register_metric
@@ -197,14 +198,15 @@ class PrometheusCollectorTest < Minitest::Test
 
     collector = PrometheusExporter::Server::Collector.new
 
-    process_instrumentation = PrometheusExporter::Instrumentation::Process.new(:web)
+    process_instrumentation = PrometheusExporter::Instrumentation::Process.new(type: "web")
     collected = process_instrumentation.collect
 
     collector.process(collected.to_json)
 
     text = collector.prometheus_metrics_text
 
-    v8_str = "v8_heap_count{pid=\"#{collected[:pid]}\",type=\"web\"} #{collected[:v8_heap_count]}"
+    v8_str = "v8_heap_count{type=\"web\",pid=\"#{collected[:pid]}\"} #{collected[:v8_heap_count]}"
+
     assert(text.include?(v8_str), "must include v8 metric")
     assert(text.include?("minor_gc_ops_total"), "must include counters")
   end

GitHub sha: 32c32508

#2

Why not include “hostname” in the metric_key? As it is internally used to identify a metric, it would be more transparent to also expose it externally.

#3

using metric[:custom_labels] here would allow to use the metrics set on the client as advised here: GitHub - discourse/prometheus_exporter: A framework for collecting and aggregating prometheus metrics

#4

using metric_key = m["custom_labels"].merge("pid" => m["pid"]).merge("hostname" => m["hostname"])here solved all my issues

#5

When you start the instrumentation you can take care of that:

PrometheusExporter::Instrumentation::Process.start(labels: { hostname: `hostname`.strip })

I opted to leave the hostname out cause I find multi host collection to be a much rares use case, so I opted for safety out of the box + an easy workaround

#6

Fair enough, thank you for the explanation.

Approved #7