Merge branch '332487-add-redis-metric-instrumentation-class' into 'master'

Add RedisMetric instrumentation class

See merge request gitlab-org/gitlab!66582

config/metrics/counts_all/20210216182006_source_code_pushes.yml

@@ -10,6 +10,10 @@ value_type: number
 status: data_available
 time_frame: all
 data_source: redis
+instrumentation_class: RedisMetric
+options:
+  counter_class: SourceCodeCounter
+  event: pushes
 distribution:
 - ce
 - ee

doc/development/service_ping/metrics_instrumentation.md

@@ -11,7 +11,7 @@ This guide describes how to develop Service Ping metrics using metrics instrumen
 ## Nomenclature
 
 - **Instrumentation class**:
-  - Inherits one of the metric classes: `DatabaseMetric`, `RedisHLLMetric` or `GenericMetric`.
+  - Inherits one of the metric classes: `DatabaseMetric`, `RedisMetric`, `RedisHLLMetric` or `GenericMetric`.
   - Implements the logic that calculates the value for a Service Ping metric.
 
 - **Metric definition**
@@ -24,7 +24,7 @@ This guide describes how to develop Service Ping metrics using metrics instrumen
 
 A metric definition has the [`instrumentation_class`](metrics_dictionary.md) field, which can be set to a class.
 
-The defined instrumentation class should have one of the existing metric classes: `DatabaseMetric`, `RedisHLLMetric`, or `GenericMetric`.
+The defined instrumentation class should have one of the existing metric classes: `DatabaseMetric`, `RedisMetric`, `RedisHLLMetric`, or `GenericMetric`.
 
 Using the instrumentation classes ensures that metrics can fail safe individually, without breaking the entire
  process of Service Ping generation.
@@ -51,6 +51,26 @@ module Gitlab
 end
 ```
 
+## Redis metrics
+
+[Example of a merge request that adds a `Redis` metric](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/66582).
+
+Count unique values for `source_code_pushes` event.
+
+Required options:
+
+- `event`: the event name.
+- `counter_class`: one of the counter classes from the `Gitlab::UsageDataCounters` namespace; it should implement `read` method or inherit it from `BaseCounter`.
+
+```yaml
+time_frame: all
+data_source: redis
+instrumentation_class: 'RedisMetric'
+options:
+  event: pushes
+  counter_class: SourceCodeCounter
+```
+
 ## Redis HyperLogLog metrics
 
 [Example of a merge request that adds a `RedisHLL` metric](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61685).
@@ -60,7 +80,7 @@ Count unique values for `i_quickactions_approve` event.
 ```yaml
 time_frame: 28d
 data_source: redis_hll
-instrumentation_class: 'Gitlab::Usage::Metrics::Instrumentations::RedisHLLMetric'
+instrumentation_class: 'RedisHLLMetric'
 options:
   events:
     - i_quickactions_approve
@@ -91,13 +111,13 @@ end
 There is support for:
 
 - `count`, `distinct_count` for [database metrics](#database-metrics).
+- [Redis metrics](#redis-metrics).
 - [Redis HLL metrics](#redis-hyperloglog-metrics).
 - [Generic metrics](#generic-metrics), which are metrics based on settings or configurations.
 
 Currently, there is no support for:
 
 - `add`, `sum`, `histogram`, `estimate_batch_distinct_count` for database metrics.
-- Regular Redis counters.
 
 You can [track the progress to support these](https://gitlab.com/groups/gitlab-org/-/epics/6118).
 
@@ -107,7 +127,7 @@ To create a stub instrumentation for a Service Ping metric, you can use a dedica
 
 The generator takes the class name as an argument and the following options:
 
-- `--type=TYPE` Required. Indicates the metric type. It must be one of: `database`, `generic`.
+- `--type=TYPE` Required. Indicates the metric type. It must be one of: `database`, `generic`, `redis`.
 - `--operation` Required for `database` type. It must be one of: `count`, `distinct_count`.
 - `--ee` Indicates if the metric is for EE.
 

lib/generators/gitlab/usage_metric/usage_metric_generator.rb

@@ -11,7 +11,8 @@ module Gitlab
 
     ALLOWED_SUPERCLASSES = {
       generic: 'Generic',
-      database: 'Database'
+      database: 'Database',
+      redis: 'Redis'
     }.freeze
 
     ALLOWED_OPERATIONS = %w(count distinct_count).freeze

lib/gitlab/usage/metrics/instrumentations/redis_metric.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Usage
+    module Metrics
+      module Instrumentations
+        # Usage example
+        #
+        # In metric YAML definition:
+        #
+        # instrumentation_class: RedisMetric
+        # options:
+        #   event: pushes
+        #   counter_class: SourceCodeCounter
+        #
+        class RedisMetric < BaseMetric
+          def initialize(time_frame:, options: {})
+            super
+
+            raise ArgumentError, "'event' option is required" unless metric_event.present?
+            raise ArgumentError, "'counter class' option is required" unless counter_class.present?
+          end
+
+          def metric_event
+            options[:event]
+          end
+
+          def counter_class_name
+            options[:counter_class]
+          end
+
+          def counter_class
+            "Gitlab::UsageDataCounters::#{counter_class_name}".constantize
+          end
+
+          def value
+            redis_usage_data do
+              counter_class.read(metric_event)
+            end
+          end
+
+          def suggested_name
+            Gitlab::Usage::Metrics::NameSuggestion.for(:redis)
+          end
+        end
+      end
+    end
+  end
+end

rubocop/rubocop-usage-data.yml

@@ -122,3 +122,4 @@ UsageData/InstrumentationSuperclass:
     - :DatabaseMetric
     - :GenericMetric
     - :RedisHLLMetric
+    - :RedisMetric

spec/lib/gitlab/usage/metrics/instrumentations/redis_metric_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Usage::Metrics::Instrumentations::RedisMetric, :clean_gitlab_redis_shared_state do
+  before do
+    4.times do
+      Gitlab::UsageDataCounters::SourceCodeCounter.count(:pushes)
+    end
+  end
+
+  let(:expected_value) { 4 }
+
+  it_behaves_like 'a correct instrumented metric value', { options: { event: 'pushes', counter_class: 'SourceCodeCounter' } }
+
+  it 'raises an exception if event option is not present' do
+    expect { described_class.new(counter_class: 'SourceCodeCounter') }.to raise_error(ArgumentError)
+  end
+
+  it 'raises an exception if counter_class option is not present' do
+    expect { described_class.new(event: 'pushes') }.to raise_error(ArgumentError)
+  end
+end