Merge branch '332954-build-usage-data-queries-using-instrumentation-classes' into 'master'

Build usage data queries and usage data non sql metrics using instrumentation classes

See merge request gitlab-org/gitlab!67049

ee/spec/lib/ee/gitlab/usage_data_non_sql_metrics_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe Gitlab::UsageDataNonSqlMetrics do
         described_class.uncached_data
       end
 
-      expect(recorder.count).to eq(55)
+      expect(recorder.count).to eq(56)
     end
   end
 end

lib/gitlab/usage/metric.rb

@@ -3,40 +3,43 @@
 module Gitlab
   module Usage
     class Metric
-      include ActiveModel::Model
+      attr_reader :definition
 
-      InvalidMetricError = Class.new(RuntimeError)
-
-      attr_accessor :key_path, :value
+      def initialize(definition)
+        @definition = definition
+      end
 
-      validates :key_path, presence: true
+      class << self
+        def all
+          @all ||= Gitlab::Usage::MetricDefinition.with_instrumentation_class.map do |definition|
+            self.new(definition)
+          end
+        end
+      end
 
-      def definition
-        self.class.definitions[key_path]
+      def with_value
+        unflatten_key_path(intrumentation_object.value)
       end
 
-      def unflatten_key_path
-        unflatten(key_path.split('.'), value)
+      def with_instrumentation
+        unflatten_key_path(intrumentation_object.instrumentation)
       end
 
-      class << self
-        def definitions
-          @definitions ||= Gitlab::Usage::MetricDefinition.definitions
-        end
+      private
 
-        def dictionary
-          definitions.map { |key, definition| definition.to_dictionary }
-        end
+      def unflatten_key_path(value)
+        ::Gitlab::Usage::Metrics::KeyPathProcessor.process(definition.key_path, value)
       end
 
-      private
+      def instrumentation_class
+        "Gitlab::Usage::Metrics::Instrumentations::#{definition.instrumentation_class}"
+      end
 
-      def unflatten(keys, value)
-        loop do
-          value = { keys.pop.to_sym => value }
-          break if keys.blank?
-        end
-        value
+      def intrumentation_object
+        instrumentation_class.constantize.new(
+          time_frame: definition.time_frame,
+          options: definition.attributes[:options]
+        )
       end
     end
   end

lib/gitlab/usage/metric_definition.rb

@@ -7,6 +7,8 @@ module Gitlab
       BASE_REPO_PATH = 'https://gitlab.com/gitlab-org/gitlab/-/blob/master'
       SKIP_VALIDATION_STATUSES = %w[deprecated removed].to_set.freeze
 
+      InvalidError = Class.new(RuntimeError)
+
       attr_reader :path
       attr_reader :attributes
 
@@ -48,7 +50,7 @@ module Gitlab
               Metric file: #{path}
             ERROR_MSG
 
-            Gitlab::ErrorTracking.track_and_raise_for_dev_exception(Gitlab::Usage::Metric::InvalidMetricError.new(error_message))
+            Gitlab::ErrorTracking.track_and_raise_for_dev_exception(InvalidError.new(error_message))
           end
         end
       end
@@ -69,6 +71,10 @@ module Gitlab
           @all ||= definitions.map { |_key_path, definition| definition }
         end
 
+        def with_instrumentation_class
+          all.select { |definition| definition.attributes[:instrumentation_class].present? }
+        end
+
         def schemer
           @schemer ||= ::JSONSchemer.schema(Pathname.new(METRIC_SCHEMA_PATH))
         end
@@ -92,7 +98,7 @@ module Gitlab
 
           self.new(path, definition).tap(&:validate!)
         rescue StandardError => e
-          Gitlab::ErrorTracking.track_and_raise_for_dev_exception(Gitlab::Usage::Metric::InvalidMetricError.new(e.message))
+          Gitlab::ErrorTracking.track_and_raise_for_dev_exception(InvalidError.new(e.message))
         end
 
         def load_all_from_path!(definitions, glob_path)
@@ -100,7 +106,7 @@ module Gitlab
             definition = load_from_file(path)
 
             if previous = definitions[definition.key]
-              Gitlab::ErrorTracking.track_and_raise_for_dev_exception(Gitlab::Usage::Metric::InvalidMetricError.new("Metric '#{definition.key}' is already defined in '#{previous.path}'"))
+              Gitlab::ErrorTracking.track_and_raise_for_dev_exception(InvalidError.new("Metric '#{definition.key}' is already defined in '#{previous.path}'"))
             end
 
             definitions[definition.key] = definition

lib/gitlab/usage_data_metrics.rb

@@ -5,26 +5,7 @@ module Gitlab
     class << self
       # Build the Usage Ping JSON payload from metrics YAML definitions which have instrumentation class set
       def uncached_data
-        ::Gitlab::Usage::MetricDefinition.all.map do |definition|
-          instrumentation_class = definition.attributes[:instrumentation_class]
-          options = definition.attributes[:options]
-
-          if instrumentation_class.present?
-            metric_value = "Gitlab::Usage::Metrics::Instrumentations::#{instrumentation_class}".constantize.new(
-              time_frame: definition.attributes[:time_frame],
-              options: options).value
-
-            metric_payload(definition.key_path, metric_value)
-          else
-            {}
-          end
-        end.reduce({}, :deep_merge)
-      end
-
-      private
-
-      def metric_payload(key_path, value)
-        ::Gitlab::Usage::Metrics::KeyPathProcessor.process(key_path, value)
+        ::Gitlab::Usage::Metric.all.map(&:with_value).reduce({}, :deep_merge)
       end
     end
   end

lib/gitlab/usage_data_non_sql_metrics.rb

@@ -5,6 +5,10 @@ module Gitlab
     SQL_METRIC_DEFAULT = -3
 
     class << self
+      def uncached_data
+        super.with_indifferent_access.deep_merge(instrumentation_metrics_queries.with_indifferent_access)
+      end
+
       def add_metric(metric, time_frame: 'none')
         metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
 
@@ -43,6 +47,12 @@ module Gitlab
           projects_jira_cloud_active: 0
         }
       end
+
+      private
+
+      def instrumentation_metrics_queries
+        ::Gitlab::Usage::Metric.all.map(&:with_instrumentation).reduce({}, :deep_merge)
+      end
     end
   end
 end

lib/gitlab/usage_data_queries.rb

@@ -5,6 +5,10 @@ module Gitlab
   # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41091
   class UsageDataQueries < UsageData
     class << self
+      def uncached_data
+        super.with_indifferent_access.deep_merge(instrumentation_metrics_queries.with_indifferent_access)
+      end
+
       def add_metric(metric, time_frame: 'none')
         metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
 
@@ -64,6 +68,12 @@ module Gitlab
       def epics_deepest_relationship_level
         { epics_deepest_relationship_level: 0 }
       end
+
+      private
+
+      def instrumentation_metrics_queries
+        ::Gitlab::Usage::Metric.all.map(&:with_instrumentation).reduce({}, :deep_merge)
+      end
     end
   end
 end

spec/lib/gitlab/usage/metric_definition_spec.rb

@@ -87,14 +87,14 @@ RSpec.describe Gitlab::Usage::MetricDefinition do
       end
 
       it 'raise exception' do
-        expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::Metric::InvalidMetricError))
+        expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::MetricDefinition::InvalidError))
 
         described_class.new(path, attributes).validate!
       end
 
       context 'with skip_validation' do
         it 'raise exception if skip_validation: false' do
-          expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::Metric::InvalidMetricError))
+          expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::MetricDefinition::InvalidError))
 
           described_class.new(path, attributes.merge( { skip_validation: false } )).validate!
         end
@@ -113,7 +113,7 @@ RSpec.describe Gitlab::Usage::MetricDefinition do
           attributes[:status] = 'broken'
           attributes.delete(:repair_issue_url)
 
-          expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::Metric::InvalidMetricError))
+          expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).at_least(:once).with(instance_of(Gitlab::Usage::MetricDefinition::InvalidError))
 
           described_class.new(path, attributes).validate!
         end
@@ -173,7 +173,7 @@ RSpec.describe Gitlab::Usage::MetricDefinition do
       write_metric(metric1, path, yaml_content)
       write_metric(metric2, path, yaml_content)
 
-      expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).with(instance_of(Gitlab::Usage::Metric::InvalidMetricError))
+      expect(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception).with(instance_of(Gitlab::Usage::MetricDefinition::InvalidError))
 
       subject
     end

spec/lib/gitlab/usage/metric_spec.rb

@@ -3,27 +3,46 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Usage::Metric do
-  describe '#definition' do
-    it 'returns key_path metric definiton' do
-      expect(described_class.new(key_path: 'uuid').definition).to be_an(Gitlab::Usage::MetricDefinition)
-    end
+  let!(:issue) { create(:issue) }
+
+  let(:attributes) do
+    {
+      data_category: "Operational",
+      key_path: "counts.issues",
+      description: "Count of Issues created",
+      product_section: "dev",
+      product_stage: "plan",
+      product_group: "group::plan",
+      product_category: "issue_tracking",
+      value_type: "number",
+      status: "data_available",
+      time_frame: "all",
+      data_source: "database",
+      instrumentation_class: "CountIssuesMetric",
+      distribution: %w(ce ee),
+      tier: %w(free premium ultimate)
+    }
   end
 
-  describe '#unflatten_default_path' do
-    using RSpec::Parameterized::TableSyntax
+  let(:issue_count_metric_definiton) do
+    double(:issue_count_metric_definiton,
+      attributes.merge({ attributes: attributes })
+    )
+  end
 
-    where(:key_path, :value, :expected_hash) do
-      'uuid'                                     | nil    | { uuid: nil }
-      'uuid'                                     | '1111' | { uuid: '1111' }
-      'counts.issues'                            | nil    | { counts: { issues: nil } }
-      'counts.issues'                            | 100    | { counts: { issues: 100 } }
-      'usage_activity_by_stage.verify.ci_builds' | 100    | { usage_activity_by_stage: { verify: { ci_builds: 100 } } }
-    end
+  before do
+    allow(ApplicationRecord.connection).to receive(:transaction_open?).and_return(false)
+  end
 
-    with_them do
-      subject { described_class.new(key_path: key_path, value: value).unflatten_key_path }
+  describe '#with_value' do
+    it 'returns key_path metric with the corresponding value' do
+      expect(described_class.new(issue_count_metric_definiton).with_value).to eq({ counts: { issues: 1 } })
+    end
+  end
 
-      it { is_expected.to eq(expected_hash) }
+  describe '#with_instrumentation' do
+    it 'returns key_path metric with the corresponding generated query' do
+      expect(described_class.new(issue_count_metric_definiton).with_instrumentation).to eq({ counts: { issues: "SELECT COUNT(\"issues\".\"id\") FROM \"issues\"" } })
     end
   end
 end