From f21bf05e37b8cc511b8ed83551ad150dc3590256 Mon Sep 17 00:00:00 2001 From: Brian Wilson Date: Fri, 30 Nov 2018 17:51:52 -0500 Subject: [PATCH] Add page context to edx.bi.email.sent events. --- lms/djangoapps/discussion/tasks.py | 23 +++++++++++----- lms/djangoapps/discussion/tests/test_tasks.py | 15 ++++++----- .../commands/tests/send_email_base.py | 4 +-- openedx/core/djangoapps/schedules/tasks.py | 27 ++++++++++++++----- 4 files changed, 48 insertions(+), 21 deletions(-) diff --git a/lms/djangoapps/discussion/tasks.py b/lms/djangoapps/discussion/tasks.py index 19a94732e1..acd4f66a99 100644 --- a/lms/djangoapps/discussion/tasks.py +++ b/lms/djangoapps/discussion/tasks.py @@ -5,7 +5,6 @@ pertaining to new discussion forum comments. import logging from urlparse import urljoin -import analytics from celery import task from django.conf import settings from django.contrib.auth.models import User @@ -16,6 +15,7 @@ from django_comment_common.models import DiscussionsIdMapping from edx_ace import ace from edx_ace.utils import date from edx_ace.recipient import Recipient +from eventtracking import tracker from opaque_keys.edx.keys import CourseKey from lms.djangoapps.django_comment_client.utils import permalink, get_accessible_discussion_xblocks_by_course_id import lms.lib.comment_client as cc @@ -24,6 +24,7 @@ from openedx.core.djangoapps.content.course_overviews.models import CourseOvervi from openedx.core.djangoapps.ace_common.template_context import get_base_template_context from openedx.core.djangoapps.ace_common.message import BaseMessageType from openedx.core.lib.celery.task_utils import emulate_http_request +from track import segment log = logging.getLogger(__name__) @@ -85,15 +86,23 @@ def _track_notification_sent(message, context): 'uuid': unicode(message.uuid), 'send_uuid': unicode(message.send_uuid), 'thread_id': context['thread_id'], + 'course_id': unicode(context['course_id']), 'thread_created_at': date.deserialize(context['thread_created_at']), 'nonInteraction': 1, } - analytics.track( - user_id=context['thread_author_id'], - event='edx.bi.email.sent', - course_id=context['course_id'], - properties=properties - ) + tracking_context = { + 'host': context['site'].domain, + 'path': '/', # make up a value, in order to allow the host to be passed along. + } + # The event used to specify the user_id as being the recipient of the email (i.e. the thread_author_id). + # This has the effect of interrupting the actual chain of events for that author, if any, while the + # email-sent event should really be associated with the sender, since that is what triggers the event. + with tracker.get_tracker().context(properties['app_label'], tracking_context): + segment.track( + user_id=context['thread_author_id'], + event_name='edx.bi.email.sent', + properties=properties + ) def _should_send_message(context): diff --git a/lms/djangoapps/discussion/tests/test_tasks.py b/lms/djangoapps/discussion/tests/test_tasks.py index 9ccefe0767..4446d6680a 100644 --- a/lms/djangoapps/discussion/tests/test_tasks.py +++ b/lms/djangoapps/discussion/tests/test_tasks.py @@ -288,6 +288,7 @@ class TaskTestCase(ModuleStoreTestCase): 'uuid': 'uuid1', 'send_uuid': 'uuid2', 'thread_id': 'dummy_discussion_id', + 'course_id': 'fake_course_edx', 'thread_created_at': datetime(2000, 1, 1, 0, 0, 0) } ), ( @@ -305,6 +306,7 @@ class TaskTestCase(ModuleStoreTestCase): 'uuid': 'uuid3', 'send_uuid': 'uuid4', 'thread_id': 'dummy_discussion_id2', + 'course_id': 'fake_course_edx2', 'thread_created_at': datetime(2000, 1, 1, 0, 0, 0) } @@ -318,12 +320,13 @@ class TaskTestCase(ModuleStoreTestCase): setattr(message, key, entry) test_props['nonInteraction'] = True - - with mock.patch('analytics.track') as mock_analytics_track: + # Also augment context with site object, for setting segment context. + site = Site.objects.get_current() + context['site'] = site + with mock.patch('lms.djangoapps.discussion.tasks.segment.track') as mock_segment_track: _track_notification_sent(message, context) - mock_analytics_track.assert_called_once_with( + mock_segment_track.assert_called_once_with( user_id=context['thread_author_id'], - event='edx.bi.email.sent', - course_id=context['course_id'], - properties=test_props + event_name='edx.bi.email.sent', + properties=test_props, ) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py index 8418bc01f3..d80b5d3e02 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py @@ -415,10 +415,10 @@ class ScheduleSendEmailTestMixin(FilteredQueryCountMixin): self.assertEqual(len(sent_messages), num_expected_messages) with self.assertNumQueries(NUM_QUERIES_PER_MESSAGE_DELIVERY): - with patch('analytics.track') as mock_analytics_track: + with patch('openedx.core.djangoapps.schedules.tasks.segment.track') as mock_segment_track: with patch('edx_ace.channel.channels', return_value=channel_map): self.deliver_task(*sent_messages[0]) - self.assertEqual(mock_analytics_track.call_count, 1) + self.assertEqual(mock_segment_track.call_count, 1) self.assertEqual(mock_channel.deliver.call_count, 1) for (_name, (_msg, email), _kwargs) in mock_channel.deliver.mock_calls: diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index dcc796dc53..ee96050311 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -1,7 +1,6 @@ import datetime import logging -import analytics from celery import task from django.conf import settings from django.contrib.auth.models import User @@ -16,12 +15,15 @@ from edx_ace import ace from edx_ace.message import Message from edx_ace.utils.date import deserialize, serialize from edx_django_utils.monitoring import set_custom_metric +from eventtracking import tracker from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.schedules import message_types from openedx.core.djangoapps.schedules.models import Schedule, ScheduleConfig from openedx.core.djangoapps.schedules import resolvers from openedx.core.lib.celery.task_utils import emulate_http_request +from track import segment + LOG = logging.getLogger(__name__) @@ -224,11 +226,24 @@ def _track_message_sent(site, user, msg): properties['course_ids'] = course_ids[:10] properties['primary_course_id'] = course_ids[0] - analytics.track( - user_id=user.id, - event='edx.bi.email.sent', - properties=properties - ) + tracking_context = { + 'host': site.domain, + 'path': '/', # make up a value, in order to allow the host to be passed along. + } + # I wonder if the user of this event should be the recipient, as they are not the ones + # who took an action. Rather, the system is acting, and they are the object. + # Admittedly that may be what 'nonInteraction' is meant to address. But sessionization may + # get confused by these events if they're attributed in this way, because there's no way for + # this event to get context that would match with what the user might be doing at the moment. + # But the events do show up in GA being joined up with existing sessions (i.e. within a half + # hour in the past), so they don't always break sessions. Not sure what happens after these. + # We can put the recipient_user_id into the properties, and then export as a custom dimension. + with tracker.get_tracker().context(msg.app_label, tracking_context): + segment.track( + user_id=user.id, + event_name='edx.bi.email.sent', + properties=properties, + ) def _is_delivery_enabled(site, delivery_config_var, log_prefix):