Merge branch 'remove-marginalia-feature-flag' into 'master'

Remove Marginalia feature flag [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!54349

config/feature_flags/ops/marginalia.yml

----
-name: marginalia
-introduced_by_url: 
-rollout_issue_url: 
-milestone: 
-type: ops
-group: 
-default_enabled: false

config/initializers/0_marginalia.rb

@@ -4,11 +4,6 @@ require 'marginalia'
 
 ::Marginalia::Comment.extend(::Gitlab::Marginalia::Comment)
 
-# Patch to modify 'Marginalia::ActiveRecordInstrumentation.annotate_sql' method with feature check.
-# Orignal Marginalia::ActiveRecordInstrumentation is included to ActiveRecord::ConnectionAdapters::PostgreSQLAdapter in the Marginalia Railtie.
-# Refer: https://github.com/basecamp/marginalia/blob/v1.8.0/lib/marginalia/railtie.rb#L67
-ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.prepend(Gitlab::Marginalia::ActiveRecordInstrumentation)
-
 # By default, PostgreSQL only tracks the first 1024 bytes of a SQL
 # query. Prepending the comment allows us to trace the source of the
 # query without having to increase the `track_activity_query_size`
@@ -25,5 +20,3 @@ Marginalia::Comment.components << :line if Rails.env.development?
 Gitlab::Marginalia.set_application_name
 
 Gitlab::Marginalia.enable_sidekiq_instrumentation
-
-Gitlab::Marginalia.set_enabled_from_feature_flag

doc/development/database_query_comments.md

@@ -47,16 +47,3 @@ Examples of queries with comments as observed in `development.log`:
    ```sql
    /*application:sidekiq,jid:e7d6668a39a991e323009833,job_class:ExpireJobCacheWorker,correlation_id:rYF4mey9CH3,line:/app/workers/expire_job_cache_worker.rb:14:in `perform'*/ SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = $1 LIMIT $2 [["id", 64], ["LIMIT", 1]]
    ```
-
-## Enable/Disable the feature
-
-Enabling or disabling the feature requires a **restart/SIGHUP** of the Web and
-Sidekiq workers, as the feature flag's state is memoized upon starting up.
-
-The `feature_flag` for this feature is **disabled** by default. You can enable
-or disable it with:
-
-```ruby
-Feature.enable(:marginalia)
-Feature.disable(:marginalia)
-```

lib/gitlab/marginalia.rb

@@ -2,8 +2,6 @@
 
 module Gitlab
   module Marginalia
-    cattr_accessor :enabled, default: false
-
     def self.set_application_name
       ::Marginalia.application_name = Gitlab.process_name
     end
@@ -13,12 +11,5 @@ module Gitlab
         ::Marginalia::SidekiqInstrumentation.enable!
       end
     end
-
-    def self.set_enabled_from_feature_flag
-      # During db:create and db:bootstrap skip feature query as DB is not available yet.
-      return false unless Gitlab::Database.cached_table_exists?('features')
-
-      self.enabled = Feature.enabled?(:marginalia, type: :ops)
-    end
   end
 end

lib/gitlab/marginalia/active_record_instrumentation.rb

-# frozen_string_literal: true
-
-# Patch to annotate sql only when the feature is enabled.
-module Gitlab
-  module Marginalia
-    module ActiveRecordInstrumentation
-      def annotate_sql(sql)
-        Gitlab::Marginalia.enabled ? super(sql) : sql
-      end
-    end
-  end
-end

spec/lib/marginalia_spec.rb

@@ -37,26 +37,9 @@ RSpec.describe 'Marginalia spec' do
       }
     end
 
-    context 'when the feature is enabled' do
-      before do
-        stub_feature(true)
-      end
-
-      it 'generates a query that includes the component and value' do
-        component_map.each do |component, value|
-          expect(recorded.log.last).to include("#{component}:#{value}")
-        end
-      end
-    end
-
-    context 'when the feature is disabled' do
-      before do
-        stub_feature(false)
-      end
-
-      it 'excludes annotations in generated queries' do
-        expect(recorded.log.last).not_to include("/*")
-        expect(recorded.log.last).not_to include("*/")
+    it 'generates a query that includes the component and value' do
+      component_map.each do |component, value|
+        expect(recorded.log.last).to include("#{component}:#{value}")
       end
     end
   end
@@ -90,59 +73,37 @@ RSpec.describe 'Marginalia spec' do
       }
     end
 
-    context 'when the feature is enabled' do
-      before do
-        stub_feature(true)
+    it 'generates a query that includes the component and value' do
+      component_map.each do |component, value|
+        expect(recorded.log.last).to include("#{component}:#{value}")
       end
+    end
 
-      it 'generates a query that includes the component and value' do
-        component_map.each do |component, value|
-          expect(recorded.log.last).to include("#{component}:#{value}")
-        end
-      end
-
-      describe 'for ActionMailer delivery jobs' do
-        let(:delivery_job) { MarginaliaTestMailer.first_user.deliver_later }
-
-        let(:recorded) do
-          ActiveRecord::QueryRecorder.new do
-            delivery_job.perform_now
-          end
-        end
-
-        let(:component_map) do
-          {
-            "application"  => "sidekiq",
-            "jid"          => delivery_job.job_id,
-            "job_class"    => delivery_job.arguments.first
-          }
-        end
+    describe 'for ActionMailer delivery jobs' do
+      let(:delivery_job) { MarginaliaTestMailer.first_user.deliver_later }
 
-        it 'generates a query that includes the component and value' do
-          component_map.each do |component, value|
-            expect(recorded.log.last).to include("#{component}:#{value}")
-          end
+      let(:recorded) do
+        ActiveRecord::QueryRecorder.new do
+          delivery_job.perform_now
         end
       end
-    end
 
-    context 'when the feature is disabled' do
-      before do
-        stub_feature(false)
+      let(:component_map) do
+        {
+          "application"  => "sidekiq",
+          "jid"          => delivery_job.job_id,
+          "job_class"    => delivery_job.arguments.first
+        }
       end
 
-      it 'excludes annotations in generated queries' do
-        expect(recorded.log.last).not_to include("/*")
-        expect(recorded.log.last).not_to include("*/")
+      it 'generates a query that includes the component and value' do
+        component_map.each do |component, value|
+          expect(recorded.log.last).to include("#{component}:#{value}")
+        end
       end
     end
   end
 
-  def stub_feature(value)
-    stub_feature_flags(marginalia: value)
-    Gitlab::Marginalia.set_enabled_from_feature_flag
-  end
-
   def make_request(correlation_id)
     request_env = Rack::MockRequest.env_for('/')
 

spec/spec_helper.rb

@@ -249,9 +249,6 @@ RSpec.configure do |config|
       unstub_all_feature_flags
     end
 
-    # Enable Marginalia feature for all specs in the test suite.
-    Gitlab::Marginalia.enabled = true
-
     # Stub these calls due to being expensive operations
     # It can be reenabled for specific tests via:
     #