track cost even with throttling disabled

refs FOO-739

TEST PLAN:
  1) disable request throttling
  2) run some requests that are expensive
  3) costs should still be tracked in headers

Change-Id: Iec1a60f797451c789c3cdb79682757b8c354005f
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/243569
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Ethan Vizitei <evizitei@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>

app/middleware/request_throttle.rb

@@ -111,7 +111,7 @@ class RequestThrottle
           Rails.logger.info("blocking request due to throttling, client id: #{client_identifier(request)} bucket: #{bucket.to_json}")
           return false
         else
-          Rails.logger.info("WOULD HAVE blocked request due to throttling, client id: #{client_identifier(request)} bucket: #{bucket.to_json}")
+          Rails.logger.info("WOULD HAVE throttled request (config disabled), client id: #{client_identifier(request)} bucket: #{bucket.to_json}")
         end
       end
       return true
@@ -308,12 +308,10 @@ class RequestThrottle
     # expecting the block to return the final cost. It then increments again,
     # subtracting the initial up_front_cost from the final cost to erase it.
     def reserve_capacity(up_front_cost = self.up_front_cost)
-      return (self.count = yield) unless RequestThrottle.enabled?
-
       increment(0, up_front_cost)
       cost = yield
     ensure
-      increment(cost || 0, -up_front_cost) if RequestThrottle.enabled?
+      increment(cost || 0, -up_front_cost)
     end
 
     def full?

spec/lib/canvas/request_throttle_spec.rb

@@ -107,7 +107,7 @@ describe 'RequestThrottle' do
       expect(strip_variable_headers(throttler.call(request_user_1))).to eq response
     end
 
-    it "should have headers even when disabled" do
+    it "should have headers even when disabled including cost tracking" do
       allow(RequestThrottle).to receive(:enabled?).and_return(false)
       allow(throttler).to receive(:calculate_cost).and_return(30)
 
@@ -115,7 +115,11 @@ describe 'RequestThrottle' do
       expected[1]['X-Request-Cost'] = '30'
       # hwm of 600 - cost of the request
       expected[1]['X-Rate-Limit-Remaining'] = '570.0'
-      expect(throttler.call(request_user_1)).to eq expected
+      output = throttler.call(request_user_1)
+      expect(output[1]['X-Request-Cost']).to eq('30')
+      remaining = output[1]['X-Rate-Limit-Remaining'].to_f
+      expect(remaining > 570.0).to be_truthy
+      expect(remaining < 580.0).to be_truthy
     end
 
     it "should blacklist based on ip" do
@@ -414,12 +418,21 @@ describe 'RequestThrottle' do
           expect(@bucket.get_up_front_cost_for_path("/somethingelse")).to eq @bucket.up_front_cost
         end
 
-        it "does nothing if disabled" do
-          expect(RequestThrottle).to receive(:enabled?).twice.and_return(false)
-          expect(@bucket).to receive(:increment).never
+        it "still tracks cost when disabled (for debugging)" do
+          allow(RequestThrottle).to receive(:enabled?).and_return(false)
+          expect(@bucket).to receive(:increment).twice
           @bucket.reserve_capacity {}
         end
 
+        it "will always be allowed when disabled, even with full bucket" do
+          allow(RequestThrottle).to receive(:enabled?).and_return(false)
+          req = request_logged_out
+          allow(req).to receive(:fullpath).and_return("/")
+          allow(req).to receive(:env).and_return({'canvas.request_throttle.user_id' => ['123']})
+          allow(@bucket).to receive(:full?).and_return(true)
+          expect(throttler.allowed?(request_logged_out, @bucket)).to be_truthy
+        end
+
         after do
           Timecop.safe_mode = true
         end