From 306f842963c8e6bca0769f5af39de44c2eb04c46 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Thu, 8 Jun 2023 13:13:54 +0800 Subject: [PATCH] TwilioPhoneProvider optimizations (#2034) # What this PR does This PR does three improvements in twilio_phone_provider: 1. [Speed-up](https://github.com/grafana/oncall/pull/2034/files#diff-7a311767169c024e60e2b4e35fd531dd6e2f1ea785cfc84263e11e7932d622af) query which calculates amount of phone_calls/sms left. 2. Remove code which was needed only for backward compatibility during the release of PhoneProvider refactoring and improves logging for handling status/gather updates. 3. Add db_index to twilio_sid. We are doing lot of lookups by sid and with increasing amount of data it became resource consuming. --- engine/apps/twilioapp/gather.py | 43 ++-- .../migrations/0006_auto_20230601_0807.py | 23 +++ .../twilioapp/models/twilio_phone_call.py | 1 + engine/apps/twilioapp/models/twilio_sms.py | 1 + engine/apps/twilioapp/status_callback.py | 184 +++++++++--------- .../free_public_beta_subscription_strategy.py | 2 - 6 files changed, 141 insertions(+), 113 deletions(-) create mode 100644 engine/apps/twilioapp/migrations/0006_auto_20230601_0807.py diff --git a/engine/apps/twilioapp/gather.py b/engine/apps/twilioapp/gather.py index 54b918d7..ad06eba2 100644 --- a/engine/apps/twilioapp/gather.py +++ b/engine/apps/twilioapp/gather.py @@ -1,6 +1,5 @@ import logging -from django.apps import apps from django.urls import reverse from twilio.twiml.voice_response import Gather, VoiceResponse @@ -56,30 +55,30 @@ def process_digit(call_sid, digit): if call_sid and digit: logger.info(f"twilioapp.process_digit: processing sid={call_sid} digit={digit}") twilio_phone_call = TwilioPhoneCall.objects.filter(sid=call_sid).first() - # Check twilio phone call and then oncall phone call for backward compatibility after PhoneCall migration. - # Will be removed soon. - if twilio_phone_call: - logger.info(f"twilioapp.process_digit: found legacy twilio_phone_call sid={call_sid} digit={digit}") - phone_call_record = twilio_phone_call.phone_call_record - else: - PhoneCallRecord = apps.get_model("phone_notifications", "PhoneCallRecord") - phone_call_record = PhoneCallRecord.objects.filter(sid=call_sid).first() + if twilio_phone_call is None: + logger.info(f"twilioapp.process_digit: twilio_phone_call not found sid={call_sid}") + return - if phone_call_record is not None: - alert_group = phone_call_record.represents_alert_group - user = phone_call_record.receiver + logger.info(f"twilioapp.process_digit: found twilio_phone_call sid={call_sid} digit={digit}") + phone_call_record = twilio_phone_call.phone_call_record - logger.info( - f"twilioapp.process_digit: processing using phone_call_record id={phone_call_record.id} " - f"twilio_phone_call sid={call_sid} digit={digit} alert_group_id={alert_group.id}" - ) + if phone_call_record is None: + logger.info(f"twilioapp.process_digit: twilio_phone_call has no phone_call_record sid={call_sid}") + return - if digit == "1": - alert_group.acknowledge_by_user(user, action_source=ActionSource.PHONE) - elif digit == "2": - alert_group.resolve_by_user(user, action_source=ActionSource.PHONE) - elif digit == "3": - alert_group.silence_by_user(user, silence_delay=1800, action_source=ActionSource.PHONE) + logger.info(f"twilioapp.process_digit: found phone_call_record id={phone_call_record.id} sid={call_sid}") + alert_group = phone_call_record.represents_alert_group + user = phone_call_record.receiver + logger.info( + f"twilioapp.process_digit: processing digit phone_call_record id={phone_call_record.id} " + f"twilio_phone_call_sid={call_sid} digit={digit} alert_group_id={alert_group.id} user_id={user.id}" + ) + if digit == "1": + alert_group.acknowledge_by_user(user, action_source=ActionSource.PHONE) + elif digit == "2": + alert_group.resolve_by_user(user, action_source=ActionSource.PHONE) + elif digit == "3": + alert_group.silence_by_user(user, silence_delay=1800, action_source=ActionSource.PHONE) def get_gather_url(): diff --git a/engine/apps/twilioapp/migrations/0006_auto_20230601_0807.py b/engine/apps/twilioapp/migrations/0006_auto_20230601_0807.py new file mode 100644 index 00000000..122a81be --- /dev/null +++ b/engine/apps/twilioapp/migrations/0006_auto_20230601_0807.py @@ -0,0 +1,23 @@ +# Generated by Django 3.2.18 on 2023-06-01 08:07 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('twilioapp', '0005_twilioaccount_twiliophonecallsender_twiliosmssender_twilioverificationsender'), + ] + + operations = [ + migrations.AlterField( + model_name='twiliophonecall', + name='sid', + field=models.CharField(blank=True, db_index=True, max_length=50), + ), + migrations.AlterField( + model_name='twiliosms', + name='sid', + field=models.CharField(blank=True, db_index=True, max_length=50), + ), + ] diff --git a/engine/apps/twilioapp/models/twilio_phone_call.py b/engine/apps/twilioapp/models/twilio_phone_call.py index 4b4423eb..ea1ffd32 100644 --- a/engine/apps/twilioapp/models/twilio_phone_call.py +++ b/engine/apps/twilioapp/models/twilio_phone_call.py @@ -63,6 +63,7 @@ class TwilioPhoneCall(ProviderPhoneCall, models.Model): sid = models.CharField( blank=True, max_length=50, + db_index=True, ) created_at = models.DateTimeField(auto_now_add=True) diff --git a/engine/apps/twilioapp/models/twilio_sms.py b/engine/apps/twilioapp/models/twilio_sms.py index 8050d50b..a19a070c 100644 --- a/engine/apps/twilioapp/models/twilio_sms.py +++ b/engine/apps/twilioapp/models/twilio_sms.py @@ -58,6 +58,7 @@ class TwilioSMS(ProviderSMS, models.Model): sid = models.CharField( blank=True, max_length=50, + db_index=True, ) created_at = models.DateTimeField(auto_now_add=True) diff --git a/engine/apps/twilioapp/status_callback.py b/engine/apps/twilioapp/status_callback.py index 8b6ab3b6..83f1eed0 100644 --- a/engine/apps/twilioapp/status_callback.py +++ b/engine/apps/twilioapp/status_callback.py @@ -25,58 +25,63 @@ def update_twilio_call_status(call_sid, call_status): if call_sid and call_status: logger.info(f"twilioapp.update_twilio_call_status: processing sid={call_sid} status={call_status}") - status = TwilioCallStatuses.DETERMINANT.get(call_status) + status_code = TwilioCallStatuses.DETERMINANT.get(call_status) + + if status_code is None: + logger.warning( + f"twilioapp.update_twilio_call_status: unexpected status sid={call_sid} status={call_status}" + ) + return twilio_phone_call = TwilioPhoneCall.objects.filter(sid=call_sid).first() - # Check twilio phone call and then oncall phone call for backward compatibility after PhoneCall migration. - # Will be removed soon. - if twilio_phone_call: - logger.info( - f"twilioapp.update_twilio_call_status: found twilio_phone_call sid={call_sid}" f" status={call_status}" + if twilio_phone_call is None: + logger.warning(f"twilioapp.update_twilio_call_status: twilio_phone_call not found sid={call_sid}") + return + + logger.info(f"twilioapp.update_twilio_call_status: found twilio_phone_call sid={call_sid}") + twilio_phone_call.status = status_code + twilio_phone_call.save(update_fields=["status"]) + phone_call_record = twilio_phone_call.phone_call_record + + if phone_call_record is None: + logger.warning( + f"twilioapp.update_twilio_call_status: twilio_phone_call has no phone_call record sid={call_sid} " + f"status={call_status}" ) - status = TwilioCallStatuses.DETERMINANT.get(call_status) - twilio_phone_call.status = status - twilio_phone_call.save(update_fields=["status"]) - phone_call_record = twilio_phone_call.phone_call_record - else: - PhoneCallRecord = apps.get_model("phone_notifications", "PhoneCallRecord") - phone_call_record = PhoneCallRecord.objects.filter(sid=call_sid).first() + return - if phone_call_record and status: - logger.info( - f"twilioapp.update_twilio_call_status: found phone_call_record_id={phone_call_record.id} " - f"sid={call_sid} status={call_status}" + logger.info( + f"twilioapp.update_twilio_call_status: found phone_call_record id={phone_call_record.id} " + f"sid={call_sid} status={call_status}" + ) + log_record_type = None + log_record_error_code = None + if status_code == TwilioCallStatuses.COMPLETED: + log_record_type = UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS + elif status_code in [TwilioCallStatuses.FAILED, TwilioCallStatuses.BUSY, TwilioCallStatuses.NO_ANSWER]: + log_record_type = UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED + log_record_error_code = get_error_code_by_twilio_status(status_code) + if log_record_type is not None: + log_record = UserNotificationPolicyLogRecord( + type=log_record_type, + notification_error_code=log_record_error_code, + author=phone_call_record.receiver, + notification_policy=phone_call_record.notification_policy, + alert_group=phone_call_record.represents_alert_group, + notification_step=phone_call_record.notification_policy.step + if phone_call_record.notification_policy + else None, + notification_channel=phone_call_record.notification_policy.notify_by + if phone_call_record.notification_policy + else None, ) - log_record_type = None - log_record_error_code = None - - if status == TwilioCallStatuses.COMPLETED: - log_record_type = UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS - elif status in [TwilioCallStatuses.FAILED, TwilioCallStatuses.BUSY, TwilioCallStatuses.NO_ANSWER]: - log_record_type = UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED - log_record_error_code = get_error_code_by_twilio_status(status) - - if log_record_type is not None: - log_record = UserNotificationPolicyLogRecord( - type=log_record_type, - notification_error_code=log_record_error_code, - author=phone_call_record.receiver, - notification_policy=phone_call_record.notification_policy, - alert_group=phone_call_record.represents_alert_group, - notification_step=phone_call_record.notification_policy.step - if phone_call_record.notification_policy - else None, - notification_channel=phone_call_record.notification_policy.notify_by - if phone_call_record.notification_policy - else None, - ) - log_record.save() - logger.info( - f"twilioapp.update_twilio_call_status: created log_record log_record_id={log_record.id} " - f"type={log_record_type}" - ) - user_notification_action_triggered_signal.send(sender=update_twilio_call_status, log_record=log_record) + log_record.save() + logger.info( + f"twilioapp.update_twilio_call_status: created log_record log_record_id={log_record.id} " + f"type={log_record_type}" + ) + user_notification_action_triggered_signal.send(sender=update_twilio_call_status, log_record=log_record) def get_error_code_by_twilio_status(status): @@ -105,55 +110,56 @@ def update_twilio_sms_status(message_sid, message_status): if message_sid and message_status: logger.info(f"twilioapp.update_twilio_message_status: processing sid={message_sid} status={message_status}") - status = TwilioSMSstatuses.DETERMINANT.get(message_status) + status_code = TwilioSMSstatuses.DETERMINANT.get(message_status) + if status_code is None: + logger.warning( + f"twilioapp.update_twilio_message_status: unexpected status sid={message_sid} status={message_status}" + ) + return twilio_sms = TwilioSMS.objects.filter(sid=message_sid).first() + if twilio_sms is None: + logger.warning(f"twilioapp.update_twilio_message_status: twilio_sms not found sid={message_sid}") + return - # Check twilio phone call and then oncall phone call for backward compatibility after PhoneCall migration. - # Will be removed soon. - if twilio_sms: - logger.info( - f"twilioapp.update_twilio_sms_status: found legacy twilio_phone_call sid={message_sid}" - f" status={message_sid}" + logger.info(f"twilioapp.update_twilio_sms_status: found twilio_sms sid={message_sid}") + twilio_sms.status = status_code + twilio_sms.save(update_fields=["status"]) + sms_record = twilio_sms.sms_record + + if sms_record is None: + logger.warning(f"twilioapp.update_twilio_sms_status: twilio_sms has no sms_record sid={message_sid}") + return + + logger.info( + f"twilioapp.update_twilio_sms_status: found sms_record id={sms_record.id} " + f"sid={message_sid} status={message_status}" + ) + log_record_type = None + log_record_error_code = None + if status_code == TwilioSMSstatuses.DELIVERED: + log_record_type = UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS + elif status_code in [TwilioSMSstatuses.UNDELIVERED, TwilioSMSstatuses.FAILED]: + log_record_type = UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED + log_record_error_code = get_sms_error_code_by_twilio_status(status_code) + if log_record_type is not None: + log_record = UserNotificationPolicyLogRecord( + type=log_record_type, + notification_error_code=log_record_error_code, + author=sms_record.receiver, + notification_policy=sms_record.notification_policy, + alert_group=sms_record.represents_alert_group, + notification_step=sms_record.notification_policy.step if sms_record.notification_policy else None, + notification_channel=sms_record.notification_policy.notify_by + if sms_record.notification_policy + else None, ) - twilio_sms.status = status - twilio_sms.save(update_fields=["status"]) - sms_record = twilio_sms.sms_record - else: - PhoneCallRecord = apps.get_model("phone_notifications", "PhoneCallRecord") - sms_record = PhoneCallRecord.objects.filter(sid=message_sid).first() - - if sms_record and status: + log_record.save() logger.info( - f"twilioapp.update_twilio_sms_status: found sms_record_id={sms_record.id} " - f"sid={message_sid} status={message_status}" + f"twilioapp.update_twilio_sms_status: created log_record log_record_id={log_record.id} " + f"type={log_record_type}" ) - log_record_type = None - log_record_error_code = None - if status == TwilioSMSstatuses.DELIVERED: - log_record_type = UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_SUCCESS - elif status in [TwilioSMSstatuses.UNDELIVERED, TwilioSMSstatuses.FAILED]: - log_record_type = UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED - log_record_error_code = get_sms_error_code_by_twilio_status(status) - - if log_record_type is not None: - log_record = UserNotificationPolicyLogRecord( - type=log_record_type, - notification_error_code=log_record_error_code, - author=sms_record.receiver, - notification_policy=sms_record.notification_policy, - alert_group=sms_record.represents_alert_group, - notification_step=sms_record.notification_policy.step if sms_record.notification_policy else None, - notification_channel=sms_record.notification_policy.notify_by - if sms_record.notification_policy - else None, - ) - log_record.save() - logger.info( - f"twilioapp.update_twilio_sms_status: created log_record log_record_id={log_record.id} " - f"type={log_record_type}" - ) - user_notification_action_triggered_signal.send(sender=update_twilio_sms_status, log_record=log_record) + user_notification_action_triggered_signal.send(sender=update_twilio_sms_status, log_record=log_record) def get_sms_error_code_by_twilio_status(status): diff --git a/engine/apps/user_management/subscription_strategy/free_public_beta_subscription_strategy.py b/engine/apps/user_management/subscription_strategy/free_public_beta_subscription_strategy.py index 3dd9498f..116a985f 100644 --- a/engine/apps/user_management/subscription_strategy/free_public_beta_subscription_strategy.py +++ b/engine/apps/user_management/subscription_strategy/free_public_beta_subscription_strategy.py @@ -61,12 +61,10 @@ class FreePublicBetaSubscriptionStrategy(BaseSubscriptionStrategy): day_start = now.replace(hour=0, minute=0, second=0, microsecond=0) calls_today = PhoneCallRecord.objects.filter( created_at__gte=day_start, - represents_alert_group__channel__organization=self.organization, receiver=user, ).count() sms_today = SMSMessage.objects.filter( created_at__gte=day_start, - represents_alert_group__channel__organization=self.organization, receiver=user, ).count()