From 01de60f3a8f4d99e641d04d31369628425137c21 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 2 Dec 2019 06:06:09 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- Gemfile | 1 - Gemfile.lock | 4 - config/initializers/0_marginalia.rb | 19 -- doc/ci/variables/README.md | 6 +- doc/ci/yaml/README.md | 1 + lib/gitlab/marginalia.rb | 23 --- .../active_record_instrumentation.rb | 20 -- lib/gitlab/marginalia/comment.rb | 42 ----- lib/gitlab/marginalia/inline_annotation.rb | 37 ---- spec/lib/feature_spec.rb | 1 - spec/lib/marginalia_spec.rb | 173 ------------------ spec/serializers/pipeline_serializer_spec.rb | 6 +- 12 files changed, 7 insertions(+), 326 deletions(-) delete mode 100644 config/initializers/0_marginalia.rb delete mode 100644 lib/gitlab/marginalia.rb delete mode 100644 lib/gitlab/marginalia/active_record_instrumentation.rb delete mode 100644 lib/gitlab/marginalia/comment.rb delete mode 100644 lib/gitlab/marginalia/inline_annotation.rb delete mode 100644 spec/lib/marginalia_spec.rb diff --git a/Gemfile b/Gemfile index 24e7a53126e..19b80dd3bd8 100644 --- a/Gemfile +++ b/Gemfile @@ -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' diff --git a/Gemfile.lock b/Gemfile.lock index a290b8f96a5..054a491a019 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -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) diff --git a/config/initializers/0_marginalia.rb b/config/initializers/0_marginalia.rb deleted file mode 100644 index 42d8f0bf5e1..00000000000 --- a/config/initializers/0_marginalia.rb +++ /dev/null @@ -1,19 +0,0 @@ -# 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 diff --git a/doc/ci/variables/README.md b/doc/ci/variables/README.md index cff797549ba..488d9a05a3c 100644 --- a/doc/ci/variables/README.md +++ b/doc/ci/variables/README.md @@ -284,6 +284,8 @@ export CI_PROJECT_PATH="gitlab-org/gitlab-foss" export CI_PROJECT_URL="https://example.com/gitlab-org/gitlab-foss" export CI_REGISTRY="registry.example.com" export CI_REGISTRY_IMAGE="registry.example.com/gitlab-org/gitlab-foss" +export CI_REGISTRY_USER="gitlab-ci-token" +export CI_REGISTRY_PASSWORD="longalfanumstring" export CI_RUNNER_ID="10" export CI_RUNNER_DESCRIPTION="my runner" export CI_RUNNER_TAGS="docker, linux" @@ -295,10 +297,8 @@ export CI_SERVER_VERSION="8.9.0" export CI_SERVER_VERSION_MAJOR="8" export CI_SERVER_VERSION_MINOR="9" export CI_SERVER_VERSION_PATCH="0" -export GITLAB_USER_ID="42" export GITLAB_USER_EMAIL="user@example.com" -export CI_REGISTRY_USER="gitlab-ci-token" -export CI_REGISTRY_PASSWORD="longalfanumstring" +export GITLAB_USER_ID="42" ``` ### `.gitlab-ci.yml` defined variables diff --git a/doc/ci/yaml/README.md b/doc/ci/yaml/README.md index 12c009d9e90..a92c95ce7d8 100644 --- a/doc/ci/yaml/README.md +++ b/doc/ci/yaml/README.md @@ -1247,6 +1247,7 @@ This is useful if you want to avoid jobs entering `pending` state immediately. You can set the period with `start_in` key. The value of `start_in` key is an elapsed time in seconds, unless a unit is provided. `start_in` key must be less than or equal to one week. Examples of valid values include: +- `'5'` - `10 seconds` - `30 minutes` - `1 day` diff --git a/lib/gitlab/marginalia.rb b/lib/gitlab/marginalia.rb deleted file mode 100644 index d2e0e335127..00000000000 --- a/lib/gitlab/marginalia.rb +++ /dev/null @@ -1,23 +0,0 @@ -# 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 diff --git a/lib/gitlab/marginalia/active_record_instrumentation.rb b/lib/gitlab/marginalia/active_record_instrumentation.rb deleted file mode 100644 index f4500a48090..00000000000 --- a/lib/gitlab/marginalia/active_record_instrumentation.rb +++ /dev/null @@ -1,20 +0,0 @@ -# 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 diff --git a/lib/gitlab/marginalia/comment.rb b/lib/gitlab/marginalia/comment.rb deleted file mode 100644 index a0eee823763..00000000000 --- a/lib/gitlab/marginalia/comment.rb +++ /dev/null @@ -1,42 +0,0 @@ -# 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 diff --git a/lib/gitlab/marginalia/inline_annotation.rb b/lib/gitlab/marginalia/inline_annotation.rb deleted file mode 100644 index 6d78f3b81ec..00000000000 --- a/lib/gitlab/marginalia/inline_annotation.rb +++ /dev/null @@ -1,37 +0,0 @@ -# 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 diff --git a/spec/lib/feature_spec.rb b/spec/lib/feature_spec.rb index 7254981d09a..3d59b1f35a9 100644 --- a/spec/lib/feature_spec.rb +++ b/spec/lib/feature_spec.rb @@ -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 diff --git a/spec/lib/marginalia_spec.rb b/spec/lib/marginalia_spec.rb deleted file mode 100644 index 2a1dbd94a9e..00000000000 --- a/spec/lib/marginalia_spec.rb +++ /dev/null @@ -1,173 +0,0 @@ -# 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 diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index f1f761a6fd0..7661c8acc13 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -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