-
Notifications
You must be signed in to change notification settings - Fork 26
feat: add tracking logs #173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
|
|
||
| from __future__ import absolute_import, division, print_function, unicode_literals | ||
|
|
||
| from eventtracking import tracker | ||
| from opaque_keys.edx.django.models import CourseKeyField, UsageKeyField | ||
| from opaque_keys.edx.keys import CourseKey, UsageKey | ||
|
|
||
|
|
@@ -171,8 +172,44 @@ def submit_completion(self, user, course_key, block_key, aggregation_name, earne | |
| 'last_modified': last_modified, | ||
| }, | ||
| ) | ||
| self.emit_completion_aggregator_logs([obj]) | ||
|
|
||
| return obj, is_new | ||
|
|
||
| @staticmethod | ||
| def emit_completion_aggregator_logs(updated_aggregators): | ||
| """ | ||
| Emit a tracking log for each element of the list parameter. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| updated_aggregators: List of Aggregator intances | ||
|
|
||
| """ | ||
| for obj in updated_aggregators: | ||
| event = "progress" if obj.percent < 1 else "completion" | ||
| event_type = obj.aggregation_name | ||
|
|
||
| if event_type not in settings.ALLOWED_COMPLETION_AGGREGATOR_EVENT_TYPES.get(event, []): | ||
| continue | ||
|
|
||
| event_name = f"edx.completion_aggregator.{event}.{event_type}" | ||
|
|
||
| tracker.emit( | ||
| event_name, | ||
| { | ||
| "user_id": obj.user_id, | ||
| "course_id": str(obj.course_key), | ||
| "block_id": str(obj.block_key), | ||
| "modified": obj.modified, | ||
| "created": obj.created, | ||
| "earned": obj.earned, | ||
| "possible": obj.possible, | ||
| "percent": obj.percent, | ||
| "type": event_type, | ||
| } | ||
| ) | ||
|
Comment on lines
+198
to
+211
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the cost of this operation (especially when we use multiple event-tracking backends)? Is it possible to emit multiple events in bulk? I want to be particularly cautious about this, as we had faced a partial outage before due to the high volume of additional operations.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After running a basic test in my local I got the following As you can see the time is in the order of 0.002 seconds, that test was performed with the following If I'm not wrong tracking_logs and segmentio comes by default, xapi and caliper are added by the event-routing-backends library. Finally I'm not aware of a bulk emit capability, I couldn't find anything related to that in the eventtracking library and all the implementation that I found are using the single emit method |
||
|
|
||
| def bulk_create_or_update(self, updated_aggregators): | ||
| """ | ||
| Update the collection of aggregator object using mysql insert on duplicate update query. | ||
|
|
@@ -194,6 +231,7 @@ def bulk_create_or_update(self, updated_aggregators): | |
| else: | ||
| aggregation_data = [obj.get_values() for obj in updated_aggregators] | ||
| cur.executemany(INSERT_OR_UPDATE_AGGREGATOR_QUERY, aggregation_data) | ||
| self.emit_completion_aggregator_logs(updated_aggregators) | ||
|
|
||
|
|
||
| class Aggregator(TimeStampedModel): | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -9,6 +9,23 @@ def plugin_settings(settings): | |||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| Modify the provided settings object with settings specific to this plugin. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # Emit feature allows to publish two kind of events progress and completion | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # This setting controls which type of event will be published to change the default behavior | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # the block type should be removed or added from the progress or completion list. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| settings.ALLOWED_COMPLETION_AGGREGATOR_EVENT_TYPES = { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "progress": [ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "course", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "chapter", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "sequential", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "vertical", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "completion": [ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "course", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "chapter", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "sequential", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| "vertical", | ||||||||||||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| "progress": [ | |
| "course", | |
| "chapter", | |
| "sequential", | |
| "vertical", | |
| ], | |
| "completion": [ | |
| "course", | |
| "chapter", | |
| "sequential", | |
| "vertical", | |
| ] | |
| "progress": { | |
| "course", | |
| "chapter", | |
| "sequential", | |
| "vertical", | |
| }, | |
| "completion": { | |
| "course", | |
| "chapter", | |
| "sequential", | |
| "vertical", | |
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,20 @@ def root(*args): | |
| return join(abspath(dirname(__file__)), *args) | ||
|
|
||
|
|
||
| ALLOWED_COMPLETION_AGGREGATOR_EVENT_TYPES = { | ||
| "progress": [ | ||
| "course", | ||
| "chapter", | ||
| "sequential", | ||
| "vertical", | ||
| ], | ||
| "completion": [ | ||
| "course", | ||
| "chapter", | ||
| "sequential", | ||
| "vertical", | ||
| ] | ||
| } | ||
| AUTH_USER_MODEL = 'auth.User' | ||
| CELERY_ALWAYS_EAGER = True | ||
| COMPLETION_AGGREGATOR_BLOCK_TYPES = {'course', 'chapter', 'sequential'} | ||
|
|
@@ -53,6 +67,7 @@ def root(*args): | |
| 'oauth2_provider', | ||
| 'waffle', | ||
| 'test_utils.test_app', | ||
| 'eventtracking.django.apps.EventTrackingConfig', | ||
| ) | ||
|
|
||
| LOCALE_PATHS = [root('completion_aggregator', 'conf', 'locale')] | ||
|
|
@@ -81,5 +96,7 @@ def root(*args): | |
| ] | ||
| USE_TZ = True | ||
|
|
||
| EVENT_TRACKING_ENABLED = True | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This variable is unused. Should we use it to gate this feature?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is used by the eventtracking library, in the test scenario I'm implementing the default django_tracker that is added by this AppConfig, that uses this method to register the tracker, after checking the |
||
|
|
||
| # pylint: disable=unused-import,wrong-import-position | ||
| from test_utils.test_app import celery # isort:skip | ||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.