xAPI-vle / moodle-logstore_xapi

A Moodle plugin to send xAPI statements to an LRS using events in the Moodle logstore.
GNU General Public License v3.0
75 stars 85 forks source link

Add JISC context extensions to statements. #422

Closed ryasmi closed 5 years ago

ryasmi commented 5 years ago

JISC are using these context extensions to all statements, I'm not sure whether to add them behind a flag or whether they're useful anyway. This is part of an effort to get all users of JISC's fork back to using the core plugin (#50 and #66 will also be needed).

{
  "http://xapi.jisc.ac.uk/sessionId": "sesskey()",
  "http://id.tincanapi.com/extension/ip-address": "$event->ip",
  "http://xapi.jisc.ac.uk/courseArea": {
    "http://xapi.jisc.ac.uk/vle_mod_id": "$course->shortname"
  },
  "http://xapi.jisc.ac.uk/statementCat": "VLE"
}
ryasmi commented 5 years ago

Would be good to get thoughts from @garemoko and @davidpesce on this if that's okay guys?

davidpesce commented 5 years ago

It seems a bit specific to one org. Does it provide value to others?

garemoko commented 5 years ago

@ryansmith94 can you provide any context on what these things are?

For ip address, I would definitely want to make recording that optional as there may be GDPR and/or security implications, or at least perceived implications (if it's an event property I guess that means it's already being stored in Moodle).

In general, I'm in favor of getting jisc 's fork back and if adding this properties along with a "JISC mode" setting or something is a quick way to do that, I think that's a good idea.

ryasmi commented 5 years ago

Hey both, thanks for sharing thoughts on this.

I agree David it is specific to one org in the sense that it's JISC, but JISC's fork of the plugin is actually used by many UK institutions so it does provide value to multiple organisations in that sense.

Andrew agree with the IP Address, I think the JISC extensions/mode flag is enough to cover that if we note in the setting description that it will record IP addresses and session IDs. Glad you're in favour of getting JISC back into this repository. Can you provide a bit more detail in terms of what you're looking for with the context on what these things are? I don't really know why the vle_mod_id is wrapped within the courseArea extension, but unfortunately it will need to stay like that for backwards compatibility for their institutions.

garemoko commented 5 years ago

Actually I think the only one I can't work out myself is the sess key - I assume that's a key for the session, but is it some Moodle thing that already exists or some custom generated thing? is it a UUID?

davidpesce commented 5 years ago

You can pull session info from the global $SESSION variable.

ryasmi commented 5 years ago

@garemoko yeah I think it's just a key for the session, I assume JISC is doing some session based analysis. According to their example, it's just a string (i.e. 32456891). In the fork they're calling that sesskey function to get that value.

garemoko commented 5 years ago

Guess I could have just Googled it: https://github.com/moodle/moodle/blob/master/lib/sessionlib.php#L28-L49

Ok, so yes it all looks like potentially useful information. On that basis, I might prefer to just split the ip address off as a separate setting and include the short name and session key all the time.

garemoko commented 5 years ago

Worth noting that there are only 10 billion (10,000,000,000) different Moodle session keys. I'm not sure what the probability of collision is once you get into large data sets, but I don't think we can do much about that in this plugin anyway.

ryasmi commented 5 years ago

Hmm okay yeah would be interested in having the session key and short name all the time if you're both happy with that. Yeah not sure on the session keys, I guess that's a Moodle problem rather than a problem for us to get too involved with.

garemoko commented 5 years ago

Just out of curiosity, apparently there's a 50% chance of collision after only 37 thousand session ids. See https://stats.stackexchange.com/questions/391140/in-a-set-of-10-billion-possible-items-if-i-have-x-items-what-is-the-probability?noredirect=1#comment735115_391140

davidpesce commented 5 years ago

The sessions aren't stored long term, they are periodically (I'm not sure on time) deleted.

ryasmi commented 5 years ago

I'm not sure the deletion in Moodle makes any difference to the problem because they will remain in the LRS and potentially cause issues in analysis.

davidpesce commented 5 years ago

Ahh, good point. I thought for some reason they were looking for a way to go back and cross-reference them. I'd raise it with JISC and see what they say. You can always add something like the date to the session ID to avoid the potential collisions.

ryasmi commented 5 years ago

To be fair they might just be trying to cross reference them, I'm making an assumption here too. Yeah adding the date might be ok to avoid collisions. Either way, I'll discuss with JISC.

garemoko commented 5 years ago

Using date might be problematic for sessions with span midnight. The same problem exists if only using month or year in the case of a session spanning midnight on new year's eve

ryasmi commented 5 years ago

Yeah that's very true.

ryasmi commented 5 years ago

I guess the IP and session ID could be combined, unlikely that the session ID would collide for the same person. The session ID might exist across IPs though if the IP changes over time for a machine.

ryasmi commented 5 years ago

Note to self: We already have an option for the short course ID send_short_course_id. It adds an extension with the key https://w3id.org/learning-analytics/learning-management-system/short-id. However, this is added in the definition.extensions for the course activity rather than the context extensions, which actually makes more sense, but might be harder to query. Jisc may also struggle to switch to a new key.

HT2Bot commented 5 years ago

:tada: This issue has been resolved in version 4.3.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: