From 3eeba0266b63279e447e92a865710ab3a4956d2a Mon Sep 17 00:00:00 2001 From: Ben Thorner Date: Thu, 28 Oct 2021 11:12:18 +0100 Subject: [PATCH] Revert "add raw request timings to provider send functions" This reverts commit f2f2509c9b38d960231e08c863c7591f44206a55. Raw request stats were added to investigate a hunch about a performance issue we were seeing [1], but turned out not to be relevant. We don't use them anymore so we can tidy up. [1]: https://github.com/alphagov/notifications-api/pull/2858 --- app/clients/email/aws_ses.py | 20 -------------------- app/clients/sms/firetext.py | 3 --- app/clients/sms/mmg.py | 4 ---- 3 files changed, 27 deletions(-) diff --git a/app/clients/email/aws_ses.py b/app/clients/email/aws_ses.py index 4e7a6346b..4588a53b7 100644 --- a/app/clients/email/aws_ses.py +++ b/app/clients/email/aws_ses.py @@ -62,22 +62,6 @@ class AwsSesClient(EmailClient): self.name = 'ses' self.statsd_client = statsd_client - # events are generally undocumented, but some that might be of interest are: - # before-call, after-call, after-call-error, request-created, response-received - self._client.meta.events.register('request-created.ses.SendEmail', self.ses_request_created_hook) - self._client.meta.events.register('response-received.ses.SendEmail', self.ses_response_received_hook) - - def ses_request_created_hook(self, **kwargs): - # request created may be called multiple times if the request auto-retries. We want to count all these as the - # same request for timing purposes, so only reset the start time if it was cleared completely - if self.ses_start_time == 0: - self.ses_start_time = monotonic() - - def ses_response_received_hook(self, **kwargs): - # response received may be called multiple times if the request auto-retries, however, we want to count the last - # time it triggers for timing purposes, so always reset the elapsed time - self.ses_elapsed_time = monotonic() - self.ses_start_time - def get_name(self): return self.name @@ -88,8 +72,6 @@ class AwsSesClient(EmailClient): body, html_body='', reply_to_address=None): - self.ses_elapsed_time = 0 - self.ses_start_time = 0 try: if isinstance(to_addresses, str): to_addresses = [to_addresses] @@ -142,8 +124,6 @@ class AwsSesClient(EmailClient): elapsed_time = monotonic() - start_time current_app.logger.info("AWS SES request finished in {}".format(elapsed_time)) self.statsd_client.timing("clients.ses.request-time", elapsed_time) - if self.ses_elapsed_time != 0: - self.statsd_client.timing("clients.ses.raw-request-time", self.ses_elapsed_time) self.statsd_client.incr("clients.ses.success") return response['MessageId'] diff --git a/app/clients/sms/firetext.py b/app/clients/sms/firetext.py index 39099e02a..0d9b23b44 100644 --- a/app/clients/sms/firetext.py +++ b/app/clients/sms/firetext.py @@ -101,7 +101,6 @@ class FiretextClient(SmsClient): "reference": reference } - response = None start_time = monotonic() try: response = request( @@ -126,6 +125,4 @@ class FiretextClient(SmsClient): elapsed_time = monotonic() - start_time self.current_app.logger.info("Firetext request for {} finished in {}".format(reference, elapsed_time)) self.statsd_client.timing("clients.firetext.request-time", elapsed_time) - if response and hasattr(response, 'elapsed'): - self.statsd_client.timing("clients.firetext.raw-request-time", response.elapsed.total_seconds()) return response diff --git a/app/clients/sms/mmg.py b/app/clients/sms/mmg.py index fdaa99ccd..ec32fa0a9 100644 --- a/app/clients/sms/mmg.py +++ b/app/clients/sms/mmg.py @@ -107,7 +107,6 @@ class MMGClient(SmsClient): "multi": multi } - response = None start_time = monotonic() try: response = request( @@ -134,9 +133,6 @@ class MMGClient(SmsClient): finally: elapsed_time = monotonic() - start_time self.statsd_client.timing("clients.mmg.request-time", elapsed_time) - if response and hasattr(response, 'elapsed'): - self.statsd_client.timing("clients.mmg.raw-request-time", response.elapsed.total_seconds()) - self.current_app.logger.info("MMG request for {} finished in {}".format(reference, elapsed_time)) return response