Merge branch 'am-remove-instrumentation-class-ff' into 'master'

Remove instrumentation class ff

See merge request gitlab-org/gitlab!75345

config/feature_flags/development/usage_data_instrumentation.yml

----
-name: usage_data_instrumentation
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68808
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/345252
-milestone: '14.5'
-type: development
-group: group::product intelligence
-default_enabled: false

ee/spec/config/metrics/every_metric_definition_spec.rb

@@ -87,37 +87,10 @@ RSpec.describe 'Every metric definition' do
     stub_usage_data_connections
   end
 
-  context 'with usage_data_instrumentation feature flag' do
-    let(:msg) { 'see https://docs.gitlab.com/ee/development/service_ping/metrics_dictionary.html#metrics-added-dynamic-to-service-ping-payload' }
-
-    context 'when enabled' do
-      let(:instrumentation_class_keys) do
-        Gitlab::Usage::MetricDefinition.with_instrumentation_class.map(&:key)
-        .reject { |v| v =~ Regexp.union(ignored_usage_ping_key_patterns) }
-      end
-
-      let(:files_key_paths) do
-        (metric_files_key_paths + instrumentation_class_keys).to_set
-      end
-
-      before do
-        stub_feature_flags(usage_data_instrumentation: true)
-      end
-
-      it 'is included in the Usage Ping hash structure', :aggregate_failures do
-        expect(files_key_paths).to eql(usage_ping_key_paths.to_set), msg
-      end
-    end
-
-    context 'when disabled' do
-      before do
-        stub_feature_flags(usage_data_instrumentation: false)
-      end
-
-      it 'is included in the Usage Ping hash structure', :aggregate_failures do
-        expect(metric_files_key_paths).to match_array(usage_ping_key_paths), msg
-      end
-    end
+  it 'is included in the Usage Ping hash structure', :aggregate_failures do
+    msg = "see https://docs.gitlab.com/ee/development/service_ping/metrics_dictionary.html#metrics-added-dynamic-to-service-ping-payload"
+    expect(metric_files_key_paths).to match_array(usage_ping_key_paths)
+    expect(metric_files_key_paths).to match_array(usage_ping_key_paths), msg
   end
 
   context 'with value json schema' do

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(61)
+      expect(recorder.count).to eq(59)
     end
   end
 end

ee/spec/lib/ee/gitlab/usage_data_spec.rb

@@ -62,7 +62,7 @@ RSpec.describe Gitlab::UsageData do
     subject { described_class.data }
 
     it 'gathers usage data' do
-      expect(subject.keys).to include(*%w(
+      expect(subject.keys).to include(*%i(
         historical_max_users
         license_add_ons
         license_plan
@@ -85,7 +85,7 @@ RSpec.describe Gitlab::UsageData do
       expect(count_data[:boards]).to eq(1)
       expect(count_data[:projects]).to eq(3)
 
-      expect(count_data.keys).to include(*%w(
+      expect(count_data.keys).to include(*%i(
         confidential_epics
         container_scanning_jobs
         coverage_fuzzing_jobs
@@ -154,10 +154,6 @@ RSpec.describe Gitlab::UsageData do
   describe '.features_usage_data_ee' do
     subject { described_class.features_usage_data_ee }
 
-    before do
-      stub_feature_flags(usage_data_instrumentation: false)
-    end
-
     it 'gathers feature usage data of EE' do
       expect(subject[:elasticsearch_enabled]).to eq(Gitlab::CurrentSettings.elasticsearch_search?)
       expect(subject[:geo_enabled]).to eq(Gitlab::Geo.enabled?)
@@ -179,39 +175,9 @@ RSpec.describe Gitlab::UsageData do
       expect(subject[:license_trial]).to eq(license.trial?)
       expect(subject[:license_subscription_id]).to eq(license.subscription_id)
       expect(subject[:license_billable_users]).to eq(license.daily_billable_users_count)
-    end
-
-    context 'with usage_data_instrumentation feature flag' do
-      let(:license) { ::License.current }
-
-      context 'when enabled' do
-        before do
-          stub_feature_flags(usage_data_instrumentation: true)
-        end
-
-        it 'returns fallback value to be overriden' do
-          expect(subject[:licensee]).to eq(Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
-          expect(subject[:license_md5]).to eq(Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
-          expect(subject[:historical_max_users]).to eq(Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
-
-          uncached_data = described_class.uncached_data
-          expect(uncached_data[:licensee]).to eq(license.licensee)
-          expect(uncached_data[:license_md5]).to eq(Digest::MD5.hexdigest(license.data))
-          expect(uncached_data[:historical_max_users]).to eq(license.historical_max)
-        end
-      end
-
-      context 'when disabled' do
-        before do
-          stub_feature_flags(usage_data_instrumentation: false)
-        end
-
-        it 'computes historical_max_users, licensee and license_md5 values' do
-          expect(subject[:licensee]).to eq(license.licensee)
-          expect(subject[:license_md5]).to eq(Digest::MD5.hexdigest(license.data))
-          expect(subject[:historical_max_users]).to eq(license.historical_max)
-        end
-      end
+      expect(subject[:licensee]).to eq(license.licensee)
+      expect(subject[:license_md5]).to eq(Digest::MD5.hexdigest(license.data))
+      expect(subject[:historical_max_users]).to eq(license.historical_max)
     end
   end
 
@@ -557,36 +523,8 @@ RSpec.describe Gitlab::UsageData do
     it 'includes accurate usage_activity_by_stage data' do
       expect(described_class.usage_activity_by_stage_release({})).to include(projects_mirrored_with_pipelines_enabled: 2)
       expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include(projects_mirrored_with_pipelines_enabled: 1)
-    end
-
-    context 'with usage_data_instrumentation feature flag' do
-      let(:license) { ::License.current }
-
-      context 'when enabled' do
-        before do
-          stub_feature_flags(usage_data_instrumentation: true)
-        end
-
-        it 'returns fallback value to be overriden' do
-          expect(described_class.usage_activity_by_stage_release({})).to include(releases_with_group_milestones: Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
-          expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include(releases_with_group_milestones: Gitlab::Utils::UsageData::INSTRUMENTATION_CLASS_FALLBACK)
-
-          uncached_data = described_class.uncached_data
-          expect(uncached_data[:usage_activity_by_stage][:release]).to include(releases_with_milestones: 2)
-          expect(uncached_data[:usage_activity_by_stage_monthly][:release]).to include(releases_with_milestones: 1)
-        end
-      end
-
-      context 'when disabled' do
-        before do
-          stub_feature_flags(usage_data_instrumentation: false)
-        end
-
-        it 'computes releases_with_group_milestones values' do
-          expect(described_class.usage_activity_by_stage_release({})).to include(releases_with_group_milestones: 2)
-          expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include(releases_with_group_milestones: 1)
-        end
-      end
+      expect(described_class.usage_activity_by_stage_release({})).to include(releases_with_group_milestones: 2)
+      expect(described_class.usage_activity_by_stage_release(described_class.monthly_time_range_db_params)).to include(releases_with_group_milestones: 1)
     end
   end
 

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

@@ -18,10 +18,6 @@ module Gitlab
 
             private
 
-            def instrumentation_metrics
-              ::Gitlab::UsageDataMetrics.suggested_names
-            end
-
             def count(relation, column = nil, batch: true, batch_size: nil, start: nil, finish: nil)
               Gitlab::Usage::Metrics::NameSuggestion.for(:count, column: column, relation: relation)
             end

lib/gitlab/usage_data.rb

@@ -51,10 +51,7 @@ module Gitlab
         clear_memoized
 
         with_finished_at(:recording_ce_finished_at) do
-          usage_data = usage_data_metrics
-          usage_data = usage_data.with_indifferent_access.deep_merge(instrumentation_metrics.with_indifferent_access) if Feature.enabled?(:usage_data_instrumentation)
-
-          usage_data
+          usage_data_metrics
         end
       end
 
@@ -757,10 +754,6 @@ module Gitlab
           .deep_merge(aggregated_metrics_data)
       end
 
-      def instrumentation_metrics
-        Gitlab::UsageDataMetrics.uncached_data # rubocop:disable UsageData/LargeTable
-      end
-
       def metric_time_period(time_period)
         time_period.present? ? '28d' : 'none'
       end

lib/gitlab/usage_data_non_sql_metrics.rb

@@ -5,13 +5,6 @@ module Gitlab
     SQL_METRIC_DEFAULT = -3
 
     class << self
-      def uncached_data
-        # instrumentation_metrics is already included with feature flag enabled
-        return super if Feature.enabled?(:usage_data_instrumentation)
-
-        super.with_indifferent_access.deep_merge(instrumentation_metrics.with_indifferent_access)
-      end
-
       def add_metric(metric, time_frame: 'none', options: {})
         metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
 
@@ -50,12 +43,6 @@ module Gitlab
           projects_jira_cloud_active: 0
         }
       end
-
-      private
-
-      def instrumentation_metrics
-        ::Gitlab::Usage::Metric.all.map(&:with_instrumentation).reduce({}, :deep_merge)
-      end
     end
   end
 end

lib/gitlab/usage_data_queries.rb

@@ -5,13 +5,6 @@ module Gitlab
   # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41091
   class UsageDataQueries < UsageData
     class << self
-      def uncached_data
-        # instrumentation_metrics is already included with feature flag enabled
-        return super if Feature.enabled?(:usage_data_instrumentation)
-
-        super.with_indifferent_access.deep_merge(instrumentation_metrics.with_indifferent_access)
-      end
-
       def add_metric(metric, time_frame: 'none', options: {})
         metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
 
@@ -71,12 +64,6 @@ module Gitlab
       def epics_deepest_relationship_level
         { epics_deepest_relationship_level: 0 }
       end
-
-      private
-
-      def instrumentation_metrics
-        ::Gitlab::Usage::Metric.all.map(&:with_instrumentation).reduce({}, :deep_merge)
-      end
     end
   end
 end

lib/gitlab/utils/usage_data.rb

@@ -43,13 +43,8 @@ module Gitlab
       HISTOGRAM_FALLBACK = { '-1' => -1 }.freeze
       DISTRIBUTED_HLL_FALLBACK = -2
       MAX_BUCKET_SIZE = 100
-      INSTRUMENTATION_CLASS_FALLBACK = -100
 
       def add_metric(metric, time_frame: 'none', options: {})
-        # Results of this method should be overwritten by instrumentation class values
-        # -100 indicates the metric was not properly merged.
-        return INSTRUMENTATION_CLASS_FALLBACK if Feature.enabled?(:usage_data_instrumentation)
-
         metric_class = "Gitlab::Usage::Metrics::Instrumentations::#{metric}".constantize
 
         metric_class.new(time_frame: time_frame, options: options).value

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

@@ -25,30 +25,10 @@ RSpec.describe Gitlab::Usage::Metrics::NamesSuggestions::Generator do
     end
 
     context 'for count with default column metrics' do
-      context 'with usage_data_instrumentation feature flag' do
-        context 'when enabled' do
-          before do
-            stub_feature_flags(usage_data_instrumentation: true)
-          end
-
-          it_behaves_like 'name suggestion' do
-            # corresponding metric is collected with ::Gitlab::UsageDataMetrics.suggested_names
-            let(:key_path) { 'counts.boards' }
-            let(:name_suggestion) { /count_boards/ }
-          end
-        end
-
-        context 'when disabled' do
-          before do
-            stub_feature_flags(usage_data_instrumentation: false)
-          end
-
-          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
+      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
 

spec/lib/gitlab/utils/usage_data_spec.rb

@@ -8,26 +8,8 @@ RSpec.describe Gitlab::Utils::UsageData do
   describe '#add_metric' do
     let(:metric) { 'UuidMetric'}
 
-    context 'with usage_data_instrumentation feature flag' do
-      context 'when enabled' do
-        before do
-          stub_feature_flags(usage_data_instrumentation: true)
-        end
-
-        it 'returns -100 value to be overriden' do
-          expect(described_class.add_metric(metric)).to eq(-100)
-        end
-      end
-
-      context 'when disabled' do
-        before do
-          stub_feature_flags(usage_data_instrumentation: false)
-        end
-
-        it 'computes the metric value for given metric' do
-          expect(described_class.add_metric(metric)).to eq(Gitlab::CurrentSettings.uuid)
-        end
-      end
+    it 'computes the metric value for given metric' do
+      expect(described_class.add_metric(metric)).to eq(Gitlab::CurrentSettings.uuid)
     end
   end