-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat: add tracking logs #173
base: master
Are you sure you want to change the base?
Conversation
d0f60e3
to
ead8f61
Compare
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.
Thank you for preparing this PR, @andrey-canon!
@@ -81,5 +96,7 @@ def root(*args): | |||
] | |||
USE_TZ = True | |||
|
|||
EVENT_TRACKING_ENABLED = True |
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.
This variable is unused. Should we use it to gate this feature?
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.
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 DJANGO_ENABLED_SETTING_NAME
which is EVENT_TRACKING_ENABLED
as you can see in this line
completion_aggregator/models.py
Outdated
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, | ||
} | ||
) |
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.
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.
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.
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 EVENT_TRACKING_BACKENDS
value
{'tracking_logs': {'ENGINE': 'eventtracking.backends.routing.RoutingBackend',
'OPTIONS': {'backends': {'logger': {'ENGINE': 'eventtracking.backends.logger.LoggerBackend',
'OPTIONS': {'name': 'tracking', 'max_event_size': 50000}}},
'processors': [{'ENGINE': 'common.djangoapps.track.shim.LegacyFieldMappingProcessor'},
{'ENGINE': 'common.djangoapps.track.shim.PrefixedEventProcessor'}]}},
'segmentio': {'ENGINE': 'eventtracking.backends.routing.RoutingBackend',
'OPTIONS': {'backends': {'segment': {'ENGINE': 'eventtracking.backends.segment.SegmentBackend'}},
'processors': [{'ENGINE': 'eventtracking.processors.whitelist.NameWhitelistProcessor',
'OPTIONS': {'whitelist': []}},
{'ENGINE': 'common.djangoapps.track.shim.GoogleAnalyticsProcessor'}]}},
'xapi': {'ENGINE': 'eventtracking.backends.async_routing.AsyncRoutingBackend',
'OPTIONS': {'backend_name': 'xapi',
'processors': [{'ENGINE': 'eventtracking.processors.whitelist.NameWhitelistProcessor',
'OPTIONS': {'whitelist': ['edx.problem.completed',
'showanswer',
'edx.completion.block_completion.changed',
'edx.grades.problem.submitted',
'edx.course.enrollment.activated',
'edx.course.enrollment.deactivated',
'edx.course.grade.passed.first_time',
'edx.course.grade.now_passed',
'edx.course.grade.now_failed',
'load_video',
'edx.video.loaded',
'play_video',
'edx.video.played',
'stop_video',
'edx.video.stopped',
'complete_video',
'edx.video.completed',
'pause_video',
'edx.video.paused']}}],
'backends': {'xapi': {'ENGINE': 'event_routing_backends.backends.events_router.EventsRouter',
'OPTIONS': {'processors': [{'ENGINE': 'event_routing_backends.processors.xapi.transformer_processor.XApiProcessor',
'OPTIONS': {}}],
'backend_name': 'xapi'}}}}},
'caliper': {'ENGINE': 'eventtracking.backends.async_routing.AsyncRoutingBackend',
'OPTIONS': {'backend_name': 'caliper',
'processors': [{'ENGINE': 'eventtracking.processors.whitelist.NameWhitelistProcessor',
'OPTIONS': {'whitelist': ['edx.course.enrollment.activated',
'edx.course.enrollment.deactivated',
'edx.ui.lms.link_clicked',
'edx.ui.lms.sequence.outline.selected',
'edx.ui.lms.outline.selected',
'edx.ui.lms.sequence.next_selected',
'edx.ui.lms.sequence.previous_selected',
'edx.ui.lms.sequence.tab_selected',
'showanswer',
'edx.problem.hint.demandhint_displayed',
'problem_check',
'load_video',
'edx.video.loaded',
'play_video',
'edx.video.played',
'complete_video',
'edx.video.completed',
'stop_video',
'edx.video.stopped',
'pause_video',
'edx.video.paused',
'seek_video',
'edx.video.position.changed',
'edx.course.grade.passed.first_time',
'edx.course.grade.now_passed',
'edx.course.grade.now_failed']}}],
'backends': {'caliper': {'ENGINE': 'event_routing_backends.backends.events_router.EventsRouter',
'OPTIONS': {'processors': [{'ENGINE': 'event_routing_backends.processors.caliper.transformer_processor.CaliperProcessor',
'OPTIONS': {}},
{'ENGINE': 'event_routing_backends.processors.caliper.envelope_processor.CaliperEnvelopeProcessor',
'OPTIONS': {'sensor_id': 'https://localhost:18000'}}],
'backend_name': 'caliper'}}}}}}
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
"progress": [ | ||
"course", | ||
"chapter", | ||
"sequential", | ||
"vertical", | ||
], | ||
"completion": [ | ||
"course", | ||
"chapter", | ||
"sequential", | ||
"vertical", | ||
] |
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.
"progress": [ | |
"course", | |
"chapter", | |
"sequential", | |
"vertical", | |
], | |
"completion": [ | |
"course", | |
"chapter", | |
"sequential", | |
"vertical", | |
] | |
"progress": { | |
"course", | |
"chapter", | |
"sequential", | |
"vertical", | |
}, | |
"completion": { | |
"course", | |
"chapter", | |
"sequential", | |
"vertical", | |
} |
completion_aggregator/models.py
Outdated
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, []): |
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.
if event_type not in settings.ALLOWED_COMPLETION_AGGREGATOR_EVENT_TYPES.get(event, []): | |
if event_type not in settings.ALLOWED_COMPLETION_AGGREGATOR_EVENT_TYPES.get(event, {}): |
@andrey-canon, I will review and test this PR next week. |
Description:
This changes allow to emit events when an aggregator is created or updated, and basically I'm proposing this to integrate those new events with the event-routing-backends library, which takes advantages of the tracking events to publish that data to a Xapi or caliper service. The default behavior that I'm proposing has the following events
Progress events, those will be emitted when the block has not reached the 100% of completion
edx.completion_aggregator.progress.course
edx.completion_aggregator.progress.chapter
edx.completion_aggregator.progress.sequential
edx.completion_aggregator.progress.vertical
Completion events, those will be emitted when the block has reached the 100% of completion
edx.completion_aggregator.completion.course
edx.completion_aggregator.completion.chapter
edx.completion_aggregator.completion.sequential
edx.completion_aggregator.completion.vertical
Testing instructions:
Reviewers:
Merge checklist:
Post merge:
finished.
Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.