From 45ff6535a0a737880e1f199ca87056272e460d14 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Fri, 14 Feb 2020 18:01:07 +1100 Subject: [PATCH] FIX: avoid shelling to get hostname Previously we were using `hostname` to get hostname, this is slow and risks orphan processes. This swaps it with Socket.gethostname which is both faster and more reliable. We also now cache it on the PrometheusExporter instance --- CHANGELOG | 3 ++- lib/prometheus_exporter.rb | 11 +++++++++++ .../instrumentation/active_record.rb | 13 +------------ lib/prometheus_exporter/instrumentation/process.rb | 13 +------------ test/prometheus_exporter_test.rb | 4 ++++ 5 files changed, 19 insertions(+), 25 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index f850ec94..fb396b0d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ -0.5.0 - pending +0.5.0 - 14-02-2019 - Breaking change: listen only to localhost by default to prevent unintended insecure configuration +- FIX: Avoid calling `hostname` aggressively, instead cache it on the exporter instance 0.4.17 - 13-01-2019 diff --git a/lib/prometheus_exporter.rb b/lib/prometheus_exporter.rb index ea8b6282..ddcf7eb0 100644 --- a/lib/prometheus_exporter.rb +++ b/lib/prometheus_exporter.rb @@ -20,6 +20,17 @@ def self.dump(obj) end end + def self.hostname + @hostname ||= + begin + require 'socket' + Socket.gethostname + rescue => e + STDERR.puts "Unable to lookup hostname #{e}" + "unknown-host" + end + end + def self.detect_json_serializer(preferred) if preferred.nil? preferred = :oj if has_oj? diff --git a/lib/prometheus_exporter/instrumentation/active_record.rb b/lib/prometheus_exporter/instrumentation/active_record.rb index de59020c..174f9a3a 100644 --- a/lib/prometheus_exporter/instrumentation/active_record.rb +++ b/lib/prometheus_exporter/instrumentation/active_record.rb @@ -51,17 +51,6 @@ def self.stop def initialize(metric_labels, config_labels) @metric_labels = metric_labels @config_labels = config_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 @@ -87,7 +76,7 @@ def collect_active_record_pool_stats(metrics) metric = { pid: pid, type: "active_record", - hostname: hostname, + hostname: ::PrometheusExporter.hostname, metric_labels: labels } metric.merge!(pool.stat) diff --git a/lib/prometheus_exporter/instrumentation/process.rb b/lib/prometheus_exporter/instrumentation/process.rb index 3d79002e..bf891e3a 100644 --- a/lib/prometheus_exporter/instrumentation/process.rb +++ b/lib/prometheus_exporter/instrumentation/process.rb @@ -42,24 +42,13 @@ def self.stop 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[:metric_labels] = @metric_labels - metric[:hostname] = hostname + metric[:hostname] = ::PrometheusExporter.hostname collect_gc_stats(metric) collect_v8_stats(metric) collect_process_stats(metric) diff --git a/test/prometheus_exporter_test.rb b/test/prometheus_exporter_test.rb index f1c42a23..7638b5c8 100644 --- a/test/prometheus_exporter_test.rb +++ b/test/prometheus_exporter_test.rb @@ -6,4 +6,8 @@ class PrometheusExporterTest < Minitest::Test def test_that_it_has_a_version_number refute_nil ::PrometheusExporter::VERSION end + + def test_it_can_get_hostname + assert_equal `hostname`.strip, ::PrometheusExporter.hostname + end end