Skip to content

G
gitlab-ce
Commit a232ec03 authored by Heinrich Lee Yu's avatar Heinrich Lee Yu
Browse files
Options

Revert "Merge branch 'marginalia' into 'master'" 

This reverts merge request !19759
parent c24720fa
Show whitespace changes
Inline Side-by-side
Showing with 3 additions and 325 deletions
Gemfile
View file @ a232ec03
......@@ -22,7 +22,6 @@ gem 'rugged', '~> 0.28'
gem 'grape-path-helpers', '~> 1.1'
gem 'faraday', '~> 0.12'
gem 'marginalia', '~> 1.8.0'
# Authentication libraries
gem 'devise', '~> 4.6'
......
Gemfile.lock
View file @ a232ec03
......@@ -591,9 +591,6 @@ GEM
mail_room (0.9.1)
marcel (0.3.3)
mimemagic (~> 0.3.2)
marginalia (1.8.0)
actionpack (>= 2.3)
activerecord (>= 2.3)
memoist (0.16.0)
memoizable (0.4.2)
thread_safe (~> 0.3, >= 0.3.1)
......@@ -1246,7 +1243,6 @@ DEPENDENCIES
lograge (~> 0.5)
loofah (~> 2.2)
mail_room (~> 0.9.1)
marginalia (~> 1.8.0)
memory_profiler (~> 0.9)
method_source (~> 0.8)
mimemagic (~> 0.3.2)
......
config/initializers/0_marginalia.rb deleted 100644 → 0
View file @ c24720fa
# frozen_string_literal: true
require 'marginalia'
::Marginalia::Comment.extend(::Gitlab::Marginalia::Comment)
# Patch to include support for 'Marginalia.without_annotation' method.
::Marginalia.singleton_class.prepend(Gitlab::Marginalia::InlineAnnotation)
# 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)
Marginalia::Comment.components = [:application, :controller, :action, :correlation_id, :jid, :job_class, :line]
Gitlab::Marginalia.set_application_name
Gitlab::Marginalia.enable_sidekiq_instrumentation
ee/spec/lib/gitlab/geo/health_check_spec.rb
View file @ a232ec03
......@@ -67,7 +67,6 @@ describe Gitlab::Geo::HealthCheck, :geo do
context 'streaming replication' do
it 'returns an error when replication is not working' do
allow(Gitlab::Database).to receive(:pg_last_wal_receive_lsn).and_return('pg_last_xlog_receive_location')
allow(Gitlab::Database).to receive(:cached_table_exists?).with('features').and_return(false)
allow(ActiveRecord::Base).to receive_message_chain('connection.execute').with(no_args).with('SELECT * FROM pg_last_xlog_receive_location() as result').and_return(['result' => 'fake'])
allow(ActiveRecord::Base).to receive_message_chain('connection.select_values').with(no_args).with('SELECT pid FROM pg_stat_wal_receiver').and_return([])
......@@ -80,7 +79,6 @@ describe Gitlab::Geo::HealthCheck, :geo do
allow(subject).to receive(:streaming_replication_enabled?).and_return(false)
allow(subject).to receive(:archive_recovery_replication_enabled?).and_return(true)
allow(Gitlab::Database).to receive(:pg_last_xact_replay_timestamp).and_return('pg_last_xact_replay_timestamp')
allow(Gitlab::Database).to receive(:cached_table_exists?).with('features').and_return(false)
allow(ActiveRecord::Base).to receive_message_chain('connection.execute').with(no_args).with('SELECT * FROM pg_last_xact_replay_timestamp() as result').and_return([{ 'result' => nil }])
expect(subject.perform_checks).to match(/Geo node does not appear to be replicating the database from the primary node/)
......
lib/gitlab/marginalia.rb deleted 100644 → 0
View file @ c24720fa
# frozen_string_literal: true
module Gitlab
module Marginalia
MARGINALIA_FEATURE_FLAG = :marginalia
def self.set_application_name
::Marginalia.application_name = Gitlab.process_name
end
def self.enable_sidekiq_instrumentation
if Sidekiq.server?
::Marginalia::SidekiqInstrumentation.enable!
end
end
def self.feature_enabled?
return false unless Gitlab::Database.cached_table_exists?('features')
Feature.enabled?(MARGINALIA_FEATURE_FLAG)
end
end
end
lib/gitlab/marginalia/active_record_instrumentation.rb deleted 100644 → 0
View file @ c24720fa
# frozen_string_literal: true
# Patch to annotate sql only when the feature is enabled.
module Gitlab
module Marginalia
module ActiveRecordInstrumentation
# CAUTION:
# Any method call which generates a query inside this function will get into a recursive loop unless called within `Marginalia.without_annotation` method.
def annotate_sql(sql)
if ActiveRecord::Base.connected? &&
::Marginalia.annotation_allowed? &&
::Marginalia.without_annotation { Gitlab::Marginalia.feature_enabled? }
super(sql)
else
sql
end
end
end
end
end
lib/gitlab/marginalia/comment.rb deleted 100644 → 0
View file @ c24720fa
# frozen_string_literal: true
# Module to support correlation_id and additional job details.
module Gitlab
module Marginalia
module Comment
private
def jid
bg_job["jid"] if bg_job.present?
end
def job_class
bg_job["class"] if bg_job.present?
end
def correlation_id
if bg_job.present?
bg_job["correlation_id"]
else
Labkit::Correlation::CorrelationId.current_id
end
end
def bg_job
job = ::Marginalia::Comment.marginalia_job
# We are using 'Marginalia::SidekiqInstrumentation' which does not support 'ActiveJob::Base'.
# Gitlab also uses 'ActionMailer::DeliveryJob' which inherits from ActiveJob::Base.
# So below condition is used to return metadata for such jobs.
if job && job.is_a?(ActionMailer::DeliveryJob)
{
"class" => job.arguments.first,
"jid" => job.job_id
}
else
job
end
end
end
end
end
lib/gitlab/marginalia/inline_annotation.rb deleted 100644 → 0
View file @ c24720fa
# frozen_string_literal: true
# Module with util methods to support ::Marginalia.without_annotation method.
module Gitlab
module Marginalia
module InlineAnnotation
def without_annotation(&block)
return unless block.present?
annotation_stack.push(false)
yield
ensure
annotation_stack.pop
end
def with_annotation(comment, &block)
annotation_stack.push(true)
super(comment, &block)
ensure
annotation_stack.pop
end
def annotation_stack
Thread.current[:annotation_stack] ||= []
end
def annotation_stack_top
annotation_stack.last
end
def annotation_allowed?
annotation_stack.empty? ? true : annotation_stack_top
end
end
end
end
spec/lib/feature_spec.rb
View file @ a232ec03
......@@ -175,7 +175,6 @@ describe Feature do
let(:flag) { :some_feature_flag }
before do
stub_feature_flags(Gitlab::Marginalia::MARGINALIA_FEATURE_FLAG => false)
described_class.flipper.memoize = false
described_class.enabled?(flag)
end
......
spec/lib/marginalia_spec.rb deleted 100644 → 0
View file @ c24720fa
# frozen_string_literal: true
require 'spec_helper'
describe 'Marginalia spec' do
class MarginaliaTestController < ActionController::Base
def first_user
User.first
render body: nil
end
end
class MarginaliaTestJob
include Sidekiq::Worker
def perform
User.first
end
end
class MarginaliaTestMailer < BaseMailer
def first_user
User.first
end
end
def add_sidekiq_middleware
# Reference: https://github.com/mperham/sidekiq/wiki/Testing#testing-server-middlewaresidekiq
# Sidekiq test harness fakes worker without its server middlewares, so include instrumentation to 'Sidekiq::Testing' server middleware.
Sidekiq::Testing.server_middleware do |chain|
chain.add Marginalia::SidekiqInstrumentation::Middleware
end
end
def remove_sidekiq_middleware
Sidekiq::Testing.server_middleware do |chain|
chain.remove Marginalia::SidekiqInstrumentation::Middleware
end
end
def stub_feature(value)
stub_feature_flags(Gitlab::Marginalia::MARGINALIA_FEATURE_FLAG => value)
end
def make_request(correlation_id)
request_env = Rack::MockRequest.env_for('/')
::Labkit::Correlation::CorrelationId.use_id(correlation_id) do
MarginaliaTestController.action(:first_user).call(request_env)
end
end
describe 'For rails web requests' do
let(:correlation_id) { SecureRandom.uuid }
let(:recorded) { ActiveRecord::QueryRecorder.new { make_request(correlation_id) } }
let(:component_map) do
{
"application" => "test",
"controller" => "marginalia_test",
"action" => "first_user",
"line" => "/spec/support/helpers/query_recorder.rb",
"correlation_id" => correlation_id
}
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("*/")
end
end
end
describe 'for Sidekiq worker jobs' do
before(:all) do
add_sidekiq_middleware
# Because of faking, 'Sidekiq.server?' does not work so implicitly set application name which is done in config/initializers/0_marginalia.rb
Marginalia.application_name = "sidekiq"
end
after(:all) do
MarginaliaTestJob.clear
remove_sidekiq_middleware
end
around do |example|
Sidekiq::Testing.fake! { example.run }
end
before do
MarginaliaTestJob.perform_async
end
let(:sidekiq_job) { MarginaliaTestJob.jobs.first }
let(:recorded) { ActiveRecord::QueryRecorder.new { MarginaliaTestJob.drain } }
let(:component_map) do
{
"application" => "sidekiq",
"job_class" => "MarginaliaTestJob",
"line" => "/spec/support/sidekiq_middleware.rb",
"correlation_id" => sidekiq_job['correlation_id'],
"jid" => sidekiq_job['jid']
}
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
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",
"line" => "/lib/gitlab/i18n.rb",
"jid" => delivery_job.job_id,
"job_class" => delivery_job.arguments.first
}
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
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("*/")
end
end
end
end
spec/serializers/pipeline_serializer_spec.rb
View file @ a232ec03
......@@ -130,10 +130,10 @@ describe PipelineSerializer do
it 'preloads related merge requests' do
recorded = ActiveRecord::QueryRecorder.new { subject }
expected_query = "SELECT \"merge_requests\".* FROM \"merge_requests\" " \
"WHERE \"merge_requests\".\"id\" IN (#{merge_request_1.id}, #{merge_request_2.id})"
expect(recorded.log).to include(a_string_starting_with(expected_query))
expect(recorded.log)
.to include("SELECT \"merge_requests\".* FROM \"merge_requests\" " \
"WHERE \"merge_requests\".\"id\" IN (#{merge_request_1.id}, #{merge_request_2.id})")
end
end
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7