MSFT sync quota points statsd for batch requests

To properly test calls to InstStatsd::Statsd.* you should add the
following to your docker-compose.override.yml under "environment":

      INST_STATSD_HOST: localhost
      INST_DOG_TAGS: '{}'

As of this commit, I have done that and then changed all instances of
Statsd.count() in the lib/microsoft_sync directory to "increment" (buggy
old version) one-by-one, and run the corresponding spec files (also
graph_service_spec.rb for program code graph_service_http.rb). Each of
them then failed with the "wrong number of arguments (given 3, expected
1..2)" error.

refs INTEROP-6798
flag=microsoft_group_enrollments_syncing

Test plan:
- kick off a sync that will make two or more users deletions
  (e.g. removing a teacher may count as 2 -- deleted as owner and as
  member)
- open a console and monkey-patch statsd:
  class << InstStatsd::Statsd
    def increment(*args)
      puts "DEBUG STATSD increment: #{args.to_json}"
    end
    def count(*args)
      puts "DEBUG STATSD count: #{args.to_json}"
    end
  end
- Watch that read quota and write quota are incremented by the correct
  amount (number of users removed), with the msft_endpoint tag equal to
  "batch_group_remove_users"

Change-Id: I00cdcb7ccfbff5cf157aa3249f1c234adade7e1d
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/267810
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Product-Review: Evan Battaglia <ebattaglia@instructure.com>
Reviewed-by: Ryan Hawkins <ryan.hawkins@instructure.com>
QA-Review: Ryan Hawkins <ryan.hawkins@instructure.com>
This commit is contained in:
Evan Battaglia 2021-06-24 21:28:32 -06:00
parent 0efe16d746
commit e9c5a0e186
4 changed files with 49 additions and 3 deletions

View File

@ -101,7 +101,8 @@ module MicrosoftSync
reqs =
group_remove_user_requests(group_id, members, 'members') +
group_remove_user_requests(group_id, owners, 'owners')
failed_req_ids = run_batch('group_remove_users', reqs) do |resp|
quota = [reqs.count, reqs.count]
failed_req_ids = run_batch('group_remove_users', reqs, quota: quota) do |resp|
(
resp['status'] == 404 && resp['body'].to_s =~
/does not exist or one of its queried reference-property objects are not present/i
@ -120,7 +121,7 @@ module MicrosoftSync
reqs =
group_add_user_requests(group_id, members, 'members') +
group_add_user_requests(group_id, owners, 'owners')
failed_req_ids = run_batch('group_add_users', reqs) do |r|
failed_req_ids = run_batch('group_add_users', reqs, quota: [reqs.count, reqs.count]) do |r|
r['status'] == 400 && r['body'].to_s =~ /One or more added object references already exist/i
end
split_request_ids_to_hash(failed_req_ids)

View File

@ -120,8 +120,9 @@ module MicrosoftSync
# HTTP request. Expected failures can be ignored by passing in a block which checks
# the response. Other non-2xx responses cause a BatchRequestFailed error.
# Returns a list of ids of the requests that were ignored.
def run_batch(endpoint_name, requests, &response_should_be_ignored)
def run_batch(endpoint_name, requests, quota:, &response_should_be_ignored)
Rails.logger.info("MicrosoftSync::GraphClient: batch of #{requests.count} #{endpoint_name}")
increment_statsd_quota_points(quota, {}, msft_endpoint: "batch_#{endpoint_name}")
response =
begin

View File

@ -161,4 +161,36 @@ describe MicrosoftSync::GraphServiceHttp do
have_received(:request).with(:get, continue_url, hash_including(quota: [2, 3]))
end
end
describe '#run_batch' do
before do
WebMock.disable_net_connect!
WebMock.stub_request(:post, 'https://graph.microsoft.com/v1.0/$batch')
.with(body: {requests: requests})
.and_return(
status: 200, body: {responses:[]}.to_json,
headers: {'Content-type' => 'application/json'}
)
end
after { WebMock.enable_net_connect! }
let(:requests) do
[
{id: 'a', method: 'GET', url: '/foo'},
{id: 'a', method: 'GET', url: '/bar'},
]
end
let(:run_batch) { subject.run_batch('wombat', requests, quota: [3, 4]) }
it 'counts statsd metrics with the quota' do
allow(InstStatsd::Statsd).to receive(:count).and_call_original
run_batch
expect(InstStatsd::Statsd).to have_received(:count)
.with("microsoft_sync.graph_service.quota_read", 3, tags: {msft_endpoint: 'batch_wombat'})
expect(InstStatsd::Statsd).to have_received(:count)
.with("microsoft_sync.graph_service.quota_write", 4, tags: {msft_endpoint: 'batch_wombat'})
end
end
end

View File

@ -597,6 +597,12 @@ describe MicrosoftSync::GraphService do
end
it { is_expected.to eq(nil) }
it 'passes along the quota used to run_batch' do
expect(service.http).to \
receive(:run_batch).with(anything, anything, quota: [4, 4]).and_call_original
subject
end
end
context 'when some owners were already in the group' do
@ -720,6 +726,12 @@ describe MicrosoftSync::GraphService do
end
it { is_expected.to eq(nil) }
it 'passes along the quota used to run_batch' do
expect(service.http).to \
receive(:run_batch).with(anything, anything, quota: [4, 4]).and_call_original
subject
end
end
context 'when some owners were not in the group' do