From 6dd6f0c79dc473ce01f066e1ba82b623071898c1 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Wed, 26 Mar 2014 14:36:26 -0400 Subject: [PATCH] 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 | 18 +++++++- lms/lib/comment_client/models.py | 44 ++++++++++++++++++-- lms/lib/comment_client/thread.py | 54 +++++++++++++++++++++--- lms/lib/comment_client/user.py | 68 +++++++++++++++++++++++++++---- lms/lib/comment_client/utils.py | 47 ++++++++++++++++++--- 5 files changed, 205 insertions(+), 26 deletions(-) diff --git a/lms/lib/comment_client/comment.py b/lms/lib/comment_client/comment.py index 2b0d77b15f..c09c7d9c98 100644 --- a/lms/lib/comment_client/comment.py +++ b/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) diff --git a/lms/lib/comment_client/models.py b/lms/lib/comment_client/models.py index 367b9abd10..5bb3205641 100644 --- a/lms/lib/comment_client/models.py +++ b/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 ``.``. 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) diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index cd1f37939b..335f24c9c5 100644 --- a/lms/lib/comment_client/thread.py +++ b/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) diff --git a/lms/lib/comment_client/user.py b/lms/lib/comment_client/user.py index 77e8b0d011..fdea00062f 100644 --- a/lms/lib/comment_client/user.py +++ b/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) diff --git a/lms/lib/comment_client/utils.py b/lms/lib/comment_client/utils.py index 62e2500bb9..17fc198973 100644 --- a/lms/lib/comment_client/utils.py +++ b/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):