From 186f5fbce29f7f2658ee5ee13be66e6acbf566be Mon Sep 17 00:00:00 2001 From: Robert Raposa Date: Wed, 18 Nov 2020 12:05:09 -0500 Subject: [PATCH] track: catch unhandled exceptions and add monitoring Add the following monitoring custom attributes: - segment_event_name - segment_event_source - segment_unexpected_context - segment_unexpected_data Also, this explicitly raises a validation error whenever context is a string instead of throwing a vague TypeError. Related to LEARNER-8034 --- common/djangoapps/track/views/segmentio.py | 16 +++++++++++++- .../track/views/tests/test_segmentio.py | 22 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/track/views/segmentio.py b/common/djangoapps/track/views/segmentio.py index 87c84cd60d..8b99142961 100644 --- a/common/djangoapps/track/views/segmentio.py +++ b/common/djangoapps/track/views/segmentio.py @@ -10,6 +10,7 @@ from django.contrib.auth.models import User from django.http import HttpResponse from django.views.decorators.csrf import csrf_exempt from django.views.decorators.http import require_POST +from edx_django_utils.monitoring import set_custom_attribute from eventtracking import tracker from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey @@ -23,6 +24,8 @@ ERROR_UNAUTHORIZED = 'Unauthorized' ERROR_MISSING_USER_ID = 'Required user_id missing from context' ERROR_USER_NOT_EXIST = 'Specified user does not exist' ERROR_INVALID_USER_ID = 'Unable to parse userId as an integer' +ERROR_INVALID_CONTEXT_FIELD_TYPE = 'The properties.context field is not a dict.' +ERROR_INVALID_DATA_FIELD_TYPE = 'The properties.data field is not a dict.' ERROR_MISSING_DATA = 'The data field must be specified in the properties dictionary' ERROR_MISSING_NAME = 'The name field must be specified in the properties dictionary' ERROR_MISSING_TIMESTAMP = 'Required timestamp field not found' @@ -155,11 +158,22 @@ def track_segmentio_event(request): # pylint: disable=too-many-statements if any(disallowed_subs_name in segment_event_name.lower() for disallowed_subs_name in disallowed_substring_names): return + set_custom_attribute('segment_event_name', segment_event_name) + set_custom_attribute('segment_event_source', event_source) + + # Attempt to extract and validate the data field. if 'data' not in segment_properties: raise EventValidationError(ERROR_MISSING_DATA) + segment_event_data = segment_properties.get('data', {}) + if type(segment_event_data) is not dict: + set_custom_attribute('segment_unexpected_data', str(segment_event_data)) + raise EventValidationError(ERROR_INVALID_DATA_FIELD_TYPE) # create and populate application field if it doesn't exist app_context = segment_properties.get('context', {}) + if type(app_context) is not dict: + set_custom_attribute('segment_unexpected_context', str(app_context)) + raise EventValidationError(ERROR_INVALID_CONTEXT_FIELD_TYPE) if 'application' not in app_context: context['application'] = { 'name': app_context.get('app_name', ''), @@ -231,7 +245,7 @@ def track_segmentio_event(request): # pylint: disable=too-many-statements context['course_id'] = segment_properties['course_id'] with tracker.get_tracker().context('edx.segmentio', context): - tracker.emit(segment_event_name, segment_properties.get('data', {})) + tracker.emit(segment_event_name, segment_event_data) def _get_segmentio_event_name(event_properties): diff --git a/common/djangoapps/track/views/tests/test_segmentio.py b/common/djangoapps/track/views/tests/test_segmentio.py index dd84673d2f..ccc0089f8e 100644 --- a/common/djangoapps/track/views/tests/test_segmentio.py +++ b/common/djangoapps/track/views/tests/test_segmentio.py @@ -180,6 +180,28 @@ class SegmentIOTrackingTestCase(SegmentIOTrackingTestCaseBase): segmentio.track_segmentio_event(request) self.assert_events_emitted() + @data( + None, + 'a string', + ['a', 'list'], + ) + @expect_failure_with_message(segmentio.ERROR_INVALID_CONTEXT_FIELD_TYPE) + def test_invalid_context_field_type(self, invalid_value): + sample_event_raw = self.create_segmentio_event() + sample_event_raw['properties']['context'] = invalid_value + self.post_modified_segmentio_event(sample_event_raw) + + @data( + None, + 'a string', + ['a', 'list'], + ) + @expect_failure_with_message(segmentio.ERROR_INVALID_DATA_FIELD_TYPE) + def test_invalid_data_field_type(self, invalid_value): + sample_event_raw = self.create_segmentio_event() + sample_event_raw['properties']['data'] = invalid_value + self.post_modified_segmentio_event(sample_event_raw) + @expect_failure_with_message(segmentio.ERROR_MISSING_NAME) def test_missing_name(self): sample_event_raw = self.create_segmentio_event()