Merge pull request #5431 from open-craft/datadog-wrapper

Consistently wrap the DataDog API to prevent logged errors & missing stats data

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):

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
 

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

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

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

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__)

common/lib/dogstats/dogstats_wrapper/__init__.py

@@ -0,0 +1 @@
+from .wrapper import increment, histogram, timer

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)

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",
+    ],
+)

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

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

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

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

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__)

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)

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
 

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,

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

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]

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:

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__)
 

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

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