From 0109f048dd274e0c108e1ed4021fac35a6c93f87 Mon Sep 17 00:00:00 2001 From: Jason Frey Date: Wed, 20 Jan 2021 16:00:08 -0500 Subject: [PATCH] Fix key collision with Thread[:current_request] Now that we've moved to manageiq-loggers, the ContainerLogger has code that expects that if Thread[:current_request] is set, it will be a request object that responds to :request_id. The RequestStartedOnMiddleware, however, happens to use the same key and stores only the request path, leading to `undefined method request_id for "/path/to/wherever"` Since this particular key's usage is isolated to the middleware, we can name it whatever we want, so this commit chooses a name that is less likely to collide. --- lib/request_started_on_middleware.rb | 8 ++++---- spec/lib/request_started_on_middleware_spec.rb | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/request_started_on_middleware.rb b/lib/request_started_on_middleware.rb index 25e1f597b17..118e652b66d 100644 --- a/lib/request_started_on_middleware.rb +++ b/lib/request_started_on_middleware.rb @@ -14,13 +14,13 @@ def call(env) complete_request end - def start_request(path, started_on) - Thread.current[:current_request] = path + def start_request(path_info, started_on) + Thread.current[:current_request_path_info] = path_info Thread.current[:current_request_started_on] = started_on end def complete_request - Thread.current[:current_request] = nil + Thread.current[:current_request_path_info] = nil Thread.current[:current_request_started_on] = nil end @@ -29,7 +29,7 @@ def self.long_running_requests allowable_request_start_time = long_request.ago relevant_thread_list.each do |thread| - request = thread[:current_request] + request = thread[:current_request_path_info] started_on = thread[:current_request_started_on] # There's a race condition where the complete_request method runs in another diff --git a/spec/lib/request_started_on_middleware_spec.rb b/spec/lib/request_started_on_middleware_spec.rb index e9ae03e2aa6..7fbbcae5cf9 100644 --- a/spec/lib/request_started_on_middleware_spec.rb +++ b/spec/lib/request_started_on_middleware_spec.rb @@ -7,7 +7,7 @@ let(:fake_threads) { [@fake_thread] } it "returns request, duration and thread" do - @fake_thread = {:current_request => "/api/ping", :current_request_started_on => 3.minutes.ago} + @fake_thread = {:current_request_path_info => "/api/ping", :current_request_started_on => 3.minutes.ago} long_requests = described_class.long_running_requests.first expect(long_requests[0]).to eql "/api/ping" expect(long_requests[1]).to be_within(0.1).of(Time.now.utc - 3.minutes.ago) @@ -15,7 +15,7 @@ end it "skips threads that haven't timed out yet" do - @fake_thread = {:current_request => "/api/ping", :current_request_started_on => 30.seconds.ago} + @fake_thread = {:current_request_path_info => "/api/ping", :current_request_started_on => 30.seconds.ago} expect(described_class.long_running_requests).to be_empty end