[SE-3136] Add client instance name to New Relic monitoring condition name

instance/models/mixins/openedx_monitoring.py

@@ -22,9 +22,12 @@
 
 # Imports #####################################################################
 
+import textwrap
+
 from django.conf import settings
 from django.db import models
 from django.db.models import ProtectedError
+from django.utils.html import escape
 
 from instance import newrelic
 
@@ -65,6 +68,26 @@ def enable_monitoring(self):
         self._update_url_monitors(alert_policy)
         self._set_email_alerts(alert_policy)
 
+    @staticmethod
+    def _generate_monitor_condition_name(short_name: str, instance_name: str) -> str:
+        """
+        Generate monitor condition name for the given URL and short name of the instance.
+
+        Based on the documentation of New Relic, the monitor name should be less than
+        or equal to 64 chars. The shortening will not split a word into two, instead it
+        will skip the word which would be splitted.
+
+        Also, based on New Relic's documentation, the names cannot contain angle brackets
+        ("<" and ">"), so we need to escape them in case the instance names would contain any.
+        """
+
+        monitoring_name = "{short} of {instance}".format(
+            short=short_name,
+            instance=escape(instance_name)
+        )
+
+        return textwrap.shorten(monitoring_name, 64, placeholder="...")
+
     def _update_url_monitors(self, alert_policy):
         """
         Create a monitor for each URL to monitor, and delete the other monitors.
@@ -83,9 +106,8 @@ def _update_url_monitors(self, alert_policy):
                 # and create one if it doesn't. This helps when the alert condition
                 # wasn't created in a previous invocation due to some issue.
                 if not monitor.new_relic_alert_conditions.exists():
-                    alert_condition_id = newrelic.add_alert_nrql_condition(
-                        alert_policy.id, url, urls_to_monitor_dict[url]
-                    )
+                    monitor_name = self._generate_monitor_condition_name(urls_to_monitor_dict[url], self.name)
+                    alert_condition_id = newrelic.add_alert_nrql_condition(alert_policy.id, url, monitor_name)
                     monitor.new_relic_alert_conditions.create(id=alert_condition_id, alert_policy=alert_policy)
             else:
                 self.logger.info('Deleting New Relic Synthetics monitor for old public URL %s', url)
@@ -95,9 +117,8 @@ def _update_url_monitors(self, alert_policy):
             self.logger.info('Creating New Relic Synthetics monitor for new public URL %s', url)
             new_monitor_id = newrelic.create_synthetics_monitor(url)
             monitor = self.new_relic_availability_monitors.create(pk=new_monitor_id)
-            alert_condition_id = newrelic.add_alert_nrql_condition(
-                alert_policy.id, url, urls_to_monitor_dict[url]
-            )
+            monitor_name = self._generate_monitor_condition_name(urls_to_monitor_dict[url], self.name)
+            alert_condition_id = newrelic.add_alert_nrql_condition(alert_policy.id, url, monitor_name)
             monitor.new_relic_alert_conditions.create(id=alert_condition_id, alert_policy=alert_policy)
 
     def _set_email_alerts(self, alert_policy):

instance/tests/models/test_openedx_monitoring_mixins.py

@@ -106,24 +106,92 @@ def test_enable_monitoring(self, additional_monitoring_emails, expected_monitor_
         )
 
         # Check that alert emails have been set up
-        created_condition_urls = set()
+        created_conditions = set()
         list_of_emails_added = []
 
         for add_call in mock_newrelic.add_alert_nrql_condition.call_args_list:
-            created_condition_urls.add(add_call[0][1])
+            created_conditions.add((add_call[0][1], add_call[0][2]))
 
         for add_call in mock_newrelic.add_email_notification_channel.call_args_list:
             list_of_emails_added.append(add_call[0][0])
 
         self.assertEqual(
-            created_condition_urls,
-            set([instance.url, instance.studio_url, instance.lms_preview_url, instance.lms_extended_heartbeat_url])
+            created_conditions,
+            set([
+                (instance.url, 'LMS of {}'.format(instance.name)),
+                (instance.studio_url, 'Studio of {}'.format(instance.name)),
+                (instance.lms_preview_url, 'Preview of {}'.format(instance.name)),
+                (instance.lms_extended_heartbeat_url, 'Extended heartbeat of {}'.format(instance.name)),
+            ])
         )
         self.assertEqual(set(list_of_emails_added), set(expected_monitor_emails))
         mock_newrelic.add_notification_channels_to_policy.assert_called_with(
             1, list(range(len(expected_monitor_emails)))
         )
 
+    @ddt.data(
+        # [additional_monitoring_emails, expected final email monitor list]
+        [[], ['admin@opencraft.com']],
+        [['other@opencraft.com'], ['admin@opencraft.com', 'other@opencraft.com']],
+    )
+    @ddt.unpack
+    @patch('instance.models.mixins.openedx_monitoring.newrelic')
+    def test_long_monitoring_condition_names(self, additional_emails, expected_emails, mock_newrelic, mock_consul):
+        """
+        Check that the `enable_monitoring` method creates New Relic Synthetics
+        monitors for each of the instance's public urls, and enables email
+        alerts even for those instance names which has extremely long names.
+        """
+        monitor_ids = [str(uuid4()) for i in range(4)]
+        mock_newrelic.get_synthetics_monitor.return_value = []
+        mock_newrelic.get_synthetics_notification_emails.return_value = []
+        mock_newrelic.create_synthetics_monitor.side_effect = monitor_ids
+        mock_newrelic.add_alert_policy.return_value = 1
+        mock_newrelic.add_alert_nrql_condition.side_effect = list(range(4))
+        mock_newrelic.add_email_notification_channel.side_effect = list(range(len(expected_emails)))
+        instance = OpenEdXInstanceFactory()
+        instance.name += " long" * 15  # Generate a very long instance name
+        instance.additional_monitoring_emails = additional_emails
+        instance.enable_monitoring()
+
+        # Check that the monitors have been created
+        mock_newrelic.delete_synthetics_monitor.assert_not_called()
+        mock_newrelic.create_synthetics_monitor.assert_has_calls([
+            call(instance.url),
+            call(instance.studio_url),
+            call(instance.lms_preview_url),
+            call(instance.lms_extended_heartbeat_url),
+        ], any_order=True)
+        self.assertCountEqual(
+            instance.new_relic_availability_monitors.values_list('pk', flat=True),
+            monitor_ids
+        )
+
+        # Check that alert emails have been set up
+        created_condition_urls = set()
+        created_condition_names = set()
+        list_of_emails_added = []
+
+        for add_call in mock_newrelic.add_alert_nrql_condition.call_args_list:
+            created_condition_urls.add(add_call[0][1])
+            created_condition_names.add(add_call[0][2])
+
+        for add_call in mock_newrelic.add_email_notification_channel.call_args_list:
+            list_of_emails_added.append(add_call[0][0])
+
+        for condition_name in created_condition_names:
+            self.assertTrue(condition_name.endswith("..."))
+            self.assertLessEqual(len(condition_name), 64)
+
+        self.assertEqual(created_condition_urls, set([
+            instance.url,
+            instance.studio_url,
+            instance.lms_preview_url,
+            instance.lms_extended_heartbeat_url,
+        ]))
+        self.assertEqual(set(list_of_emails_added), set(expected_emails))
+        mock_newrelic.add_notification_channels_to_policy.assert_called_with(1, list(range(len(expected_emails))))
+
     @patch('instance.models.mixins.openedx_monitoring.newrelic')
     def test_enable_monitoring_for_pre_new_relic_alerts_instances(self, mock_newrelic, mock_consul):
         """