From 832d38269d957d1788b4df76ea7a73b6dee8c143 Mon Sep 17 00:00:00 2001 From: Roman Samoilov <2270393+rsamoilov@users.noreply.github.com> Date: Wed, 4 Feb 2026 16:41:34 +0000 Subject: [PATCH] Use fiber storage for Rage logger * store logger info in fiber storage so that it's automatically passed to child fibers * split logger info into separate keys to ensure correct isolation between fibers --- lib/rage/controller/api.rb | 6 +- lib/rage/deferred/context.rb | 6 +- lib/rage/deferred/task.rb | 4 +- lib/rage/fiber_scheduler.rb | 3 - lib/rage/log_processor.rb | 16 ++- lib/rage/logger/json_formatter.rb | 5 +- lib/rage/logger/logger.rb | 21 ++-- lib/rage/logger/text_formatter.rb | 5 +- spec/deferred/context_spec.rb | 8 +- spec/deferred/task_spec.rb | 6 +- spec/json_formatter_spec.rb | 64 ++++++------ spec/log_processor_spec.rb | 12 ++- spec/logger_spec.rb | 155 +++++++++++++++++++++++++++--- 13 files changed, 218 insertions(+), 93 deletions(-) diff --git a/lib/rage/controller/api.rb b/lib/rage/controller/api.rb index 56b93968..5efa870d 100644 --- a/lib/rage/controller/api.rb +++ b/lib/rage/controller/api.rb @@ -155,11 +155,11 @@ def __run_#{action} context = {} append_info_to_payload(context) - log_context = Thread.current[:rage_logger][:context] + log_context = Fiber[:__rage_logger_context] if log_context.empty? - Thread.current[:rage_logger][:context] = context + Fiber[:__rage_logger_context] = context else - Thread.current[:rage_logger][:context] = log_context.merge(context) + Fiber[:__rage_logger_context] = log_context.merge(context) end RUBY end} diff --git a/lib/rage/deferred/context.rb b/lib/rage/deferred/context.rb index a350a009..bb739560 100644 --- a/lib/rage/deferred/context.rb +++ b/lib/rage/deferred/context.rb @@ -7,15 +7,13 @@ # @private class Rage::Deferred::Context def self.build(task, args, kwargs) - logger = Thread.current[:rage_logger] - [ task, args.empty? ? nil : args, kwargs.empty? ? nil : kwargs, nil, - logger&.dig(:tags), - logger&.dig(:context), + Fiber[:__rage_logger_tags], + Fiber[:__rage_logger_context], nil ] end diff --git a/lib/rage/deferred/task.rb b/lib/rage/deferred/task.rb index 95b69815..90defc06 100644 --- a/lib/rage/deferred/task.rb +++ b/lib/rage/deferred/task.rb @@ -93,10 +93,10 @@ def __perform(context) log_context = Rage::Deferred::Context.get_log_context(context) if log_tags.is_a?(Array) - Thread.current[:rage_logger] = { tags: log_tags, context: log_context } + Fiber[:__rage_logger_tags], Fiber[:__rage_logger_context] = log_tags, log_context elsif log_tags # support the previous format where only `request_id` was passed - Thread.current[:rage_logger] = { tags: [log_tags], context: {} } + Fiber[:__rage_logger_tags], Fiber[:__rage_logger_context] = [log_tags], {} end end diff --git a/lib/rage/fiber_scheduler.rb b/lib/rage/fiber_scheduler.rb index 1beecde3..0c74c773 100644 --- a/lib/rage/fiber_scheduler.rb +++ b/lib/rage/fiber_scheduler.rb @@ -126,10 +126,7 @@ def fiber(&block) end else # the fiber was created in the user code - logger = Thread.current[:rage_logger] - Fiber.new(blocking: false) do - Thread.current[:rage_logger] = logger Rage::Telemetry.tracer.span_core_fiber_spawn(parent:) do Fiber.current.__set_result(block.call) end diff --git a/lib/rage/log_processor.rb b/lib/rage/log_processor.rb index d2b2e6a4..a38141f2 100644 --- a/lib/rage/log_processor.rb +++ b/lib/rage/log_processor.rb @@ -21,15 +21,13 @@ def add_custom_tags(tag_objects) end def finalize_request_logger(env, response, params) - logger = Thread.current[:rage_logger] - duration = ( - (Process.clock_gettime(Process::CLOCK_MONOTONIC) - logger[:request_start]) * 1000 + (Process.clock_gettime(Process::CLOCK_MONOTONIC) - Fiber[:__rage_logger_request_start]) * 1000 ).round(2) - logger[:final] = { env:, params:, response:, duration: } + Fiber[:__rage_logger_final] = { env:, params:, response:, duration: } Rage.logger.info(nil) - logger[:final] = nil + Fiber[:__rage_logger_final] = nil end private @@ -106,11 +104,9 @@ def rebuild! def init_request_logger(env) env["rage.request_id"] ||= Iodine::Rack::Utils.gen_request_tag - Thread.current[:rage_logger] = { - tags: #{build_static_tags}, - context: #{build_static_context}, - request_start: Process.clock_gettime(Process::CLOCK_MONOTONIC) - } + Fiber[:__rage_logger_tags] = #{build_static_tags} + Fiber[:__rage_logger_context] = #{build_static_context} + Fiber[:__rage_logger_request_start] = Process.clock_gettime(Process::CLOCK_MONOTONIC) end RUBY end diff --git a/lib/rage/logger/json_formatter.rb b/lib/rage/logger/json_formatter.rb index 9b91f382..bdc42d10 100644 --- a/lib/rage/logger/json_formatter.rb +++ b/lib/rage/logger/json_formatter.rb @@ -26,8 +26,7 @@ def initialize end def call(severity, timestamp, _, message) - logger = Thread.current[:rage_logger] || { tags: [], context: {} } - tags, context = logger[:tags], logger[:context] + tags, context = Fiber[:__rage_logger_tags] || [], Fiber[:__rage_logger_context] || {} if !context.empty? context_msg = "" @@ -50,7 +49,7 @@ def call(severity, timestamp, _, message) msg << "]," end - if (final = logger[:final]) + if (final = Fiber[:__rage_logger_final]) params, env = final[:params], final[:env] if params && params[:controller] return "#{tags_msg}\"timestamp\":\"#{timestamp}\",\"pid\":\"#{@pid}\",\"level\":\"info\",\"method\":\"#{env["REQUEST_METHOD"]}\",\"path\":\"#{env["PATH_INFO"]}\",\"controller\":\"#{Rage::Router::Util.path_to_name(params[:controller])}\",\"action\":\"#{params[:action]}\",#{context_msg}\"status\":#{final[:response][0]},\"duration\":#{final[:duration]}}\n" diff --git a/lib/rage/logger/logger.rb b/lib/rage/logger/logger.rb index 2f2a88c2..82d28b46 100644 --- a/lib/rage/logger/logger.rb +++ b/lib/rage/logger/logger.rb @@ -140,17 +140,17 @@ def dynamic_context=(dynamic_context) # Rage.logger.info "cache miss" # end def with_context(context) - old_context = (Thread.current[:rage_logger] ||= { tags: [], context: {} })[:context] + old_context = (Fiber[:__rage_logger_context] ||= {}) if old_context.empty? # there's nothing in the context yet - Thread.current[:rage_logger][:context] = context + Fiber[:__rage_logger_context] = context else # it's not the first `with_context` call in the chain - Thread.current[:rage_logger][:context] = old_context.merge(context) + Fiber[:__rage_logger_context] = old_context.merge(context) end yield(self) ensure - Thread.current[:rage_logger][:context] = old_context + Fiber[:__rage_logger_context] = old_context end # Add a custom tag to an entry. @@ -161,11 +161,11 @@ def with_context(context) # Rage.logger.info "success" # end def tagged(*tags) - old_tags = (Thread.current[:rage_logger] ||= { tags: [], context: {} })[:tags] - Thread.current[:rage_logger][:tags] = old_tags + tags + old_tags = (Fiber[:__rage_logger_tags] ||= []) + Fiber[:__rage_logger_tags] = old_tags + tags yield(self) ensure - Thread.current[:rage_logger][:tags] = old_tags + Fiber[:__rage_logger_tags] = old_tags end alias_method :with_tag, :tagged @@ -217,17 +217,16 @@ def #{level_name}(msg = nil) parameters = Rage::Internal.build_arguments(call_method, { severity: ":#{level_name}", - tags: "logger[:tags].freeze", - context: "logger[:context].freeze", + tags: "Fiber[:__rage_logger_tags].freeze", + context: "Fiber[:__rage_logger_context].freeze", message: "block_given? ? yield : msg", - request_info: "logger[:final].freeze" + request_info: "Fiber[:__rage_logger_final].freeze" }) <<~RUBY def #{level_name}(msg = nil) #{with_dynamic_tags_and_context do <<~RUBY - logger = Thread.current[:rage_logger] || { tags: [], context: {} } @external_logger.wrapped.call(#{parameters}) RUBY end} diff --git a/lib/rage/logger/text_formatter.rb b/lib/rage/logger/text_formatter.rb index bca63127..6c7b0d96 100644 --- a/lib/rage/logger/text_formatter.rb +++ b/lib/rage/logger/text_formatter.rb @@ -24,15 +24,14 @@ def initialize end def call(severity, timestamp, _, message) - logger = Thread.current[:rage_logger] || { tags: [], context: {} } - tags, context = logger[:tags], logger[:context] + tags, context = Fiber[:__rage_logger_tags] || [], Fiber[:__rage_logger_context] || {} if !context.empty? context_msg = "" context.each { |k, v| context_msg << "#{k}=#{v} " } end - if (final = logger[:final]) + if (final = Fiber[:__rage_logger_final]) params, env = final[:params], final[:env] tags = tags.map { |tag| "[#{tag}]" }.join diff --git a/spec/deferred/context_spec.rb b/spec/deferred/context_spec.rb index 7da85b7c..9db252f5 100644 --- a/spec/deferred/context_spec.rb +++ b/spec/deferred/context_spec.rb @@ -5,10 +5,10 @@ let(:kwargs) { { key: "value" } } after do - Thread.current[:rage_logger] = nil + Fiber[:__rage_logger_tags] = nil end - context "when Thread.current[:rage_logger] is not set" do + context "when rage_logger is not set" do it "builds context with nil request_id" do context = described_class.build(task, args, kwargs) expect(context[0]).to eq(task) @@ -25,9 +25,9 @@ end end - context "when Thread.current[:rage_logger] is set" do + context "when rage_logger is set" do before do - Thread.current[:rage_logger] = { tags: ["req-123", "test"] } + Fiber[:__rage_logger_tags] = ["req-123", "test"] end it "builds context including log tags" do diff --git a/spec/deferred/task_spec.rb b/spec/deferred/task_spec.rb index 881cacda..a2572b4b 100644 --- a/spec/deferred/task_spec.rb +++ b/spec/deferred/task_spec.rb @@ -86,7 +86,8 @@ def perform(arg, kwarg:) end after do - Thread.current[:rage_logger] = nil + Fiber[:__rage_logger_tags] = nil + Fiber[:__rage_logger_context] = nil end it "calls perform with correct arguments" do @@ -97,7 +98,8 @@ def perform(arg, kwarg:) it "logs with context and tag" do task.__perform(context) expect(logger).to have_received(:with_context).with({ task: "MyTask", attempt: 2 }) - expect(Thread.current[:rage_logger]).to eq({ tags: ["request-id"], context: {} }) + expect(Fiber[:__rage_logger_tags]).to eq(["request-id"]) + expect(Fiber[:__rage_logger_context]).to eq({}) end it "returns true" do diff --git a/spec/json_formatter_spec.rb b/spec/json_formatter_spec.rb index a8c57131..99ec4579 100644 --- a/spec/json_formatter_spec.rb +++ b/spec/json_formatter_spec.rb @@ -8,7 +8,9 @@ let(:message) { "test message" } after do - Thread.current[:rage_logger] = nil + Fiber[:__rage_logger_tags] = nil + Fiber[:__rage_logger_context] = nil + Fiber[:__rage_logger_final] = nil end context "with no logger info" do @@ -37,7 +39,8 @@ context "with one tag" do before do - Thread.current[:rage_logger] = { tags: ["json-test-tag"], context: {} } + Fiber[:__rage_logger_tags] = ["json-test-tag"] + Fiber[:__rage_logger_context] = {} end it "correctly formats the message" do @@ -52,7 +55,8 @@ context "with custom context" do before do - Thread.current[:rage_logger] = { tags: ["json-test-tag"], context: { user_id: "test-1", account_id: "test-2" } } + Fiber[:__rage_logger_tags] = ["json-test-tag"] + Fiber[:__rage_logger_context] = { user_id: "test-1", account_id: "test-2" } end it "correctly formats the message" do @@ -71,10 +75,8 @@ context "with multiple tags" do before do - Thread.current[:rage_logger] = { - tags: ["json-test-tag-1", "json-test-tag-2"], - context: {} - } + Fiber[:__rage_logger_tags] = ["json-test-tag-1", "json-test-tag-2"] + Fiber[:__rage_logger_context] = {} end it "correctly formats the message" do @@ -89,10 +91,8 @@ context "with custom context" do before do - Thread.current[:rage_logger] = { - tags: ["json-test-tag-1", "json-test-tag-2"], - context: { user_id: "test-1", account_id: "test-2" } - } + Fiber[:__rage_logger_tags] = ["json-test-tag-1", "json-test-tag-2"] + Fiber[:__rage_logger_context] = { user_id: "test-1", account_id: "test-2" } end it "correctly formats the message" do @@ -113,15 +113,13 @@ before do stub_const("UserProfilesController", double(name: "UserProfilesController")) - Thread.current[:rage_logger] = { - tags: ["json-test-tag"], - context: {}, - final: { - env: { "REQUEST_METHOD" => "POST", "PATH_INFO" => "/user_profiles/12345" }, - params: { controller: "user_profiles", action: "create" }, - response: [207, {}, []], - duration: 1.234 - } + Fiber[:__rage_logger_tags] = ["json-test-tag"] + Fiber[:__rage_logger_context] = {} + Fiber[:__rage_logger_final] = { + env: { "REQUEST_METHOD" => "POST", "PATH_INFO" => "/user_profiles/12345" }, + params: { controller: "user_profiles", action: "create" }, + response: [207, {}, []], + duration: 1.234 } end @@ -142,7 +140,7 @@ context "with custom tags" do before do - Thread.current[:rage_logger][:tags] << "custom-tag-1" << "custom-tag-2" + Fiber[:__rage_logger_tags] << "custom-tag-1" << "custom-tag-2" end it "correctly formats the message" do @@ -162,7 +160,7 @@ context "with custom context" do before do - Thread.current[:rage_logger][:context] = { user_id: "test-1", account_id: "test-2" } + Fiber[:__rage_logger_context] = { user_id: "test-1", account_id: "test-2" } end it "correctly formats the message" do @@ -186,7 +184,7 @@ context "with custom context" do before do - Thread.current[:rage_logger][:context] = { user_id: "test-1", account_id: "test-2" } + Fiber[:__rage_logger_context] = { user_id: "test-1", account_id: "test-2" } end it "correctly formats the message" do @@ -209,15 +207,13 @@ context "with no controller/action info" do before do - Thread.current[:rage_logger] = { - tags: ["json-test-tag"], - context: {}, - final: { - env: { "REQUEST_METHOD" => "POST", "PATH_INFO" => "/user_profiles/12345" }, - params: nil, - response: [207, {}, []], - duration: 1.234 - } + Fiber[:__rage_logger_tags] = ["json-test-tag"] + Fiber[:__rage_logger_context] = {} + Fiber[:__rage_logger_final] = { + env: { "REQUEST_METHOD" => "POST", "PATH_INFO" => "/user_profiles/12345" }, + params: nil, + response: [207, {}, []], + duration: 1.234 } end @@ -236,8 +232,8 @@ context "with custom tags and context" do before do - Thread.current[:rage_logger][:tags] << "custom-tag" - Thread.current[:rage_logger][:context] = { user_id: "test-1", account_id: "test-2" } + Fiber[:__rage_logger_tags] << "custom-tag" + Fiber[:__rage_logger_context] = { user_id: "test-1", account_id: "test-2" } end it "correctly formats the message" do diff --git a/spec/log_processor_spec.rb b/spec/log_processor_spec.rb index e4d1ff7d..592a8536 100644 --- a/spec/log_processor_spec.rb +++ b/spec/log_processor_spec.rb @@ -4,7 +4,13 @@ RSpec.describe Rage::LogProcessor do describe "#init_request_logger" do let(:log_processor) { described_class.new } - let(:log_context) { Thread.current[:rage_logger] } + let(:log_context) do + { + tags: Fiber[:__rage_logger_tags], + context: Fiber[:__rage_logger_context], + request_start: Fiber[:__rage_logger_request_start] + } + end let(:env) { {} } let(:request_tag) { "test-request-id-tag" } @@ -22,7 +28,9 @@ end after do - Thread.current[:rage_logger] = nil + Fiber[:__rage_logger_tags] = nil + Fiber[:__rage_logger_context] = nil + Fiber[:__rage_logger_request_start] = nil end it "correctly initializes static logger" do diff --git a/spec/logger_spec.rb b/spec/logger_spec.rb index 41595c77..4720120b 100644 --- a/spec/logger_spec.rb +++ b/spec/logger_spec.rb @@ -6,13 +6,13 @@ subject { described_class.new(io) } let(:io) { StringIO.new } + let(:log) { io.tap(&:rewind).read } before do - Thread.current[:rage_logger] = { - tags: ["my_test_tag"], - context: {}, - request_start: 123 - } + Fiber[:__rage_logger_tags] = ["my_test_tag"] + Fiber[:__rage_logger_context] = {} + Fiber[:__rage_logger_request_start] = 123 + Fiber[:__rage_logger_final] = nil allow(Iodine::Rack::Utils).to receive(:gen_timestamp).and_return("very_accurate_timestamp") allow(Process).to receive(:pid).and_return(777) @@ -128,7 +128,7 @@ referenced_tags = nil subject.tagged("rspec") do - referenced_tags = Thread.current[:rage_logger][:tags] + referenced_tags = Fiber[:__rage_logger_tags] subject.info "test passed" end @@ -186,7 +186,7 @@ referenced_context = nil subject.with_context(rspec: true) do - referenced_context = Thread.current[:rage_logger][:context] + referenced_context = Fiber[:__rage_logger_context] subject.info "text" end @@ -197,7 +197,9 @@ context "outside the request/response cycle" do before do - Thread.current[:rage_logger] = nil + Fiber[:__rage_logger_tags] = nil + Fiber[:__rage_logger_context] = nil + Fiber[:__rage_logger_final] = nil end it "correctly adds an entry" do @@ -222,7 +224,7 @@ context "with request logs" do before do - Thread.current[:rage_logger][:final] = { + Fiber[:__rage_logger_final] = { env: { "REQUEST_METHOD" => "GET", "PATH_INFO" => "/test_path" }, params: { controller: "rspec", action: "index", id: "123" }, response: [300, {}, []], @@ -239,7 +241,7 @@ context "with no params" do before do - Thread.current[:rage_logger][:final].delete(:params) + Fiber[:__rage_logger_final].delete(:params) end it "correctly add an entry" do @@ -250,7 +252,7 @@ context "with empty params" do before do - Thread.current[:rage_logger][:final][:params] = {} + Fiber[:__rage_logger_final][:params] = {} end it "correctly add an entry" do @@ -261,7 +263,7 @@ context "with namespaced controllers" do before do - Thread.current[:rage_logger][:final][:params][:controller] = "api/v2/users" + Fiber[:__rage_logger_final][:params][:controller] = "api/v2/users" stub_const("Api::V2::UsersController", double(name: "Api::V2::UsersController")) end @@ -312,6 +314,9 @@ end it "delegates logger calls to external logger" do + p "grep_me !!!!!!!" + p Fiber.current.storage + expect(external_logger).to receive(:info).with("[my_test_tag] timestamp=very_accurate_timestamp pid=777 level=info message=test\n") subject.info "test" @@ -584,4 +589,130 @@ def call(message:, tags:) end end end + + context "with intersecting fibers" do + before :all do + Fiber.set_scheduler(Rage::FiberScheduler.new) + end + + after :all do + Fiber.set_scheduler(nil) + end + + context "with separate fibers" do + it "correctly isolates context" do + within_reactor do + f1 = Fiber.schedule do + subject.with_context(fiber_no: 1, test_key_1: true) do + sleep 0.2 + subject.info "test fiber 1" + end + end + + f2 = Fiber.schedule do + subject.with_context(fiber_no: 2, test_key_2: true) do + sleep 0.1 + subject.info "test fiber 2" + end + end + + Fiber.await([f1, f2]) + + -> do + expect(log).to include("fiber_no=1 test_key_1=true message=test fiber 1") + expect(log).to include("fiber_no=2 test_key_2=true message=test fiber 2") + end + end + end + + it "correctly isolates tags" do + within_reactor do + f1 = Fiber.schedule do + subject.tagged("fiber_no_1") do + sleep 0.2 + subject.info "test fiber 1" + end + end + + f2 = Fiber.schedule do + subject.tagged("fiber_no_2") do + sleep 0.1 + subject.info "test fiber 2" + end + end + + Fiber.await([f1, f2]) + + -> do + expect(log).to match(/\[fiber_no_1\] [\w\s=]+ message=test fiber 1/) + expect(log).to match(/\[fiber_no_2\] [\w\s=]+ message=test fiber 2/) + end + end + end + end + + context "with logging in base fiber" do + it "correctly isolates context" do + within_reactor do + subject.with_context(fiber_no: 0, test_key_0: true) do + f1 = Fiber.schedule do + subject.with_context(fiber_no: 1, test_key_1: true) do + sleep 0.2 + subject.info "test fiber 1" + end + end + + f2 = Fiber.schedule do + subject.with_context(fiber_no: 2, test_key_2: true) do + sleep 0.1 + subject.info "test fiber 2" + end + end + + subject.info "(1) test fiber 0" + Fiber.await([f1, f2]) + subject.info "(2) test fiber 0" + end + + -> do + expect(log).to include("fiber_no=1 test_key_0=true test_key_1=true message=test fiber 1") + expect(log).to include("fiber_no=2 test_key_0=true test_key_2=true message=test fiber 2") + expect(log).to include("fiber_no=0 test_key_0=true message=(1) test fiber 0") + expect(log).to include("fiber_no=0 test_key_0=true message=(2) test fiber 0") + end + end + end + + it "correctly isolates tags" do + within_reactor do + subject.tagged("fiber_no_0") do + f1 = Fiber.schedule do + subject.tagged("fiber_no_1") do + sleep 0.2 + subject.info "test fiber 1" + end + end + + f2 = Fiber.schedule do + subject.tagged("fiber_no_2") do + sleep 0.1 + subject.info "test fiber 2" + end + end + + subject.info "(1) test fiber 0" + Fiber.await([f1, f2]) + subject.info "(2) test fiber 0" + end + + -> do + expect(log).to include("[my_test_tag][fiber_no_0][fiber_no_1] timestamp=very_accurate_timestamp pid=777 level=info message=test fiber 1") + expect(log).to include("[my_test_tag][fiber_no_0][fiber_no_2] timestamp=very_accurate_timestamp pid=777 level=info message=test fiber 2") + expect(log).to include("[my_test_tag][fiber_no_0] timestamp=very_accurate_timestamp pid=777 level=info message=(1) test fiber 0") + expect(log).to include("[my_test_tag][fiber_no_0] timestamp=very_accurate_timestamp pid=777 level=info message=(2) test fiber 0") + end + end + end + end + end end