Commit fc3707d5 authored by Sean McGivern's avatar Sean McGivern

Remove Marginalia feature flag

We've had this enabled on GitLab.com for over 10 months and have no
intention of disabling it. Even if we did, it would need an application
restart, so the value of a feature flag is quite low.
parent 02d31976
---
name: marginalia
introduced_by_url:
rollout_issue_url:
milestone:
type: ops
group:
default_enabled: false
......@@ -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
......@@ -47,16 +47,3 @@ Examples of queries with comments as observed in `development.log`:
```sql
SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = $1 LIMIT $2 [["id", 64], ["LIMIT", 1]] /*application:sidekiq,jid:e7d6668a39a991e323009833,job_class:ExpireJobCacheWorker,correlation_id:rYF4mey9CH3,line:/app/workers/expire_job_cache_worker.rb:14:in `perform'*/
```
## 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)
```
......@@ -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
# 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
......@@ -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('/')
......
......@@ -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:
#
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment