From a8f622af7f9de12a1c557f07eb5ecf879168c504 Mon Sep 17 00:00:00 2001
From: Braden MacDonald <braden@opencraft.com>
Date: Fri, 26 Sep 2014 22:35:47 -0700
Subject: [PATCH] Consistently ensure dog_stats_api tags are formatted
 correctly

---
 common/djangoapps/monitoring/signals.py       |  2 +-
 common/djangoapps/student/models.py           |  2 +-
 common/djangoapps/student/views.py            |  2 +-
 common/djangoapps/util/views.py               |  2 +-
 common/lib/capa/capa/responsetypes.py         |  2 +-
 common/lib/capa/capa/xqueue_interface.py      |  2 +-
 .../lib/dogstats/dogstats_wrapper/__init__.py |  1 +
 .../lib/dogstats/dogstats_wrapper/wrapper.py  | 47 +++++++++++++++++++
 common/lib/dogstats/setup.py                  | 10 ++++
 common/lib/xmodule/xmodule/capa_base.py       |  2 +-
 .../controller_query_service.py               |  2 +-
 .../grading_service_module.py                 |  2 +-
 .../peer_grading_service.py                   |  2 +-
 common/lib/xmodule/xmodule/x_module.py        |  2 +-
 lms/djangoapps/bulk_email/tasks.py            |  8 ++--
 lms/djangoapps/certificates/views.py          |  2 +-
 lms/djangoapps/courseware/grades.py           |  4 +-
 lms/djangoapps/courseware/module_render.py    |  2 +-
 lms/djangoapps/instructor_task/subtasks.py    | 20 +++-----
 .../instructor_task/tasks_helper.py           |  4 +-
 .../staff_grading_service.py                  |  2 +-
 lms/lib/comment_client/utils.py               |  2 +-
 requirements/edx/local.txt                    |  1 +
 23 files changed, 88 insertions(+), 37 deletions(-)
 create mode 100644 common/lib/dogstats/dogstats_wrapper/__init__.py
 create mode 100644 common/lib/dogstats/dogstats_wrapper/wrapper.py
 create mode 100644 common/lib/dogstats/setup.py

diff --git a/common/djangoapps/monitoring/signals.py b/common/djangoapps/monitoring/signals.py
index f870ff1c54..89cf7cc94b 100644
--- a/common/djangoapps/monitoring/signals.py
+++ b/common/djangoapps/monitoring/signals.py
@@ -10,7 +10,7 @@ the recorded metrics.
 from django.db.models.signals import post_save, post_delete, m2m_changed, post_init
 from django.dispatch import receiver
 
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 
 
 def _database_tags(action, sender, kwargs):
diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py
index 13910c2fe2..24a122e9a5 100644
--- a/common/djangoapps/student/models.py
+++ b/common/djangoapps/student/models.py
@@ -17,7 +17,7 @@ import logging
 from pytz import UTC
 import uuid
 from collections import defaultdict
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 from django.db.models import Q
 import pytz
 
diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py
index 34d3d4c9b1..5e15026011 100644
--- a/common/djangoapps/student/views.py
+++ b/common/djangoapps/student/views.py
@@ -78,7 +78,7 @@ from lang_pref import LANGUAGE_KEY
 
 import track.views
 
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 
 from util.db import commit_on_success_with_read_committed
 from util.json_request import JsonResponse
diff --git a/common/djangoapps/util/views.py b/common/djangoapps/util/views.py
index 05b6789ee5..5ac26f7819 100644
--- a/common/djangoapps/util/views.py
+++ b/common/djangoapps/util/views.py
@@ -9,7 +9,7 @@ from django.views.decorators.csrf import requires_csrf_token
 from django.views.defaults import server_error
 from django.http import (Http404, HttpResponse, HttpResponseNotAllowed,
                          HttpResponseServerError)
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 from edxmako.shortcuts import render_to_response
 import zendesk
 from microsite_configuration import microsite
diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py
index caf2d08452..b1d122b405 100644
--- a/common/lib/capa/capa/responsetypes.py
+++ b/common/lib/capa/capa/responsetypes.py
@@ -33,7 +33,7 @@ from sys import float_info
 from collections import namedtuple
 from shapely.geometry import Point, MultiPoint
 
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 
 # specific library imports
 from calc import evaluator, UndefinedVariable
diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py
index 0603b65237..5b877e998b 100644
--- a/common/lib/capa/capa/xqueue_interface.py
+++ b/common/lib/capa/capa/xqueue_interface.py
@@ -5,7 +5,7 @@ import hashlib
 import json
 import logging
 import requests
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 
 
 log = logging.getLogger(__name__)
diff --git a/common/lib/dogstats/dogstats_wrapper/__init__.py b/common/lib/dogstats/dogstats_wrapper/__init__.py
new file mode 100644
index 0000000000..879139f4e9
--- /dev/null
+++ b/common/lib/dogstats/dogstats_wrapper/__init__.py
@@ -0,0 +1 @@
+from .wrapper import increment, histogram, timer
diff --git a/common/lib/dogstats/dogstats_wrapper/wrapper.py b/common/lib/dogstats/dogstats_wrapper/wrapper.py
new file mode 100644
index 0000000000..d25e317d95
--- /dev/null
+++ b/common/lib/dogstats/dogstats_wrapper/wrapper.py
@@ -0,0 +1,47 @@
+"""
+Wrapper for dog_stats_api, ensuring tags are valid.
+See: http://help.datadoghq.com/customer/portal/questions/908720-api-guidelines
+"""
+from dogapi import dog_stats_api
+
+
+def _clean_tags(tags):
+    """
+    Helper method that does the actual cleaning of tags for sending to statsd.
+    1. Handles any type of tag - a plain string, UTF-8 binary, or a unicode
+       string, and converts it to UTF-8 encoded bytestring needed by statsd.
+    2. Escape pipe character - used by statsd as a field separator.
+    3. Trim to 200 characters (DataDog API limitation)
+    """
+    def clean(tagstr):
+        if isinstance(tagstr, str):
+            return tagstr.replace('|', '_')[:200]
+        return unicode(tagstr).replace('|', '_')[:200].encode("utf-8")
+    return [clean(t) for t in tags]
+
+
+def increment(metric_name, *args, **kwargs):
+    """
+    Wrapper around dog_stats_api.increment that cleans any tags used.
+    """
+    if "tags" in kwargs:
+        kwargs["tags"] = _clean_tags(kwargs["tags"])
+    dog_stats_api.increment(metric_name, *args, **kwargs)
+
+
+def histogram(metric_name, *args, **kwargs):
+    """
+    Wrapper around dog_stats_api.histogram that cleans any tags used.
+    """
+    if "tags" in kwargs:
+        kwargs["tags"] = _clean_tags(kwargs["tags"])
+    dog_stats_api.histogram(metric_name, *args, **kwargs)
+
+
+def timer(metric_name, *args, **kwargs):
+    """
+    Wrapper around dog_stats_api.timer that cleans any tags used.
+    """
+    if "tags" in kwargs:
+        kwargs["tags"] = _clean_tags(kwargs["tags"])
+    return dog_stats_api.timer(metric_name, *args, **kwargs)
diff --git a/common/lib/dogstats/setup.py b/common/lib/dogstats/setup.py
new file mode 100644
index 0000000000..5db5d7cc59
--- /dev/null
+++ b/common/lib/dogstats/setup.py
@@ -0,0 +1,10 @@
+from setuptools import setup
+
+setup(
+    name="dogstats_wrapper",
+    version="0.1",
+    packages=["dogstats_wrapper"],
+    install_requires=[
+        "dogapi",
+    ],
+)
diff --git a/common/lib/xmodule/xmodule/capa_base.py b/common/lib/xmodule/xmodule/capa_base.py
index 4a03e99d96..c0ba5f0643 100644
--- a/common/lib/xmodule/xmodule/capa_base.py
+++ b/common/lib/xmodule/xmodule/capa_base.py
@@ -12,7 +12,7 @@ import sys
 
 # We don't want to force a dependency on datadog, so make the import conditional
 try:
-    from dogapi import dog_stats_api
+    import dogstats_wrapper as dog_stats_api
 except ImportError:
     # pylint: disable=invalid-name
     dog_stats_api = None
diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py
index 1ef75a20ef..bd5c4d592a 100644
--- a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py
+++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py
@@ -1,4 +1,4 @@
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 
 import logging
 from .grading_service_module import GradingService
diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py
index 5626c05b6f..5be3728ae0 100644
--- a/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py
+++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/grading_service_module.py
@@ -2,7 +2,7 @@
 import json
 import logging
 import requests
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 from requests.exceptions import RequestException, ConnectionError, HTTPError
 
 from .combined_open_ended_rubric import CombinedOpenEndedRubric, RubricParsingError
diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py
index 9c5309d25b..0b72ad7b6f 100644
--- a/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py
+++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py
@@ -1,5 +1,5 @@
 import logging
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 
 from .grading_service_module import GradingService
 from opaque_keys.edx.keys import UsageKey
diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py
index 92776eabd8..bb6c486807 100644
--- a/common/lib/xmodule/xmodule/x_module.py
+++ b/common/lib/xmodule/xmodule/x_module.py
@@ -25,7 +25,7 @@ from xmodule.errortracker import exc_info_to_str
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from opaque_keys.edx.keys import UsageKey
 from xmodule.exceptions import UndefinedContext
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 
 
 log = logging.getLogger(__name__)
diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py
index 6a34b7d6d6..3820a9be1f 100644
--- a/lms/djangoapps/bulk_email/tasks.py
+++ b/lms/djangoapps/bulk_email/tasks.py
@@ -7,7 +7,7 @@ import random
 import json
 from time import sleep
 
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 from smtplib import SMTPServerDisconnected, SMTPDataError, SMTPConnectError, SMTPException
 from boto.ses.exceptions import (
     SESAddressNotVerifiedError,
@@ -690,7 +690,7 @@ def _submit_for_retry(entry_id, email_id, to_list, global_email_context, current
 
 def _statsd_tag(course_title):
     """
-    Calculate the tag we will use for DataDog.
+    Prefix the tag we will use for DataDog.
+    The tag also gets modified by our dogstats_wrapper code.
     """
-    tag = u"course_email:{0}".format(course_title).encode('utf-8')
-    return tag[:200]
+    return u"course_email:{0}".format(course_title)
diff --git a/lms/djangoapps/certificates/views.py b/lms/djangoapps/certificates/views.py
index 53073d1cf1..7ad5aeb8ef 100644
--- a/lms/djangoapps/certificates/views.py
+++ b/lms/djangoapps/certificates/views.py
@@ -1,5 +1,5 @@
 """URL handlers related to certificate handling by LMS"""
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 import json
 import logging
 
diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py
index 72bac150f7..61781dbc78 100644
--- a/lms/djangoapps/courseware/grades.py
+++ b/lms/djangoapps/courseware/grades.py
@@ -10,7 +10,7 @@ from django.conf import settings
 from django.db import transaction
 from django.test.client import RequestFactory
 
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 
 from courseware import courses
 from courseware.model_data import FieldDataCache
@@ -522,7 +522,7 @@ def iterate_grades_for(course_id, students):
     request = RequestFactory().get('/')
 
     for student in students:
-        with dog_stats_api.timer('lms.grades.iterate_grades_for', tags=['action:{}'.format(course_id)]):
+        with dog_stats_api.timer('lms.grades.iterate_grades_for', tags=[u'action:{}'.format(course_id)]):
             try:
                 request.user = student
                 # Grading calls problem rendering, which calls masquerading,
diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py
index f80943f729..c948cd3d3d 100644
--- a/lms/djangoapps/courseware/module_render.py
+++ b/lms/djangoapps/courseware/module_render.py
@@ -7,7 +7,7 @@ import xblock.reference.plugins
 
 from functools import partial
 from requests.auth import HTTPBasicAuth
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 from opaque_keys import InvalidKeyError
 
 from django.conf import settings
diff --git a/lms/djangoapps/instructor_task/subtasks.py b/lms/djangoapps/instructor_task/subtasks.py
index e450a6cd23..6280213e76 100644
--- a/lms/djangoapps/instructor_task/subtasks.py
+++ b/lms/djangoapps/instructor_task/subtasks.py
@@ -10,7 +10,7 @@ from contextlib import contextmanager
 
 from celery.utils.log import get_task_logger
 from celery.states import SUCCESS, READY_STATES, RETRY
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 
 from django.db import transaction, DatabaseError
 from django.core.cache import cache
@@ -393,7 +393,7 @@ def check_subtask_is_valid(entry_id, current_task_id, new_subtask_status):
         format_str = "Unexpected task_id '{}': unable to find subtasks of instructor task '{}': rejecting task {}"
         msg = format_str.format(current_task_id, entry, new_subtask_status)
         TASK_LOG.warning(msg)
-        dog_stats_api.increment('instructor_task.subtask.duplicate.nosubtasks', tags=[_statsd_tag(entry.course_id)])
+        dog_stats_api.increment('instructor_task.subtask.duplicate.nosubtasks', tags=[entry.course_id])
         raise DuplicateTaskException(msg)
 
     # Confirm that the InstructorTask knows about this particular subtask.
@@ -403,7 +403,7 @@ def check_subtask_is_valid(entry_id, current_task_id, new_subtask_status):
         format_str = "Unexpected task_id '{}': unable to find status for subtask of instructor task '{}': rejecting task {}"
         msg = format_str.format(current_task_id, entry, new_subtask_status)
         TASK_LOG.warning(msg)
-        dog_stats_api.increment('instructor_task.subtask.duplicate.unknown', tags=[_statsd_tag(entry.course_id)])
+        dog_stats_api.increment('instructor_task.subtask.duplicate.unknown', tags=[entry.course_id])
         raise DuplicateTaskException(msg)
 
     # Confirm that the InstructorTask doesn't think that this subtask has already been
@@ -414,7 +414,7 @@ def check_subtask_is_valid(entry_id, current_task_id, new_subtask_status):
         format_str = "Unexpected task_id '{}': already completed - status {} for subtask of instructor task '{}': rejecting task {}"
         msg = format_str.format(current_task_id, subtask_status, entry, new_subtask_status)
         TASK_LOG.warning(msg)
-        dog_stats_api.increment('instructor_task.subtask.duplicate.completed', tags=[_statsd_tag(entry.course_id)])
+        dog_stats_api.increment('instructor_task.subtask.duplicate.completed', tags=[entry.course_id])
         raise DuplicateTaskException(msg)
 
     # Confirm that the InstructorTask doesn't think that this subtask is already being
@@ -428,7 +428,7 @@ def check_subtask_is_valid(entry_id, current_task_id, new_subtask_status):
             format_str = "Unexpected task_id '{}': already retried - status {} for subtask of instructor task '{}': rejecting task {}"
             msg = format_str.format(current_task_id, subtask_status, entry, new_subtask_status)
             TASK_LOG.warning(msg)
-            dog_stats_api.increment('instructor_task.subtask.duplicate.retried', tags=[_statsd_tag(entry.course_id)])
+            dog_stats_api.increment('instructor_task.subtask.duplicate.retried', tags=[entry.course_id])
             raise DuplicateTaskException(msg)
 
     # Now we are ready to start working on this.  Try to lock it.
@@ -438,7 +438,7 @@ def check_subtask_is_valid(entry_id, current_task_id, new_subtask_status):
         format_str = "Unexpected task_id '{}': already being executed - for subtask of instructor task '{}'"
         msg = format_str.format(current_task_id, entry)
         TASK_LOG.warning(msg)
-        dog_stats_api.increment('instructor_task.subtask.duplicate.locked', tags=[_statsd_tag(entry.course_id)])
+        dog_stats_api.increment('instructor_task.subtask.duplicate.locked', tags=[entry.course_id])
         raise DuplicateTaskException(msg)
 
 
@@ -570,11 +570,3 @@ def _update_subtask_status(entry_id, current_task_id, new_subtask_status):
     else:
         TASK_LOG.debug("about to commit....")
         transaction.commit()
-
-
-def _statsd_tag(course_id):
-    """
-    Calculate the tag we will use for DataDog.
-    """
-    tag = unicode(course_id).encode('utf-8')
-    return tag[:200]
diff --git a/lms/djangoapps/instructor_task/tasks_helper.py b/lms/djangoapps/instructor_task/tasks_helper.py
index d1a8fbebd5..9b7bfcae06 100644
--- a/lms/djangoapps/instructor_task/tasks_helper.py
+++ b/lms/djangoapps/instructor_task/tasks_helper.py
@@ -13,7 +13,7 @@ from celery.utils.log import get_task_logger
 from celery.states import SUCCESS, FAILURE
 from django.contrib.auth.models import User
 from django.db import transaction, reset_queries
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 from pytz import UTC
 
 from xmodule.modulestore.django import modulestore
@@ -198,7 +198,7 @@ def run_main_task(entry_id, task_fcn, action_name):
         raise ValueError(message)
 
     # Now do the work:
-    with dog_stats_api.timer('instructor_tasks.time.overall', tags=['action:{name}'.format(name=action_name)]):
+    with dog_stats_api.timer('instructor_tasks.time.overall', tags=[u'action:{name}'.format(name=action_name)]):
         task_progress = task_fcn(entry_id, course_id, task_input, action_name)
 
     # Release any queries that the connection has been hanging onto:
diff --git a/lms/djangoapps/open_ended_grading/staff_grading_service.py b/lms/djangoapps/open_ended_grading/staff_grading_service.py
index 946bbcc107..c6042d3f51 100644
--- a/lms/djangoapps/open_ended_grading/staff_grading_service.py
+++ b/lms/djangoapps/open_ended_grading/staff_grading_service.py
@@ -19,7 +19,7 @@ from edxmako.shortcuts import render_to_string
 from student.models import unique_id_for_user
 
 from open_ended_grading.utils import does_location_exist
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 
 log = logging.getLogger(__name__)
 
diff --git a/lms/lib/comment_client/utils.py b/lms/lib/comment_client/utils.py
index 56ae8d5269..626880c626 100644
--- a/lms/lib/comment_client/utils.py
+++ b/lms/lib/comment_client/utils.py
@@ -1,5 +1,5 @@
 from contextlib import contextmanager
-from dogapi import dog_stats_api
+import dogstats_wrapper as dog_stats_api
 import logging
 import requests
 from django.conf import settings
diff --git a/requirements/edx/local.txt b/requirements/edx/local.txt
index c677c8d087..48bc8294da 100644
--- a/requirements/edx/local.txt
+++ b/requirements/edx/local.txt
@@ -3,6 +3,7 @@
 -e common/lib/calc
 -e common/lib/capa
 -e common/lib/chem
+-e common/lib/dogstats
 -e common/lib/safe_lxml
 -e common/lib/sandbox-packages
 -e common/lib/symmath