Merge branch 'mwaw/include_constraints_in_metrics_name_suggestion_generator' into 'master'

Include constraints in suggested metric names [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!55755

lib/gitlab/usage/metrics/names_suggestions/generator.rb

@@ -15,11 +15,11 @@ module Gitlab
             private
 
             def count(relation, column = nil, batch: true, batch_size: nil, start: nil, finish: nil)
-              "count_#{parse_target_and_source(column, relation)}"
+              name_suggestion(column: column, relation: relation, prefix: 'count')
             end
 
             def distinct_count(relation, column = nil, batch: true, batch_size: nil, start: nil, finish: nil)
-              "count_distinct_#{parse_target_and_source(column, relation)}"
+              name_suggestion(column: column, relation: relation, prefix: 'count_distinct', distinct: :distinct)
             end
 
             def redis_usage_counter
@@ -35,22 +35,67 @@ module Gitlab
             end
 
             def sum(relation, column, *rest)
-              "sum_#{parse_target_and_source(column, relation)}"
+              name_suggestion(column: column, relation: relation, prefix: 'sum')
             end
 
             def estimate_batch_distinct_count(relation, column = nil, *rest)
-              "estimate_distinct_#{parse_target_and_source(column, relation)}"
+              name_suggestion(column: column, relation: relation, prefix: 'estimate_distinct_count')
             end
 
             def add(*args)
               "add_#{args.join('_and_')}"
             end
 
-            def parse_target_and_source(column, relation)
+            def name_suggestion(relation:, column: nil, prefix: nil, distinct: nil)
+              parts = [prefix]
+
               if column
-                "#{column}_from_#{relation.table_name}"
+                parts << parse_target(column)
+                parts << 'from'
+              end
+
+              source = parse_source(relation)
+              constraints = parse_constraints(relation: relation, column: column, distinct: distinct)
+
+              if constraints.include?(source)
+                parts << "<adjective describing: '#{constraints}'>"
+              end
+
+              parts << source
+              parts.compact.join('_')
+            end
+
+            def parse_constraints(relation:, column: nil, distinct: nil)
+              connection = relation.connection
+              ::Gitlab::Usage::Metrics::NamesSuggestions::RelationParsers::Constraints
+                .new(connection)
+                .accept(arel(relation: relation, column: column, distinct: distinct), collector(connection))
+                .value
+            end
+
+            def parse_target(column)
+              if column.is_a?(Arel::Attribute)
+                "#{column.relation.name}.#{column.name}"
+              else
+                column
+              end
+            end
+
+            def parse_source(relation)
+              relation.table_name
+            end
+
+            def collector(connection)
+              Arel::Collectors::SubstituteBinds.new(connection, Arel::Collectors::SQLString.new)
+            end
+
+            def arel(relation:, column: nil, distinct: nil)
+              column ||= relation.primary_key
+
+              if column.is_a?(Arel::Attribute)
+                relation.select(column.count(distinct)).arel
               else
-                relation.table_name
+                relation.select(relation.all.table[column].count(distinct)).arel
               end
             end
           end

lib/gitlab/usage/metrics/names_suggestions/relation_parsers/constraints.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Usage
+    module Metrics
+      module NamesSuggestions
+        module RelationParsers
+          class Constraints < ::Arel::Visitors::PostgreSQL
+            # rubocop:disable Naming/MethodName
+            def visit_Arel_Nodes_SelectCore(object, collector)
+              collect_nodes_for(object.wheres, collector, "") || collector
+            end
+            # rubocop:enable Naming/MethodName
+
+            def quote(value)
+              "#{value}"
+            end
+
+            def quote_table_name(name)
+              "#{name}"
+            end
+
+            def quote_column_name(name)
+              "#{name}"
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/usage/metrics/names_suggestions/generator_spec.rb

@@ -10,39 +10,57 @@ RSpec.describe Gitlab::Usage::Metrics::NamesSuggestions::Generator do
   end
 
   describe '#generate' do
-    context 'for count metrics' do
+    shared_examples 'name suggestion' do
       it 'return correct name' do
-        expect(described_class.generate('counts.boards')).to eq 'count_boards'
+        expect(described_class.generate(key_path)).to eq name_suggestion
       end
     end
 
-    context 'for count distinct metrics' do
-      it 'return correct name' do
-        expect(described_class.generate('counts.issues_using_zoom_quick_actions')).to eq 'count_distinct_issue_id_from_zoom_meetings'
+    context 'for count with default column metrics' do
+      it_behaves_like 'name suggestion' do
+        # corresponding metric is collected with count(Board)
+        let(:key_path) { 'counts.boards' }
+        let(:name_suggestion) { 'count_boards' }
+      end
+    end
+
+    context 'for count distinct with column defined metrics' do
+      it_behaves_like 'name suggestion' do
+        # corresponding metric is collected with distinct_count(ZoomMeeting, :issue_id)
+        let(:key_path) { 'counts.issues_using_zoom_quick_actions' }
+        let(:name_suggestion) { 'count_distinct_issue_id_from_zoom_meetings' }
       end
     end
 
     context 'for sum metrics' do
-      it 'return correct name' do
-        expect(described_class.generate('counts.jira_imports_total_imported_issues_count')).to eq 'sum_imported_issues_count_from_jira_imports'
+      it_behaves_like 'name suggestion' do
+        # corresponding metric is collected with sum(JiraImportState.finished, :imported_issues_count)
+        let(:key_path) { 'counts.jira_imports_total_imported_issues_count' }
+        let(:name_suggestion) { "sum_imported_issues_count_from_<adjective describing: '(jira_imports.status = 4)'>_jira_imports" }
       end
     end
 
     context 'for add metrics' do
-      it 'return correct name' do
-        expect(described_class.generate('counts.snippets')).to eq 'add_count_snippets_and_count_snippets'
+      it_behaves_like 'name suggestion' do
+        # corresponding metric is collected with add(data[:personal_snippets], data[:project_snippets])
+        let(:key_path) { 'counts.snippets' }
+        let(:name_suggestion) { "add_count_<adjective describing: '(snippets.type = 'PersonalSnippet')'>_snippets_and_count_<adjective describing: '(snippets.type = 'ProjectSnippet')'>_snippets" }
       end
     end
 
     context 'for redis metrics' do
-      it 'return correct name' do
-        expect(described_class.generate('analytics_unique_visits.analytics_unique_visits_for_any_target')).to eq '<please fill metric name>'
+      it_behaves_like 'name suggestion' do
+        # corresponding metric is collected with redis_usage_data { unique_visit_service.unique_visits_for(targets: :analytics) }
+        let(:key_path) { 'analytics_unique_visits.analytics_unique_visits_for_any_target' }
+        let(:name_suggestion) { '<please fill metric name>' }
       end
     end
 
     context 'for alt_usage_data metrics' do
-      it 'return correct name' do
-        expect(described_class.generate('settings.operating_system')).to eq '<please fill metric name>'
+      it_behaves_like 'name suggestion' do
+        # corresponding metric is collected with alt_usage_data(fallback: nil) { operating_system }
+        let(:key_path) { 'settings.operating_system' }
+        let(:name_suggestion) { '<please fill metric name>' }
       end
     end
   end

spec/lib/gitlab/usage/metrics/names_suggestions/relation_parsers/constraints_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Usage::Metrics::NamesSuggestions::RelationParsers::Constraints do
+  describe '#accept' do
+    let(:collector) { Arel::Collectors::SubstituteBinds.new(ActiveRecord::Base.connection, Arel::Collectors::SQLString.new) }
+
+    it 'builds correct constraints description' do
+      table = Arel::Table.new('records')
+      arel = table.from.project(table['id'].count).where(table[:attribute].eq(true).and(table[:some_value].gt(5)))
+      described_class.new(ApplicationRecord.connection).accept(arel, collector)
+
+      expect(collector.value).to eql '(records.attribute = true AND records.some_value > 5)'
+    end
+  end
+end