From 442f59917708d663b7dac36560dd174dc8fbc2d2 Mon Sep 17 00:00:00 2001 From: Ben Kochie Date: Fri, 19 Jul 2019 13:34:04 +0000 Subject: Adjust redis cache metrics * Remove `controller` and `action` labels from duration histogram. * Create a new simple counter for `controller` and `action`. * Adjust histogram buckets to observe smaller response times. --- changelogs/unreleased/bjk-64064_cache_metrics.yml | 5 +++++ .../monitoring/prometheus/gitlab_metrics.md | 3 +++ lib/gitlab/metrics/subscribers/rails_cache.rb | 15 ++++++++++++--- spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb | 10 +++++++++- 4 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 changelogs/unreleased/bjk-64064_cache_metrics.yml diff --git a/changelogs/unreleased/bjk-64064_cache_metrics.yml b/changelogs/unreleased/bjk-64064_cache_metrics.yml new file mode 100644 index 00000000000..c9baff7cb7b --- /dev/null +++ b/changelogs/unreleased/bjk-64064_cache_metrics.yml @@ -0,0 +1,5 @@ +--- +title: Adjust redis cache metrics +merge_request: 30572 +author: +type: changed diff --git a/doc/administration/monitoring/prometheus/gitlab_metrics.md b/doc/administration/monitoring/prometheus/gitlab_metrics.md index 89501f20d99..054fa547704 100644 --- a/doc/administration/monitoring/prometheus/gitlab_metrics.md +++ b/doc/administration/monitoring/prometheus/gitlab_metrics.md @@ -34,6 +34,9 @@ The following metrics are available: | filesystem_writable | Gauge | 9.4 | Whether or not the filesystem is writable | | filesystem_read_latency_seconds | Gauge | 9.4 | Read latency of a specific filesystem | | filesystem_readable | Gauge | 9.4 | Whether or not the filesystem is readable | +| gitlab_cache_misses_total | Counter | 10.2 | Cache read miss | +| gitlab_cache_operation_duration_seconds | Histogram | 10.2 | Cache access time | +| gitlab_cache_operations_total | Counter | 12.2 | Cache operations by controller/action | | http_requests_total | Counter | 9.4 | Rack request count | | http_request_duration_seconds | Histogram | 9.4 | HTTP response time from rack middleware | | pipelines_created_total | Counter | 9.4 | Counter of pipelines created | diff --git a/lib/gitlab/metrics/subscribers/rails_cache.rb b/lib/gitlab/metrics/subscribers/rails_cache.rb index 01db507761b..2ee7144fe2f 100644 --- a/lib/gitlab/metrics/subscribers/rails_cache.rb +++ b/lib/gitlab/metrics/subscribers/rails_cache.rb @@ -50,7 +50,8 @@ module Gitlab def observe(key, duration) return unless current_transaction - metric_cache_operation_duration_seconds.observe(current_transaction.labels.merge({ operation: key }), duration / 1000.0) + metric_cache_operations_total.increment(current_transaction.labels.merge({ operation: key })) + metric_cache_operation_duration_seconds.observe({ operation: key }, duration / 1000.0) current_transaction.increment(:cache_duration, duration, false) current_transaction.increment(:cache_count, 1, false) current_transaction.increment("cache_#{key}_duration".to_sym, duration, false) @@ -63,12 +64,20 @@ module Gitlab Transaction.current end + def metric_cache_operations_total + @metric_cache_operations_total ||= ::Gitlab::Metrics.counter( + :gitlab_cache_operations_total, + 'Cache operations', + Transaction::BASE_LABELS + ) + end + def metric_cache_operation_duration_seconds @metric_cache_operation_duration_seconds ||= ::Gitlab::Metrics.histogram( :gitlab_cache_operation_duration_seconds, 'Cache access time', - Transaction::BASE_LABELS.merge({ action: nil }), - [0.001, 0.01, 0.1, 1, 10] + {}, + [0.00001, 0.0001, 0.001, 0.01, 0.1, 1.0] ) end diff --git a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb index 6795c1ab56b..e04056b3450 100644 --- a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb +++ b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb @@ -201,7 +201,15 @@ describe Gitlab::Metrics::Subscribers::RailsCache do it 'observes cache metric' do expect(subscriber.send(:metric_cache_operation_duration_seconds)) .to receive(:observe) - .with(transaction.labels.merge(operation: :delete), event.duration / 1000.0) + .with({ operation: :delete }, event.duration / 1000.0) + + subscriber.observe(:delete, event.duration) + end + + it 'increments the operations total' do + expect(subscriber.send(:metric_cache_operations_total)) + .to receive(:increment) + .with(transaction.labels.merge(operation: :delete)) subscriber.observe(:delete, event.duration) end -- cgit v1.2.1