From 86cbef4df42efae51732e01070dfad456dc5c1b5 Mon Sep 17 00:00:00 2001 From: Ben Kochie Date: Wed, 12 Jul 2017 14:48:46 +0200 Subject: [PATCH] Add unit to latency metrics. Add `seconds` unit to latency metrics based on uptream naming convention[0]. [0]: https://prometheus.io/docs/practices/naming/#metric-names --- .../monitoring/prometheus/gitlab_metrics.md | 10 +++++----- lib/gitlab/health_checks/fs_shards_check.rb | 6 +++--- .../health_checks/simple_abstract_check.rb | 2 +- spec/controllers/metrics_controller_spec.rb | 16 ++++++++-------- .../health_checks/fs_shards_check_spec.rb | 18 +++++++++--------- .../health_checks/simple_check_shared.rb | 6 +++--- 6 files changed, 29 insertions(+), 29 deletions(-) diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 07c05b5a6fb..56aa191941b 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -27,15 +27,15 @@ In this experimental phase, only a few metrics are available: | ------ | ---- | ----------- | | db_ping_timeout | Gauge | Whether or not the last database ping timed out | | db_ping_success | Gauge | Whether or not the last database ping succeeded | -| db_ping_latency | Gauge | Round trip time of the database ping | +| db_ping_latency_seconds | Gauge | Round trip time of the database ping | | redis_ping_timeout | Gauge | Whether or not the last redis ping timed out | | redis_ping_success | Gauge | Whether or not the last redis ping succeeded | -| redis_ping_latency | Gauge | Round trip time of the redis ping | -| filesystem_access_latency | gauge | Latency in accessing a specific filesystem | +| redis_ping_latency_seconds | Gauge | Round trip time of the redis ping | +| filesystem_access_latency_seconds | gauge | Latency in accessing a specific filesystem | | filesystem_accessible | gauge | Whether or not a specific filesystem is accessible | -| filesystem_write_latency | gauge | Write latency of a specific filesystem | +| filesystem_write_latency_seconds | gauge | Write latency of a specific filesystem | | filesystem_writable | gauge | Whether or not the filesystem is writable | -| filesystem_read_latency | gauge | Read latency of a specific filesystem | +| filesystem_read_latency_seconds | gauge | Read latency of a specific filesystem | | filesystem_readable | gauge | Whether or not the filesystem is readable | | user_sessions_logins | Counter | Counter of how many users have logged in | diff --git a/lib/gitlab/health_checks/fs_shards_check.rb b/lib/gitlab/health_checks/fs_shards_check.rb index 70da4080cae..bebde857b16 100644 --- a/lib/gitlab/health_checks/fs_shards_check.rb +++ b/lib/gitlab/health_checks/fs_shards_check.rb @@ -35,9 +35,9 @@ module Gitlab repository_storages.flat_map do |storage_name| tmp_file_path = tmp_file_path(storage_name) [ - operation_metrics(:filesystem_accessible, :filesystem_access_latency, -> { storage_stat_test(storage_name) }, shard: storage_name), - operation_metrics(:filesystem_writable, :filesystem_write_latency, -> { storage_write_test(tmp_file_path) }, shard: storage_name), - operation_metrics(:filesystem_readable, :filesystem_read_latency, -> { storage_read_test(tmp_file_path) }, shard: storage_name) + operation_metrics(:filesystem_accessible, :filesystem_access_latency_seconds, -> { storage_stat_test(storage_name) }, shard: storage_name), + operation_metrics(:filesystem_writable, :filesystem_write_latency_seconds, -> { storage_write_test(tmp_file_path) }, shard: storage_name), + operation_metrics(:filesystem_readable, :filesystem_read_latency_seconds, -> { storage_read_test(tmp_file_path) }, shard: storage_name) ].flatten end end diff --git a/lib/gitlab/health_checks/simple_abstract_check.rb b/lib/gitlab/health_checks/simple_abstract_check.rb index fbe1645c1b1..3dcb28a193c 100644 --- a/lib/gitlab/health_checks/simple_abstract_check.rb +++ b/lib/gitlab/health_checks/simple_abstract_check.rb @@ -20,7 +20,7 @@ module Gitlab [ metric("#{metric_prefix}_timeout", result.is_a?(Timeout::Error) ? 1 : 0), metric("#{metric_prefix}_success", is_successful?(result) ? 1 : 0), - metric("#{metric_prefix}_latency", elapsed) + metric("#{metric_prefix}_latency_seconds", elapsed) ] end end diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index 86847c07c09..8964d89b438 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -24,7 +24,7 @@ describe MetricsController do expect(response.body).to match(/^db_ping_timeout 0$/) expect(response.body).to match(/^db_ping_success 1$/) - expect(response.body).to match(/^db_ping_latency [0-9\.]+$/) + expect(response.body).to match(/^db_ping_latency_seconds [0-9\.]+$/) end it 'returns Redis ping metrics' do @@ -32,7 +32,7 @@ describe MetricsController do expect(response.body).to match(/^redis_ping_timeout 0$/) expect(response.body).to match(/^redis_ping_success 1$/) - expect(response.body).to match(/^redis_ping_latency [0-9\.]+$/) + expect(response.body).to match(/^redis_ping_latency_seconds [0-9\.]+$/) end it 'returns Caching ping metrics' do @@ -40,7 +40,7 @@ describe MetricsController do expect(response.body).to match(/^redis_cache_ping_timeout 0$/) expect(response.body).to match(/^redis_cache_ping_success 1$/) - expect(response.body).to match(/^redis_cache_ping_latency [0-9\.]+$/) + expect(response.body).to match(/^redis_cache_ping_latency_seconds [0-9\.]+$/) end it 'returns Queues ping metrics' do @@ -48,7 +48,7 @@ describe MetricsController do expect(response.body).to match(/^redis_queues_ping_timeout 0$/) expect(response.body).to match(/^redis_queues_ping_success 1$/) - expect(response.body).to match(/^redis_queues_ping_latency [0-9\.]+$/) + expect(response.body).to match(/^redis_queues_ping_latency_seconds [0-9\.]+$/) end it 'returns SharedState ping metrics' do @@ -56,17 +56,17 @@ describe MetricsController do expect(response.body).to match(/^redis_shared_state_ping_timeout 0$/) expect(response.body).to match(/^redis_shared_state_ping_success 1$/) - expect(response.body).to match(/^redis_shared_state_ping_latency [0-9\.]+$/) + expect(response.body).to match(/^redis_shared_state_ping_latency_seconds [0-9\.]+$/) end it 'returns file system check metrics' do get :index - expect(response.body).to match(/^filesystem_access_latency{shard="default"} [0-9\.]+$/) + expect(response.body).to match(/^filesystem_access_latency_seconds{shard="default"} [0-9\.]+$/) expect(response.body).to match(/^filesystem_accessible{shard="default"} 1$/) - expect(response.body).to match(/^filesystem_write_latency{shard="default"} [0-9\.]+$/) + expect(response.body).to match(/^filesystem_write_latency_seconds{shard="default"} [0-9\.]+$/) expect(response.body).to match(/^filesystem_writable{shard="default"} 1$/) - expect(response.body).to match(/^filesystem_read_latency{shard="default"} [0-9\.]+$/) + expect(response.body).to match(/^filesystem_read_latency_seconds{shard="default"} [0-9\.]+$/) expect(response.body).to match(/^filesystem_readable{shard="default"} 1$/) end diff --git a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb index b333e162909..3de73a9ff65 100644 --- a/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb +++ b/spec/lib/gitlab/health_checks/fs_shards_check_spec.rb @@ -109,9 +109,9 @@ describe Gitlab::HealthChecks::FsShardsCheck do expect(subject).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) expect(subject).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency, value: be >= 0)) + expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) + expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) + expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) end end @@ -127,9 +127,9 @@ describe Gitlab::HealthChecks::FsShardsCheck do expect(subject).to include(an_object_having_attributes(name: :filesystem_readable, value: 1)) expect(subject).to include(an_object_having_attributes(name: :filesystem_writable, value: 1)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency, value: be >= 0)) + expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) + expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) + expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) end end end @@ -159,9 +159,9 @@ describe Gitlab::HealthChecks::FsShardsCheck do expect(subject).to include(an_object_having_attributes(name: :filesystem_readable, value: 0)) expect(subject).to include(an_object_having_attributes(name: :filesystem_writable, value: 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency, value: be >= 0)) - expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency, value: be >= 0)) + expect(subject).to include(an_object_having_attributes(name: :filesystem_access_latency_seconds, value: be >= 0)) + expect(subject).to include(an_object_having_attributes(name: :filesystem_read_latency_seconds, value: be >= 0)) + expect(subject).to include(an_object_having_attributes(name: :filesystem_write_latency_seconds, value: be >= 0)) end end end diff --git a/spec/lib/gitlab/health_checks/simple_check_shared.rb b/spec/lib/gitlab/health_checks/simple_check_shared.rb index 1abebeac4dd..e2643458aca 100644 --- a/spec/lib/gitlab/health_checks/simple_check_shared.rb +++ b/spec/lib/gitlab/health_checks/simple_check_shared.rb @@ -8,7 +8,7 @@ shared_context 'simple_check' do |metrics_prefix, check_name, success_result| it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 1)) } it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 0)) } - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be >= 0)) } + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency_seconds", value: be >= 0)) } end context 'Check is misbehaving' do @@ -18,7 +18,7 @@ shared_context 'simple_check' do |metrics_prefix, check_name, success_result| it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 0)) } it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 0)) } - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be >= 0)) } + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency_seconds", value: be >= 0)) } end context 'Check is timeouting' do @@ -28,7 +28,7 @@ shared_context 'simple_check' do |metrics_prefix, check_name, success_result| it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_success", value: 0)) } it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_timeout", value: 1)) } - it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency", value: be >= 0)) } + it { is_expected.to include(have_attributes(name: "#{metrics_prefix}_latency_seconds", value: be >= 0)) } end end