Skip to content
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

xAPI mapping is using terminated event for video where it should be using paused. #230

Open
e0d opened this issue Sep 21, 2022 · 4 comments

Comments

@e0d
Copy link
Contributor

e0d commented Sep 21, 2022

The video profile defines the paused verb as:

Indicates that the actor temporary or permanently stopped experiencing the recorded media object.

    'stop_video': {
        'id': constants.XAPI_VERB_TERMINATED,
        'display': constants.TERMINATED
    },
    'edx.video.stopped': {
        'id': constants.XAPI_VERB_TERMINATED,
        'display': constants.TERMINATED
    },

Code is here.

Terminated is defined as

Used to express that the actor ended a video.

Terminated could also be sent, though it's optional, but only if a pause event is also sent first.

The LRP MAY use "Terminated" when the Actor has terminated the video.
A "Paused" statement MUST be sent before "Terminated" statement if not already sent.
Any statements after this in the current video session MUST NOT be sent, and will be ignored if sent.

There's another question here, does the current backend support chaining xAPI statements such that multiple are emitted for a single platform event?

@e0d e0d added this to the xapi-video-profile milestone Sep 21, 2022
@bmtcril
Copy link
Contributor

bmtcril commented Sep 22, 2022

My reading of the current code is that each edX event can be mapped to multiple processors (xAPI, Caliper) but is expected to be transformed into one output statement for each of those. @ziafazal can correct me if I'm wrong, though.

@e0d
Copy link
Contributor Author

e0d commented Sep 22, 2022

Another requirement is that from time-to-time the spec requires multiple statements to come is a specific order.

@ziafazal
Copy link
Contributor

@bmtcril yes you read it right we can multiple backends but one event is going to transformed into one statement for each backend.
Ordering statements can be challenging because we don't have any persistence layer at the moment.

@bmtcril
Copy link
Contributor

bmtcril commented Nov 4, 2022

From our Slack thread about the requirement to order events, here are my thoughts on ordering guarantees. I'm curious what @e0d thinks, and have an item to bring it up to the Data WG next meeting as well.

Yes, this is a difficult issue and one I think we need to address as a group. The timing of events is very dependent on configuration of the servers, even if the backend supports ordering guarantees.

For instance, redis streams will deliver events in the order that they are received, but it’s possible for the LMS to be run in configurations where a rapid series of requests get load balanced to different servers (ex: a round robin config without sticky sessions), which may have different levels of load that cause the actual insertion into redis to be out of order. It also complicates disaster recovery situations where we may be replaying log files from different servers into the storage backend, where events would likely arrive very out of order.

The events will still have their original timestamps for reporting / analytic purposes. Even with synchronous communication to an LRS I’m not sure we could provide this as a guarantee unless we single threaded the whole app. 🙂

I think this is a place where we may have to deviate from the spec, but I’d like to discuss it with the WG in the next meeting to see if someone smarter than I am can untangle a solution. I’ll put it on the agenda!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants