Commit 0d0fd0d9 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch 'refactor-labkit-context-to-gitlab-application-context' into 'master'

Refactor Labkit::Context to Gitlab::ApplicationContext

See merge request gitlab-org/gitlab!56003
parents 6d39b00e b3d88d8f
...@@ -463,7 +463,7 @@ class ApplicationController < ActionController::Base ...@@ -463,7 +463,7 @@ class ApplicationController < ActionController::Base
feature_category: feature_category) do feature_category: feature_category) do
yield yield
ensure ensure
@current_context = Labkit::Context.current.to_h @current_context = Gitlab::ApplicationContext.current
end end
end end
......
...@@ -8,7 +8,7 @@ module Mutations ...@@ -8,7 +8,7 @@ module Mutations
ADMIN_MESSAGE = 'You must be an admin to use this mutation' ADMIN_MESSAGE = 'You must be an admin to use this mutation'
Labkit::Context::KNOWN_KEYS.each do |key| Gitlab::ApplicationContext::KNOWN_KEYS.each do |key|
argument key, argument key,
GraphQL::STRING_TYPE, GraphQL::STRING_TYPE,
required: false, required: false,
......
...@@ -18,7 +18,7 @@ module ApplicationWorker ...@@ -18,7 +18,7 @@ module ApplicationWorker
set_queue set_queue
def structured_payload(payload = {}) def structured_payload(payload = {})
context = Labkit::Context.current.to_h.merge( context = Gitlab::ApplicationContext.current.merge(
'class' => self.class.name, 'class' => self.class.name,
'job_status' => 'running', 'job_status' => 'running',
'queue' => self.class.queue, 'queue' => self.class.queue,
......
...@@ -15,7 +15,7 @@ module CronjobQueue ...@@ -15,7 +15,7 @@ module CronjobQueue
# Cronjobs never get scheduled with arguments, so this is safe to # Cronjobs never get scheduled with arguments, so this is safe to
# override # override
def context_for_arguments(_args) def context_for_arguments(_args)
return if Gitlab::ApplicationContext.current_context_include?('meta.caller_id') return if Gitlab::ApplicationContext.current_context_include?(:caller_id)
Gitlab::ApplicationContext.new(caller_id: "Cronjob") Gitlab::ApplicationContext.new(caller_id: "Cronjob")
end end
......
...@@ -6,7 +6,7 @@ module Elastic ...@@ -6,7 +6,7 @@ module Elastic
# This class should only be used with sidekiq workers which extend Elastic::IndexingControl module # This class should only be used with sidekiq workers which extend Elastic::IndexingControl module
class IndexingControlService class IndexingControlService
LIMIT = 1000 LIMIT = 1000
PROJECT_CONTEXT_KEY = "#{Labkit::Context::LOG_KEY}.project" PROJECT_CONTEXT_KEY = "#{Gitlab::ApplicationContext::LOG_KEY}.project"
def initialize(klass) def initialize(klass)
raise ArgumentError, "passed class must extend Elastic::IndexingControl" unless klass.include?(Elastic::IndexingControl) raise ArgumentError, "passed class must extend Elastic::IndexingControl" unless klass.include?(Elastic::IndexingControl)
...@@ -93,7 +93,7 @@ module Elastic ...@@ -93,7 +93,7 @@ module Elastic
end end
def send_to_processing_queue(job) def send_to_processing_queue(job)
Labkit::Context.with_context(job['context']) do Gitlab::ApplicationContext.with_raw_context(job['context']) do
klass.perform_async(*job['args']) klass.perform_async(*job['args'])
end end
end end
......
...@@ -51,7 +51,7 @@ module Elastic ...@@ -51,7 +51,7 @@ module Elastic
end end
def current_context def current_context
Labkit::Context.current.to_h Gitlab::ApplicationContext.current
end end
end end
end end
...@@ -20,7 +20,7 @@ RSpec.describe Elastic::IndexingControlService, :clean_gitlab_redis_shared_state ...@@ -20,7 +20,7 @@ RSpec.describe Elastic::IndexingControlService, :clean_gitlab_redis_shared_state
end end
let(:stored_context) do let(:stored_context) do
{ "#{Labkit::Context::LOG_KEY}.project" => 'gitlab-org/gitlab' } { "#{Gitlab::ApplicationContext::LOG_KEY}.project" => 'gitlab-org/gitlab' }
end end
let(:worker_args) { [1, 2] } let(:worker_args) { [1, 2] }
...@@ -133,7 +133,7 @@ RSpec.describe Elastic::IndexingControlService, :clean_gitlab_redis_shared_state ...@@ -133,7 +133,7 @@ RSpec.describe Elastic::IndexingControlService, :clean_gitlab_redis_shared_state
subject.add_to_waiting_queue!(j, worker_context) subject.add_to_waiting_queue!(j, worker_context)
end end
expect(Labkit::Context).to receive(:with_context).with(stored_context).exactly(jobs.count).times.and_call_original expect(Gitlab::ApplicationContext).to receive(:with_raw_context).with(stored_context).exactly(jobs.count).times.and_call_original
expect(worker_class).to receive(:perform_async).exactly(jobs.count).times expect(worker_class).to receive(:perform_async).exactly(jobs.count).times
expect { subject.resume_processing! }.to change { subject.has_jobs_in_waiting_queue? }.from(true).to(false) expect { subject.resume_processing! }.to change { subject.has_jobs_in_waiting_queue? }.from(true).to(false)
......
...@@ -75,7 +75,7 @@ RSpec.describe Elastic::IndexingControl do ...@@ -75,7 +75,7 @@ RSpec.describe Elastic::IndexingControl do
expect_any_instance_of(Gitlab::Elastic::Indexer).not_to receive(:run) expect_any_instance_of(Gitlab::Elastic::Indexer).not_to receive(:run)
expect(Elastic::IndexingControlService).to receive(:add_to_waiting_queue!).with(worker.class, worker_args, worker_context) expect(Elastic::IndexingControlService).to receive(:add_to_waiting_queue!).with(worker.class, worker_args, worker_context)
Labkit::Context.with_context(worker_context) do Gitlab::ApplicationContext.with_raw_context(worker_context) do
worker.perform(*worker_args) worker.perform(*worker_args)
end end
end end
......
...@@ -32,10 +32,10 @@ RSpec.describe UpdateAllMirrorsWorker do ...@@ -32,10 +32,10 @@ RSpec.describe UpdateAllMirrorsWorker do
outer_context = nil outer_context = nil
Gitlab::ApplicationContext.with_context(project: build(:project)) do Gitlab::ApplicationContext.with_context(project: build(:project)) do
outer_context = Labkit::Context.current.to_h outer_context = Gitlab::ApplicationContext.current
expect(worker).to receive(:schedule_mirrors!) do expect(worker).to receive(:schedule_mirrors!) do
inner_context = Labkit::Context.current.to_h inner_context = Gitlab::ApplicationContext.current
# `schedule_mirrors!` needs to return an integer. # `schedule_mirrors!` needs to return an integer.
0 0
......
...@@ -12,11 +12,11 @@ module API ...@@ -12,11 +12,11 @@ module API
namespace 'queues' do namespace 'queues' do
desc 'Drop jobs matching the given metadata from the Sidekiq queue' desc 'Drop jobs matching the given metadata from the Sidekiq queue'
params do params do
Labkit::Context::KNOWN_KEYS.each do |key| Gitlab::ApplicationContext::KNOWN_KEYS.each do |key|
optional key, type: String, allow_blank: false optional key, type: String, allow_blank: false
end end
at_least_one_of(*Labkit::Context::KNOWN_KEYS) at_least_one_of(*Gitlab::ApplicationContext::KNOWN_KEYS)
end end
delete ':queue_name' do delete ':queue_name' do
result = result =
......
...@@ -7,6 +7,9 @@ module Gitlab ...@@ -7,6 +7,9 @@ module Gitlab
Attribute = Struct.new(:name, :type) Attribute = Struct.new(:name, :type)
LOG_KEY = Labkit::Context::LOG_KEY
KNOWN_KEYS = Labkit::Context::KNOWN_KEYS
APPLICATION_ATTRIBUTES = [ APPLICATION_ATTRIBUTES = [
Attribute.new(:project, Project), Attribute.new(:project, Project),
Attribute.new(:namespace, Namespace), Attribute.new(:namespace, Namespace),
...@@ -22,6 +25,10 @@ module Gitlab ...@@ -22,6 +25,10 @@ module Gitlab
application_context.use(&block) application_context.use(&block)
end end
def self.with_raw_context(attributes = {}, &block)
Labkit::Context.with_context(attributes, &block)
end
def self.push(args) def self.push(args)
application_context = new(**args) application_context = new(**args)
Labkit::Context.push(application_context.to_lazy_hash) Labkit::Context.push(application_context.to_lazy_hash)
......
...@@ -215,7 +215,7 @@ module Gitlab ...@@ -215,7 +215,7 @@ module Gitlab
'client_name' => CLIENT_NAME 'client_name' => CLIENT_NAME
} }
context_data = Labkit::Context.current&.to_h context_data = Gitlab::ApplicationContext.current
feature_stack = Thread.current[:gitaly_feature_stack] feature_stack = Thread.current[:gitaly_feature_stack]
feature = feature_stack && feature_stack[0] feature = feature_stack && feature_stack[0]
......
...@@ -6,7 +6,7 @@ module Gitlab ...@@ -6,7 +6,7 @@ module Gitlab
module Loggers module Loggers
class ContextLogger < ::GrapeLogging::Loggers::Base class ContextLogger < ::GrapeLogging::Loggers::Base
def parameters(_, _) def parameters(_, _)
Labkit::Context.current.to_h Gitlab::ApplicationContext.current
end end
end end
end end
......
...@@ -21,7 +21,7 @@ module Gitlab ...@@ -21,7 +21,7 @@ module Gitlab
job_search_metadata = job_search_metadata =
search_metadata search_metadata
.stringify_keys .stringify_keys
.slice(*Labkit::Context::KNOWN_KEYS) .slice(*Gitlab::ApplicationContext::KNOWN_KEYS)
.transform_keys { |key| "meta.#{key}" } .transform_keys { |key| "meta.#{key}" }
.compact .compact
......
...@@ -900,7 +900,7 @@ RSpec.describe ApplicationController do ...@@ -900,7 +900,7 @@ RSpec.describe ApplicationController do
feature_category :issue_tracking feature_category :issue_tracking
def index def index
Labkit::Context.with_context do |context| Gitlab::ApplicationContext.with_raw_context do |context|
render json: context.to_h render json: context.to_h
end end
end end
......
...@@ -524,7 +524,7 @@ RSpec.describe SessionsController do ...@@ -524,7 +524,7 @@ RSpec.describe SessionsController do
it 'sets the username and caller_id in the context' do it 'sets the username and caller_id in the context' do
expect(controller).to receive(:destroy).and_wrap_original do |m, *args| expect(controller).to receive(:destroy).and_wrap_original do |m, *args|
expect(Labkit::Context.current.to_h) expect(Gitlab::ApplicationContext.current)
.to include('meta.user' => user.username, .to include('meta.user' => user.username,
'meta.caller_id' => 'SessionsController#destroy') 'meta.caller_id' => 'SessionsController#destroy')
...@@ -538,9 +538,9 @@ RSpec.describe SessionsController do ...@@ -538,9 +538,9 @@ RSpec.describe SessionsController do
context 'when not signed in' do context 'when not signed in' do
it 'sets the caller_id in the context' do it 'sets the caller_id in the context' do
expect(controller).to receive(:new).and_wrap_original do |m, *args| expect(controller).to receive(:new).and_wrap_original do |m, *args|
expect(Labkit::Context.current.to_h) expect(Gitlab::ApplicationContext.current)
.to include('meta.caller_id' => 'SessionsController#new') .to include('meta.caller_id' => 'SessionsController#new')
expect(Labkit::Context.current.to_h) expect(Gitlab::ApplicationContext.current)
.not_to include('meta.user') .not_to include('meta.user')
m.call(*args) m.call(*args)
...@@ -557,9 +557,9 @@ RSpec.describe SessionsController do ...@@ -557,9 +557,9 @@ RSpec.describe SessionsController do
it 'sets the caller_id in the context' do it 'sets the caller_id in the context' do
allow_any_instance_of(User).to receive(:lock_access!).and_wrap_original do |m, *args| allow_any_instance_of(User).to receive(:lock_access!).and_wrap_original do |m, *args|
expect(Labkit::Context.current.to_h) expect(Gitlab::ApplicationContext.current)
.to include('meta.caller_id' => 'SessionsController#create') .to include('meta.caller_id' => 'SessionsController#create')
expect(Labkit::Context.current.to_h) expect(Gitlab::ApplicationContext.current)
.not_to include('meta.user') .not_to include('meta.user')
m.call(*args) m.call(*args)
......
...@@ -27,6 +27,20 @@ RSpec.describe Gitlab::ApplicationContext do ...@@ -27,6 +27,20 @@ RSpec.describe Gitlab::ApplicationContext do
end end
end end
describe '.with_raw_context' do
it 'yields the block' do
expect { |b| described_class.with_raw_context({}, &b) }.to yield_control
end
it 'passes the attributes unaltered on to labkit' do
attrs = { foo: :bar }
expect(Labkit::Context).to receive(:with_context).with(attrs)
described_class.with_raw_context(attrs) {}
end
end
describe '.push' do describe '.push' do
it 'passes the expected context on to labkit' do it 'passes the expected context on to labkit' do
fake_proc = duck_type(:call) fake_proc = duck_type(:call)
...@@ -110,7 +124,7 @@ RSpec.describe Gitlab::ApplicationContext do ...@@ -110,7 +124,7 @@ RSpec.describe Gitlab::ApplicationContext do
it 'does not cause queries' do it 'does not cause queries' do
context = described_class.new(project: create(:project), namespace: create(:group, :nested), user: create(:user)) context = described_class.new(project: create(:project), namespace: create(:group, :nested), user: create(:user))
expect { context.use { Labkit::Context.current.to_h } }.not_to exceed_query_limit(0) expect { context.use { Gitlab::ApplicationContext.current } }.not_to exceed_query_limit(0)
end end
end end
end end
...@@ -18,7 +18,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Server do ...@@ -18,7 +18,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::WorkerContext::Server do
worker_context user: nil worker_context user: nil
def perform(identifier, *args) def perform(identifier, *args)
self.class.contexts.merge!(identifier => Labkit::Context.current.to_h) self.class.contexts.merge!(identifier => Gitlab::ApplicationContext.current)
end end
end end
end end
......
...@@ -105,7 +105,7 @@ RSpec.describe API::API do ...@@ -105,7 +105,7 @@ RSpec.describe API::API do
it 'logs all application context fields' do it 'logs all application context fields' do
allow_any_instance_of(Gitlab::GrapeLogging::Loggers::ContextLogger).to receive(:parameters) do allow_any_instance_of(Gitlab::GrapeLogging::Loggers::ContextLogger).to receive(:parameters) do
Labkit::Context.current.to_h.tap do |log_context| Gitlab::ApplicationContext.current.tap do |log_context|
expect(log_context).to match('correlation_id' => an_instance_of(String), expect(log_context).to match('correlation_id' => an_instance_of(String),
'meta.caller_id' => '/api/:version/projects/:id/issues', 'meta.caller_id' => '/api/:version/projects/:id/issues',
'meta.remote_ip' => an_instance_of(String), 'meta.remote_ip' => an_instance_of(String),
...@@ -121,7 +121,7 @@ RSpec.describe API::API do ...@@ -121,7 +121,7 @@ RSpec.describe API::API do
it 'skips fields that do not apply' do it 'skips fields that do not apply' do
allow_any_instance_of(Gitlab::GrapeLogging::Loggers::ContextLogger).to receive(:parameters) do allow_any_instance_of(Gitlab::GrapeLogging::Loggers::ContextLogger).to receive(:parameters) do
Labkit::Context.current.to_h.tap do |log_context| Gitlab::ApplicationContext.current.tap do |log_context|
expect(log_context).to match('correlation_id' => an_instance_of(String), expect(log_context).to match('correlation_id' => an_instance_of(String),
'meta.caller_id' => '/api/:version/users', 'meta.caller_id' => '/api/:version/users',
'meta.remote_ip' => an_instance_of(String), 'meta.remote_ip' => an_instance_of(String),
......
...@@ -304,10 +304,10 @@ RSpec.configure do |config| ...@@ -304,10 +304,10 @@ RSpec.configure do |config|
RequestStore.clear! RequestStore.clear!
end end
config.around do |example| config.around(:example, :context_aware) do |example|
# Wrap each example in it's own context to make sure the contexts don't # Wrap each example in it's own context to make sure the contexts don't
# leak # leak
Labkit::Context.with_context { example.run } Gitlab::ApplicationContext.with_raw_context { example.run }
end end
config.around do |example| config.around do |example|
......
# frozen_string_literal: true # frozen_string_literal: true
RSpec.shared_examples 'API::CI::Runner application context metadata' do |api_route| RSpec.shared_examples 'API::CI::Runner application context metadata' do |api_route|
it 'contains correct context metadata' do it 'contains correct context metadata', :context_aware do
# Avoids popping the context from the thread so we can # Avoids popping the context from the thread so we can
# check its content after the request. # check its content after the request.
allow(Labkit::Context).to receive(:pop) allow(Labkit::Context).to receive(:pop)
send_request send_request
Labkit::Context.with_context do |context| Gitlab::ApplicationContext.with_raw_context do |context|
expected_context = { expected_context = {
'meta.caller_id' => api_route, 'meta.caller_id' => api_route,
'meta.user' => job.user.username, 'meta.user' => job.user.username,
......
# frozen_string_literal: true # frozen_string_literal: true
RSpec.shared_examples 'storing arguments in the application context' do RSpec.shared_examples 'storing arguments in the application context' do
around do |example| it 'places the expected params in the application context', :context_aware do
Labkit::Context.with_context { example.run }
end
it 'places the expected params in the application context' do
# Stub the clearing of the context so we can validate it later # Stub the clearing of the context so we can validate it later
# The `around` block above makes sure we do clean it up later
allow(Labkit::Context).to receive(:pop) allow(Labkit::Context).to receive(:pop)
subject subject
Labkit::Context.with_context do |context| expect(Gitlab::ApplicationContext.current).to include(log_hash(expected_params))
expect(context.to_h)
.to include(log_hash(expected_params))
end
end end
def log_hash(hash) def log_hash(hash)
......
...@@ -101,7 +101,7 @@ RSpec.describe BackgroundMigrationWorker, :clean_gitlab_redis_shared_state do ...@@ -101,7 +101,7 @@ RSpec.describe BackgroundMigrationWorker, :clean_gitlab_redis_shared_state do
it 'sets the class that will be executed as the caller_id' do it 'sets the class that will be executed as the caller_id' do
expect(Gitlab::BackgroundMigration).to receive(:perform) do expect(Gitlab::BackgroundMigration).to receive(:perform) do
expect(Labkit::Context.current.to_h).to include('meta.caller_id' => 'Foo') expect(Gitlab::ApplicationContext.current).to include('meta.caller_id' => 'Foo')
end end
worker.perform('Foo', [10, 20]) worker.perform('Foo', [10, 20])
......
...@@ -103,7 +103,7 @@ RSpec.describe WorkerContext do ...@@ -103,7 +103,7 @@ RSpec.describe WorkerContext do
describe '#with_context' do describe '#with_context' do
it 'allows modifying context when the job is running' do it 'allows modifying context when the job is running' do
worker.new.with_context(user: build_stubbed(:user, username: 'jane-doe')) do worker.new.with_context(user: build_stubbed(:user, username: 'jane-doe')) do
expect(Labkit::Context.current.to_h).to include('meta.user' => 'jane-doe') expect(Gitlab::ApplicationContext.current).to include('meta.user' => 'jane-doe')
end end
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