add addressbook performance tap option

closes CNVS-34528

another option for the address book strategy chosen in the plugin.
combines answers from current MessageableUser implementation with
traffic directed to the service as if it were in use. but the service is
instructed via `ignore_result` parameter to reply to canvas as quickly
as possible, so that if the service does have performance issues, actual
canvas performance is not affected.

test-plan:
- set up canvas to be able to talk to the address book service
- have a published course with an active teacher and an active student
- do _not_ replicate that data into the address book service; i.e. the
  service answer should be wrong if used
- choose the "AddressBook performance tap" as the address book
  implementation in the plugin setting
- logged in as the teacher, query for the student in the inbox. should
  find the student (backed by the messageableuser implementation)
- check the service request logs; even though the service response
  wasn't used, there should still be a request made against it
- take down the service entirely
- repeat the query in the inbox. should still get the result from the
  messageableuser implementation despite the service being unavailable

Change-Id: I031b7c397d2e20d74d7699a81ca8040064a8df75
Reviewed-on: https://gerrit.instructure.com/104112
Reviewed-by: Andrew Huff <[email protected]>
Tested-by: Jenkins
QA-Review: Deepeeca Soundarrajan <[email protected]>
Product-Review: Jacob Fugal <[email protected]>

lib/address_book.rb

@@ -7,6 +7,7 @@ module AddressBook
   STRATEGIES = {
     'messageable_user' => { implementation: AddressBook::MessageableUser, label: lambda{ I18n.t('MessageableUser library') } }.freeze,
     'microservice' => { implementation: AddressBook::Service, label: lambda{ I18n.t('AddressBook microservice') } }.freeze,
+    'performance_tap' => { implementation: AddressBook::PerformanceTap, label: lambda{ I18n.t('AddressBook performance tap') } }.freeze,
     'empty' => { implementation: AddressBook::Empty, label: lambda{ I18n.t('Empty stub (for testing only)') } }.freeze
   }.freeze
   DEFAULT_STRATEGY = 'messageable_user'

lib/address_book/performance_tap.rb

@@ -0,0 +1,66 @@
+module AddressBook
+  class PerformanceTap < AddressBook::MessageableUser
+    def initialize(sender)
+      super sender
+      @service_tap = AddressBook::Service.new(sender, ignore_result: true)
+    end
+
+    def known_users(users, options={})
+      @service_tap.known_users(users, options)
+      super
+    end
+
+    def known_in_context(context, is_admin=false)
+      @service_tap.known_in_context(context, is_admin)
+      super
+    end
+
+    def count_in_context(context)
+      @service_tap.count_in_context(context)
+      super
+    end
+
+    # makes a wrapper around two paginated collections, one for source and one
+    # for the tap. proxies pagination information and results to and from the
+    # source collection, but also executes pagination against the tap
+    # collection for every source page.
+    class TapProxy < PaginatedCollection::Proxy
+      def initialize(source_collection:, tap_collection:)
+        @source_collection = source_collection
+        super lambda{ |source_pager|
+          tap_pager = tap_collection.configure_pager(
+            tap_collection.new_pager,
+            per_page: source_pager.per_page,
+            total_entries: nil
+          )
+          tap_collection.execute_pager(tap_pager)
+          source_collection.execute_pager(source_pager)
+        }
+      end
+
+      def depth
+        @source_collection.depth
+      end
+
+      def new_pager
+        @source_collection.new_pager
+      end
+
+      def configure_pager(pager, options)
+        @source_collection.configure_pager(pager, options)
+      end
+    end
+
+    def search_users(options={})
+      TapProxy.new(
+        source_collection: super,
+        tap_collection: @service_tap.search_users(options)
+      )
+    end
+
+    def preload_users(users)
+      @service_tap.preload_users(users)
+      super
+    end
+  end
+end

lib/address_book/service.rb

@@ -1,15 +1,20 @@
 module AddressBook
   class Service < AddressBook::Base
+    def initialize(sender, ignore_result: false)
+      super(sender)
+      @ignore_result = ignore_result
+    end
+
     def known_users(users, options={})
       return [] if users.empty?
 
       # start with users I know directly
       user_ids = users.map{ |user| Shard.global_id_for(user) }
-      common_contexts = Services::AddressBook.common_contexts(@sender, user_ids)
+      common_contexts = Services::AddressBook.common_contexts(@sender, user_ids, @ignore_result)
 
       if options[:include_context]
         # add users any admin over the specified context knows indirectly
-        admin_contexts = Services::AddressBook.roles_in_context(options[:include_context], user_ids)
+        admin_contexts = Services::AddressBook.roles_in_context(options[:include_context], user_ids, @ignore_result)
         merge_common_contexts(common_contexts, admin_contexts)
       end
 
@@ -37,14 +42,14 @@ def known_users(users, options={})
 
     def known_in_context(context, is_admin=false)
       # just query, hydrate, and cache
-      user_ids, common_contexts = Services::AddressBook.known_in_context(@sender, context, is_admin)
+      user_ids, common_contexts = Services::AddressBook.known_in_context(@sender, context, is_admin, @ignore_result)
       users = hydrate(user_ids)
       cache_contexts(users, common_contexts)
       users
     end
 
     def count_in_context(context)
-      Services::AddressBook.count_in_context(@sender, context)
+      Services::AddressBook.count_in_context(@sender, context, @ignore_result)
     end
 
     class Bookmarker
@@ -89,7 +94,7 @@ def search_users(options={})
         end
 
         # query, hydrate, and cache
-        user_ids, common_contexts, cursors = Services::AddressBook.search_users(@sender, options, service_options)
+        user_ids, common_contexts, cursors = Services::AddressBook.search_users(@sender, options, service_options, @ignore_result)
         bookmarker.update(user_ids, cursors)
         users = hydrate(user_ids)
         cache_contexts(users, common_contexts)
@@ -107,7 +112,7 @@ def preload_users(users)
 
       # query only those directly known, but all are "whitelisted" for caching
       global_user_ids = users.map(&:global_id)
-      common_contexts = Services::AddressBook.common_contexts(@sender, global_user_ids)
+      common_contexts = Services::AddressBook.common_contexts(@sender, global_user_ids, @ignore_result)
       cache_contexts(users, common_contexts)
     end
 

lib/services/address_book.rb

@@ -13,14 +13,14 @@ class AddressBook
 
     # which of the users does the sender know, and what contexts do they and
     # the sender have in common?
-    def self.common_contexts(sender, users)
-      recipients(sender: sender, user_ids: users).common_contexts
+    def self.common_contexts(sender, users, ignore_result=false)
+      recipients(sender: sender, user_ids: users, ignore_result: ignore_result).common_contexts
     end
 
     # which of the users have roles in the context and what are those roles?
-    def self.roles_in_context(context, users)
+    def self.roles_in_context(context, users, ignore_result=false)
       context = context.course if context.is_a?(CourseSection)
-      recipients(context: context, user_ids: users).common_contexts
+      recipients(context: context, user_ids: users, ignore_result: ignore_result).common_contexts
     end
 
     # which users:
@@ -32,16 +32,16 @@ def self.roles_in_context(context, users)
     #
     #  - have roles in the context and what are those roles? (is_admin=true)
     #
-    def self.known_in_context(sender, context, is_admin=false)
-      params = { context: context }
+    def self.known_in_context(sender, context, is_admin=false, ignore_result=false)
+      params = { context: context, ignore_result: ignore_result }
       params[:sender] = sender unless is_admin
       response = recipients(params)
       [response.user_ids, response.common_contexts]
     end
 
     # how many users does the sender know in the context?
-    def self.count_in_context(sender, context)
-      count_recipients(sender: sender, context: context)
+    def self.count_in_context(sender, context, ignore_result=false)
+      count_recipients(sender: sender, context: context, ignore_result: ignore_result)
     end
 
     # of the users who are not in `exclude_ids` and whose name matches the
@@ -58,8 +58,9 @@ def self.count_in_context(sender, context)
     #  - have roles in the context and what are those roles? (context provided
     #    and is_admin true)
     #
-    def self.search_users(sender, options, service_options={})
+    def self.search_users(sender, options, service_options, ignore_result=false)
       params = options.slice(:search, :context, :exclude_ids, :weak_checks)
+      params[:ignore_result] = ignore_result
 
       # include sender only if not admin
       params[:sender] = sender unless options[:context] && options[:is_admin]
@@ -115,16 +116,21 @@ def fetch(path, params={})
         url = app_host + path
         url += '?' + params.to_query unless params.empty?
         fallback = { "records" => [] }
-        Canvas.timeout_protection("address_book") do
+        timeout_service_name = params[:ignore_result] == 1 ?
+          "address_book_performance_tap" :
+          "address_book"
+        Canvas.timeout_protection(timeout_service_name) do
           response = CanvasHttp.get(url, 'Authorization' => "Bearer #{jwt}")
-          if response.code.to_i == 200
-            return JSON.parse(response.body)
-          else
+          if ![200, 202].include?(response.code.to_i)
             Canvas::Errors.capture(CanvasHttp::InvalidResponseCodeError.new(response.code.to_i), {
               extra: { url: url, response: response.body },
               tags: { type: 'address_book_fault' }
             })
             return fallback
+          elsif params[:ignore_result] == 1
+            return fallback
+          else
+            return JSON.parse(response.body)
           end
         end || fallback
       end
@@ -148,6 +154,7 @@ def query_params(params={})
         query_params[:user_ids] = serialize_list(params[:user_ids]) if params[:user_ids]
         query_params[:exclude_ids] = serialize_list(params[:exclude_ids]) if params[:exclude_ids]
         query_params[:weak_checks] = 1 if params[:weak_checks]
+        query_params[:ignore_result] = 1 if params[:ignore_result]
         query_params
       end
 

spec/lib/address_book/service_spec.rb

@@ -42,6 +42,7 @@ def expand_cursors(returns)
   end
 
   def stub_common_contexts(args, returns={})
+    args << false # ignore_result
     returns = expand_common_contexts(returns)
     allow(Services::AddressBook).to receive(:common_contexts).with(*args).and_return(returns)
   end
@@ -94,6 +95,7 @@ def stub_common_contexts(args, returns={})
 
     describe "with optional :include_context" do
       def stub_roles_in_context(args, returns={})
+        args << false # ignore_result
         returns = expand_common_contexts(returns)
         allow(Services::AddressBook).to receive(:roles_in_context).with(*args).and_return(returns)
       end
@@ -232,6 +234,7 @@ def stub_roles_in_context(args, returns={})
 
   describe "known_in_context" do
     def stub_known_in_context(args, compact_returns={})
+      args << false # ignore_result
       user_ids = expand_user_ids(compact_returns)
       common_contexts = expand_common_contexts(compact_returns)
       allow(Services::AddressBook).to receive(:known_in_context).with(*args).and_return([user_ids, common_contexts])
@@ -311,7 +314,7 @@ def stub_known_in_context(args, compact_returns={})
     before do
       @course = course_model
       allow(Services::AddressBook).to receive(:count_in_context).
-        with(@sender, @course.global_asset_string).
+        with(@sender, @course.global_asset_string, false).
         and_return(3)
     end
 
@@ -322,6 +325,7 @@ def stub_known_in_context(args, compact_returns={})
 
   describe "search_users" do
     def stub_search_users(args, compact_returns={})
+      args << false # ignore_result
       user_ids = expand_user_ids(compact_returns)
       common_contexts = expand_common_contexts(compact_returns)
       cursors = expand_cursors(compact_returns)

spec/lib/services/address_book_spec.rb

@@ -173,6 +173,11 @@ def not_match(*args)
         Services::AddressBook.recipients(weak_checks: false)
       end
 
+      it "normalizes ignore_result to 1 if truthy" do
+        expect_request(with_param(:ignore_result, 1))
+        Services::AddressBook.recipients(ignore_result: true)
+      end
+
       it "reshapes results returned from service endpoint" do
         stub_response(anything, example_response)
         result = Services::AddressBook.recipients(@sender)
@@ -202,6 +207,26 @@ def not_match(*args)
         expect(result.cursors).to eq([])
       end
 
+      it "reads separate timeout setting when ignoring result (for performance tapping)" do
+        allow(Canvas).to receive(:redis_enabled?).and_return(true)
+        allow(Canvas).to receive(:redis).and_return(double)
+        allow(Canvas.redis).to receive(:get).with("service:timeouts:address_book_performance_tap:error_count").and_return(4)
+        expect(Rails.logger).to receive(:error).with("Skipping service call due to error count: address_book_performance_tap 4")
+        result = nil
+        expect { result = Services::AddressBook.recipients(sender: @sender, ignore_result: true) }.not_to raise_error
+        expect(result.user_ids).to eq([])
+        expect(result.common_contexts).to eq({})
+        expect(result.cursors).to eq([])
+      end
+
+      it "returns empty response when ignoring result, regardless of what service returns" do
+        stub_response(anything, example_response)
+        result = Services::AddressBook.recipients(sender: @sender, ignore_result: true)
+        expect(result.user_ids).to eql([])
+        expect(result.common_contexts).to eql({})
+        expect(result.cursors).to eql([])
+      end
+
       it "reports errors in service request but then returns sane value" do
         stub_response(anything, { 'errors' => { 'something' => 'went wrong' } }, status: 400)
         expect(Canvas::Errors).to receive(:capture)
@@ -355,45 +380,45 @@ def not_match(*args)
     describe "search_users" do
       it "makes a recipient request" do
         expect_request(%r{/recipients\?})
-        Services::AddressBook.search_users(@sender, search: 'bob')
+        Services::AddressBook.search_users(@sender, {search: 'bob'}, {})
       end
 
       it "only has search parameter by default" do
         expect_request(with_param(:search, 'bob'))
         expect_request(with_param_present(:in_context)).never
         expect_request(with_param_present(:exclude_ids)).never
         expect_request(with_param_present(:weak_checks)).never
-        Services::AddressBook.search_users(@sender, search: 'bob')
+        Services::AddressBook.search_users(@sender, {search: 'bob'}, {})
       end
 
       it "passes context to recipients call" do
         expect_request(with_param_present(:in_context))
-        Services::AddressBook.search_users(@sender, search: 'bob', context: @course)
+        Services::AddressBook.search_users(@sender, {search: 'bob', context: @course}, {})
       end
 
       it "includes sender if is_admin but no context given" do
         expect_request(with_param_present(:for_sender))
-        Services::AddressBook.search_users(@sender, search: 'bob', is_admin: true)
+        Services::AddressBook.search_users(@sender, {search: 'bob', is_admin: true}, {})
       end
 
       it "omits sender if is_admin specified with context" do
         expect_request(not_match(with_param_present(:sender)))
-        Services::AddressBook.search_users(@sender, search: 'bob', context: @course, is_admin: true)
+        Services::AddressBook.search_users(@sender, {search: 'bob', context: @course, is_admin: true}, {})
       end
 
       it "passes exclude_ids to recipients call" do
         expect_request(with_param_present(:exclude_ids))
-        Services::AddressBook.search_users(@sender, search: 'bob', exclude_ids: [1, 2, 3])
+        Services::AddressBook.search_users(@sender, {search: 'bob', exclude_ids: [1, 2, 3]}, {})
       end
 
       it "passes weak_checks flag along to recipients" do
         expect_request(with_param_present(:weak_checks))
-        Services::AddressBook.search_users(@sender, search: 'bob', weak_checks: true)
+        Services::AddressBook.search_users(@sender, {search: 'bob', weak_checks: true}, {})
       end
 
       it "returns ids, contexts, and cursors" do
         stub_response(anything, example_response)
-        user_ids, common_contexts, cursors = Services::AddressBook.search_users(@sender, search: 'bob')
+        user_ids, common_contexts, cursors = Services::AddressBook.search_users(@sender, {search: 'bob'}, {})
         expect(user_ids).to eql([ 10000000000002, 10000000000005 ])
         expect(common_contexts).to eql({
           10000000000002 => {