Merge branch 'michold-add-instrumentation-availability' into 'master'

Add metric availability to instrumentation classes

See merge request gitlab-org/gitlab!84259

doc/development/service_ping/metrics_instrumentation.md

@@ -40,6 +40,7 @@ We have built a domain-specific language (DSL) to define the metrics instrumenta
 - `start`: Specifies the start value of the batch counting, by default is `relation.minimum(:id)`.
 - `finish`: Specifies the end value of the batch counting, by default is `relation.maximum(:id)`.
 - `cache_start_and_finish_as`: Specifies the cache key for `start` and `finish` values and sets up caching them. Use this call when `start` and `finish` are expensive queries that should be reused between different metric calculations.
+- `available?`: Specifies whether the metric should be reported. The default is `true`.
 
 [Example of a merge request that adds a database metric](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60022).
 
@@ -123,6 +124,37 @@ options:
   counter_class: SourceCodeCounter
 ```
 
+### Availability-restrained Redis metrics
+
+If the Redis metric should only be available in the report under some conditions, then you must specify these conditions in a new class that is a child of the `RedisMetric` class.
+
+```ruby
+# frozen_string_literal: true
+
+module Gitlab
+  module Usage
+    module Metrics
+      module Instrumentations
+        class MergeUsageCountRedisMetric < RedisMetric
+          available? { Feature.enabled?(:merge_usage_data_missing_key_paths) }
+        end
+      end
+    end
+  end
+end
+```
+
+You must also use the class's name in the YAML setup.
+
+```yaml
+time_frame: all
+data_source: redis
+instrumentation_class: 'MergeUsageCountRedisMetric'
+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).
@@ -138,8 +170,42 @@ options:
     - i_quickactions_approve
 ```
 
+### Availability-restrained Redis HyperLogLog metrics
+
+If the Redis HyperLogLog metric should only be available in the report under some conditions, then you must specify these conditions in a new class that is a child of the `RedisHLLMetric` class.
+
+```ruby
+# frozen_string_literal: true
+
+module Gitlab
+  module Usage
+    module Metrics
+      module Instrumentations
+        class MergeUsageCountRedisHLLMetric < RedisHLLMetric
+          available? { Feature.enabled?(:merge_usage_data_missing_key_paths) }
+        end
+      end
+    end
+  end
+end
+```
+
+You must also use the class's name in the YAML setup.
+
+```yaml
+time_frame: 28d
+data_source: redis_hll
+instrumentation_class: 'MergeUsageCountRedisHLLMetric'
+options:
+  events:
+    - i_quickactions_approve
+```
+
 ## Generic metrics
 
+- `value`: Specifies the value of the metric.
+- `available?`: Specifies whether the metric should be reported. The default is `true`.
+
 [Example of a merge request that adds a generic metric](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60256).
 
 ```ruby

ee/spec/config/metrics/every_metric_definition_spec.rb

@@ -104,6 +104,7 @@ def assert_uses_all_nested_classes(parent_module)
         if constant.is_a? Class
           metric_class_instance = instance_double(constant)
           expect(constant).to receive(:new).at_least(:once).and_return(metric_class_instance)
+          allow(metric_class_instance).to receive(:available?).and_return(true)
           expect(metric_class_instance).to receive(:value).at_least(:once)
         elsif constant.is_a? Module
           assert_uses_all_nested_classes(constant)

lib/gitlab/usage/metric.rb

@@ -18,19 +18,25 @@ def all
       end
 
       def with_value
-        unflatten_key_path(intrumentation_object.value)
+        with_availability(proc { instrumentation_object.value })
       end
 
       def with_instrumentation
-        unflatten_key_path(intrumentation_object.instrumentation)
+        with_availability(proc { instrumentation_object.instrumentation })
       end
 
       def with_suggested_name
-        unflatten_key_path(intrumentation_object.suggested_name)
+        with_availability(proc { instrumentation_object.suggested_name })
       end
 
       private
 
+      def with_availability(value_proc)
+        return {} unless instrumentation_object.available?
+
+        unflatten_key_path(value_proc.call)
+      end
+
       def unflatten_key_path(value)
         ::Gitlab::Usage::Metrics::KeyPathProcessor.process(definition.key_path, value)
       end
@@ -39,8 +45,8 @@ def instrumentation_class
         "Gitlab::Usage::Metrics::Instrumentations::#{definition.instrumentation_class}"
       end
 
-      def intrumentation_object
-        instrumentation_class.constantize.new(
+      def instrumentation_object
+        @instrumentation_object ||= instrumentation_class.constantize.new(
           time_frame: definition.time_frame,
           options: definition.attributes[:options]
         )

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

@@ -11,6 +11,18 @@ class BaseMetric
           attr_reader :time_frame
           attr_reader :options
 
+          class << self
+            def available?(&block)
+              return @metric_available = block if block_given?
+
+              return @metric_available.call if instance_variable_defined?('@metric_available')
+
+              true
+            end
+
+            attr_reader :metric_available
+          end
+
           def initialize(time_frame:, options: {})
             @time_frame = time_frame
             @options = options
@@ -19,6 +31,10 @@ def initialize(time_frame:, options: {})
           def instrumentation
             value
           end
+
+          def available?
+            self.class.available?
+          end
         end
       end
     end

spec/lib/gitlab/usage/metric_spec.rb

@@ -51,4 +51,31 @@
       expect(described_class.new(issue_count_metric_definiton).with_suggested_name).to eq({ counts: { issues: 'count_issues' } })
     end
   end
+
+  context 'unavailable metric' do
+    let(:instrumentation_class) { "UnavailableMetric" }
+    let(:issue_count_metric_definiton) do
+      double(:issue_count_metric_definiton,
+        attributes.merge({ attributes: attributes, instrumentation_class: instrumentation_class })
+      )
+    end
+
+    before do
+      unavailable_metric_class = Class.new(Gitlab::Usage::Metrics::Instrumentations::CountIssuesMetric) do
+        def available?
+          false
+        end
+      end
+
+      stub_const("Gitlab::Usage::Metrics::Instrumentations::#{instrumentation_class}", unavailable_metric_class)
+    end
+
+    [:with_value, :with_instrumentation, :with_suggested_name].each do |method_name|
+      describe "##{method_name}" do
+        it 'returns an empty hash' do
+          expect(described_class.new(issue_count_metric_definiton).public_send(method_name)).to eq({})
+        end
+      end
+    end
+  end
 end

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

@@ -71,6 +71,33 @@
       end
     end
 
+    context 'with availability defined' do
+      subject do
+        described_class.tap do |metric_class|
+          metric_class.relation { Issue }
+          metric_class.operation :count
+          metric_class.available? { false }
+        end.new(time_frame: 'all')
+      end
+
+      it 'responds to #available? properly' do
+        expect(subject.available?).to eq(false)
+      end
+    end
+
+    context 'with availability not defined' do
+      subject do
+        Class.new(described_class) do
+          relation { Issue }
+          operation :count
+        end.new(time_frame: 'all')
+      end
+
+      it 'responds to #available? properly' do
+        expect(subject.available?).to eq(true)
+      end
+    end
+
     context 'with cache_start_and_finish_as called' do
       subject do
         described_class.tap do |metric_class|

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

@@ -25,4 +25,28 @@
   it 'raise exception if events options is not present' do
     expect { described_class.new(time_frame: '28d') }.to raise_error(ArgumentError)
   end
+
+  describe 'children classes' do
+    let(:options) { { events: ['i_quickactions_approve'] } }
+
+    context 'availability not defined' do
+      subject { Class.new(described_class).new(time_frame: nil, options: options) }
+
+      it 'returns default availability' do
+        expect(subject.available?).to eq(true)
+      end
+    end
+
+    context 'availability defined' do
+      subject do
+        Class.new(described_class) do
+          available? { false }
+        end.new(time_frame: nil, options: options)
+      end
+
+      it 'returns defined availability' do
+        expect(subject.available?).to eq(false)
+      end
+    end
+  end
 end

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

@@ -20,4 +20,28 @@
   it 'raises an exception if counter_class option is not present' do
     expect { described_class.new(event: 'pushes') }.to raise_error(ArgumentError)
   end
+
+  describe 'children classes' do
+    let(:options) { { event: 'pushes', counter_class: 'SourceCodeCounter' } }
+
+    context 'availability not defined' do
+      subject { Class.new(described_class).new(time_frame: nil, options: options) }
+
+      it 'returns default availability' do
+        expect(subject.available?).to eq(true)
+      end
+    end
+
+    context 'availability defined' do
+      subject do
+        Class.new(described_class) do
+          available? { false }
+        end.new(time_frame: nil, options: options)
+      end
+
+      it 'returns defined availability' do
+        expect(subject.available?).to eq(false)
+      end
+    end
+  end
 end