Add more tags to comment_client request metrics

This captures real-time metrics for all of the comment client actions,
segregated by course_id, as well as other small-cardinality fields.

The goal is to be able to detect changes in forum usage, with the goal
of alerting on potential error conditions.

lms/lib/comment_client/comment.py

@@ -21,6 +21,8 @@ class Comment(models.Model):
 
     initializable_fields = updatable_fields
 
+    metrics_tag_fields = ['course_id', 'endorsed', 'closed']
+
     base_url = "{prefix}/comments".format(prefix=settings.PREFIX)
     type = 'comment'
 
@@ -50,7 +52,13 @@ class Comment(models.Model):
         else:
             raise CommentClientRequestError("Can only flag/unflag threads or comments")
         params = {'user_id': user.id}
-        request = perform_request('put', url, params)
+        request = perform_request(
+            'put',
+            url,
+            params,
+            metric_tags=self._metric_tags,
+            metric_action='comment.abuse.flagged'
+        )
         voteable.update_attributes(request)
 
     def unFlagAbuse(self, user, voteable, removeAll):
@@ -65,7 +73,13 @@ class Comment(models.Model):
         if removeAll:
             params['all'] = True
 
-        request = perform_request('put', url, params)
+        request = perform_request(
+            'put',
+            url,
+            params,
+            metric_tags=self._metric_tags,
+            metric_action='comment.abuse.unflagged'
+        )
         voteable.update_attributes(request)
 
 

lms/lib/comment_client/models.py

@@ -8,6 +8,7 @@ class Model(object):
     initializable_fields = ['id']
     base_url = None
     default_retrieve_params = {}
+    metric_tag_fields = []
 
     DEFAULT_ACTIONS_WITH_ID = ['get', 'put', 'delete']
     DEFAULT_ACTIONS_WITHOUT_ID = ['get_all', 'post']
@@ -62,9 +63,32 @@ class Model(object):
 
     def _retrieve(self, *args, **kwargs):
         url = self.url(action='get', params=self.attributes)
-        response = perform_request('get', url, self.default_retrieve_params)
+        response = perform_request(
+            'get',
+            url,
+            self.default_retrieve_params,
+            metric_tags=self._metric_tags,
+            metric_action='model.retrieve'
+        )
         self.update_attributes(**response)
 
+    @property
+    def _metric_tags(self):
+        """
+        Returns a list of tags to be used when recording metrics about this model.
+
+        Each field named in ``self.metric_tag_fields`` is used as a tag value,
+        under the key ``<class>.<metric_field>``. The tag model_class is used to
+        record the class name of the model.
+        """
+        tags = [
+            u'{}.{}:{}'.format(self.__class__.__name__, attr, self[attr])
+            for attr in self.metric_tag_fields
+            if attr in self.attributes
+        ]
+        tags.append(u'model_class:{}'.format(self.__class__.__name__))
+        return tags
+
     @classmethod
     def find(cls, id):
         return cls(id=id)
@@ -94,17 +118,29 @@ class Model(object):
         self.before_save(self)
         if self.id:   # if we have id already, treat this as an update
             url = self.url(action='put', params=self.attributes)
-            response = perform_request('put', url, self.updatable_attributes())
+            response = perform_request(
+                'put',
+                url,
+                self.updatable_attributes(),
+                metric_tags=self._metric_tags,
+                metric_action='model.update'
+            )
         else:   # otherwise, treat this as an insert
             url = self.url(action='post', params=self.attributes)
-            response = perform_request('post', url, self.initializable_attributes())
+            response = perform_request(
+                'post',
+                url,
+                self.initializable_attributes(),
+                metric_tags=self._metric_tags,
+                metric_action='model.insert'
+            )
         self.retrieved = True
         self.update_attributes(**response)
         self.after_save(self)
 
     def delete(self):
         url = self.url(action='delete', params=self.attributes)
-        response = perform_request('delete', url)
+        response = perform_request('delete', url, metric_tags=self._metric_tags, metric_action='model.delete')
         self.retrieved = True
         self.update_attributes(**response)
 

lms/lib/comment_client/thread.py

@@ -20,6 +20,11 @@ class Thread(models.Model):
         'closed', 'user_id', 'commentable_id', 'group_id', 'group_name', 'pinned'
     ]
 
+    metric_tag_fields = [
+        'course_id', 'group_id', 'pinned', 'closed', 'anonymous', 'anonymous_to_peers',
+        'endorsed', 'read'
+    ]
+
     initializable_fields = updatable_fields
 
     base_url = "{prefix}/threads".format(prefix=settings.PREFIX)
@@ -41,7 +46,14 @@ class Thread(models.Model):
             url = cls.url(action='get_all', params=extract(params, 'commentable_id'))
             if params.get('commentable_id'):
                 del params['commentable_id']
-        response = perform_request('get', url, params)
+        response = perform_request(
+            'get',
+            url,
+            params,
+            metric_tags=[u'course_id:{}'.format(query_params['course_id'])],
+            metric_action='thread.search',
+            paged_results=True
+        )
         return response.get('collection', []), response.get('page', 1), response.get('num_pages', 1)
 
     @classmethod
@@ -79,7 +91,13 @@ class Thread(models.Model):
         }
         request_params = strip_none(request_params)
 
-        response = perform_request('get', url, request_params)
+        response = perform_request(
+            'get',
+            url,
+            request_params,
+            metric_action='model.retrieve',
+            metric_tags=self._metric_tags
+        )
         self.update_attributes(**response)
 
     def flagAbuse(self, user, voteable):
@@ -90,7 +108,13 @@ class Thread(models.Model):
         else:
             raise CommentClientRequestError("Can only flag/unflag threads or comments")
         params = {'user_id': user.id}
-        request = perform_request('put', url, params)
+        request = perform_request(
+            'put',
+            url,
+            params,
+            metric_action='thread.abuse.flagged',
+            metric_tags=self._metric_tags
+        )
         voteable.update_attributes(request)
 
     def unFlagAbuse(self, user, voteable, removeAll):
@@ -105,19 +129,37 @@ class Thread(models.Model):
         if removeAll:
             params['all'] = True
 
-        request = perform_request('put', url, params)
+        request = perform_request(
+            'put',
+            url,
+            params,
+            metric_tags=self._metric_tags,
+            metric_action='thread.abuse.unflagged'
+        )
         voteable.update_attributes(request)
 
     def pin(self, user, thread_id):
         url = _url_for_pin_thread(thread_id)
         params = {'user_id': user.id}
-        request = perform_request('put', url, params)
+        request = perform_request(
+            'put',
+            url,
+            params,
+            metric_tags=self._metric_tags,
+            metric_action='thread.pin'
+        )
         self.update_attributes(request)
 
     def un_pin(self, user, thread_id):
         url = _url_for_un_pin_thread(thread_id)
         params = {'user_id': user.id}
-        request = perform_request('put', url, params)
+        request = perform_request(
+            'put',
+            url,
+            params,
+            metric_tags=self._metric_tags,
+            metric_action='thread.unpin'
+        )
         self.update_attributes(request)
 
 

lms/lib/comment_client/user.py

@@ -16,6 +16,8 @@ class User(models.Model):
     updatable_fields = ['username', 'external_id', 'email', 'default_sort_key']
     initializable_fields = updatable_fields
 
+    metric_tag_fields = ['course_id']
+
     base_url = "{prefix}/users".format(prefix=settings.PREFIX)
     default_retrieve_params = {'complete': True}
     type = 'user'
@@ -29,11 +31,23 @@ class User(models.Model):
 
     def follow(self, source):
         params = {'source_type': source.type, 'source_id': source.id}
-        response = perform_request('post', _url_for_subscription(self.id), params)
+        response = perform_request(
+            'post',
+            _url_for_subscription(self.id),
+            params,
+            metric_action='user.follow',
+            metric_tags=self._metric_tags + ['target.type:{}'.format(source.type)],
+        )
 
     def unfollow(self, source):
         params = {'source_type': source.type, 'source_id': source.id}
-        response = perform_request('delete', _url_for_subscription(self.id), params)
+        response = perform_request(
+            'delete',
+            _url_for_subscription(self.id),
+            params,
+            metric_action='user.unfollow',
+            metric_tags=self._metric_tags + ['target.type:{}'.format(source.type)],
+        )
 
     def vote(self, voteable, value):
         if voteable.type == 'thread':
@@ -43,7 +57,13 @@ class User(models.Model):
         else:
             raise CommentClientRequestError("Can only vote / unvote for threads or comments")
         params = {'user_id': self.id, 'value': value}
-        request = perform_request('put', url, params)
+        request = perform_request(
+            'put',
+            url,
+            params,
+            metric_action='user.vote',
+            metric_tags=self._metric_tags + ['target.type:{}'.format(voteable.type)],
+        )
         voteable.update_attributes(request)
 
     def unvote(self, voteable):
@@ -54,7 +74,13 @@ class User(models.Model):
         else:
             raise CommentClientRequestError("Can only vote / unvote for threads or comments")
         params = {'user_id': self.id}
-        request = perform_request('delete', url, params)
+        request = perform_request(
+            'delete',
+            url,
+            params,
+            metric_action='user.unvote',
+            metric_tags=self._metric_tags + ['target.type:{}'.format(voteable.type)],
+        )
         voteable.update_attributes(request)
 
     def active_threads(self, query_params={}):
@@ -63,7 +89,14 @@ class User(models.Model):
         url = _url_for_user_active_threads(self.id)
         params = {'course_id': self.course_id}
         params = merge_dict(params, query_params)
-        response = perform_request('get', url, params)
+        response = perform_request(
+            'get',
+            url,
+            params,
+            metric_action='user.active_threads',
+            metric_tags=self._metric_tags,
+            paged_results=True,
+        )
         return response.get('collection', []), response.get('page', 1), response.get('num_pages', 1)
 
     def subscribed_threads(self, query_params={}):
@@ -72,7 +105,14 @@ class User(models.Model):
         url = _url_for_user_subscribed_threads(self.id)
         params = {'course_id': self.course_id}
         params = merge_dict(params, query_params)
-        response = perform_request('get', url, params)
+        response = perform_request(
+            'get',
+            url,
+            params,
+            metric_action='user.subscribed_threads',
+            metric_tags=self._metric_tags,
+            paged_results=True
+        )
         return response.get('collection', []), response.get('page', 1), response.get('num_pages', 1)
 
     def _retrieve(self, *args, **kwargs):
@@ -81,13 +121,25 @@ class User(models.Model):
         if self.attributes.get('course_id'):
             retrieve_params['course_id'] = self.course_id
         try:
-            response = perform_request('get', url, retrieve_params)
+            response = perform_request(
+                'get',
+                url,
+                retrieve_params,
+                metric_action='model.retrieve',
+                metric_tags=self._metric_tags,
+            )
         except CommentClientRequestError as e:
             if e.status_code == 404:
                 # attempt to gracefully recover from a previous failure
                 # to sync this user to the comments service.
                 self.save()
-                response = perform_request('get', url, retrieve_params)
+                response = perform_request(
+                    'get',
+                    url,
+                    retrieve_params,
+                    metric_action='model.retrieve',
+                    metric_tags=self._metric_tags,
+                )
             else:
                 raise
         self.update_attributes(**response)

lms/lib/comment_client/utils.py

@@ -33,12 +33,13 @@ def merge_dict(dic1, dic2):
 
 
 @contextmanager
-def request_timer(request_id, method, url):
+def request_timer(request_id, method, url, tags=None):
     start = time()
-    yield
+    with dog_stats_api.timer('comment_client.request.time', tags=tags):
+        yield
     end = time()
     duration = end - start
-    dog_stats_api.histogram('comment_client.request.time', duration, end)
+
     log.info(
         "comment_client_request_log: request_id={request_id}, method={method}, "
         "url={url}, duration={duration}".format(
@@ -50,7 +51,16 @@ def request_timer(request_id, method, url):
     )
 
 
-def perform_request(method, url, data_or_params=None, raw=False):
+def perform_request(method, url, data_or_params=None, raw=False,
+                    metric_action=None, metric_tags=None, paged_results=False):
+
+    if metric_tags is None:
+        metric_tags = []
+
+    metric_tags.append(u'method:{}'.format(method))
+    if metric_action:
+        metric_tags.append(u'action:{}'.format(metric_action))
+
     if data_or_params is None:
         data_or_params = {}
     headers = {
@@ -66,7 +76,7 @@ def perform_request(method, url, data_or_params=None, raw=False):
     else:
         data = None
         params = merge_dict(data_or_params, request_id_dict)
-    with request_timer(request_id, method, url):
+    with request_timer(request_id, method, url, metric_tags):
         response = requests.request(
             method,
             url,
@@ -76,6 +86,14 @@ def perform_request(method, url, data_or_params=None, raw=False):
             timeout=5
         )
 
+    metric_tags.append(u'status_code:{}'.format(response.status_code))
+    if response.status_code > 200:
+        metric_tags.append(u'result:failure')
+    else:
+        metric_tags.append(u'result:success')
+
+    dog_stats_api.increment('comment_client.request.count', tags=metric_tags)
+
     if 200 < response.status_code < 500:
         raise CommentClientRequestError(response.text, response.status_code)
     # Heroku returns a 503 when an application is in maintenance mode
@@ -87,7 +105,24 @@ def perform_request(method, url, data_or_params=None, raw=False):
         if raw:
             return response.text
         else:
-            return response.json()
+            data = response.json()
+            if paged_results:
+                dog_stats_api.histogram(
+                    'comment_client.request.paged.result_count',
+                    value=len(data.get('collection', [])),
+                    tags=metric_tags
+                )
+                dog_stats_api.histogram(
+                    'comment_client.request.paged.page',
+                    value=data.get('page', 1),
+                    tags=metric_tags
+                )
+                dog_stats_api.histogram(
+                    'comment_client.request.paged.num_pages',
+                    value=data.get('num_pages', 1),
+                    tags=metric_tags
+                )
+            return data
 
 
 class CommentClientError(Exception):