645860 - Studies should have a way to write metadata

Status: RESOLVED FIXED

(Mozilla Labs Graveyard :: Test Pilot, defect, P1)

Description

[Jono Xia]

This came up in a discussion about the garbage collection study; the study would like to be able to record metadata about the user's hardware.  This is essentially study-specific metadata.  Up until now we've done stuff like this by writing the value into one of the columns of the schema as a special 'status' event, but that's a really ugly kluge (it violates the schema and makes it hard to filter on when querying the database).

There's currently no way for a study to write to metadata because the metadata object doesn't exist until submission time.  We can implement this either by creating temp metadata storage earlier in the process (a database table or something) and providing an API to write to it; or else (and this is easier) having the task object query the study to say "any more metadata for me?" when collecting all the other metadata right before submission.  So a study can optionally implement a provideMetadata() method.

Comment 1

[Jono Xia]

Implemented in http://hg.mozilla.org/labs/testpilot/rev/7dd6788c1500

I tried to write it in as error-tolerant a way as possible (it will do the right thing if a study's getStudyMetadata() method throws an exception or if it returns something that's not an array) but we're not currently sanitizing name or value strings; should think about whether doing so is needed for security reasons.

Comment 2

[Jono Xia]

Also how to handle the case where it returns an invalid object or where name or value is not a string.

Comment 3

[Jono Xia]

Attachment: Patch to mozilla-central to add this feature

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 524919 [details] [diff] [review]
Patch to mozilla-central to add this feature

Should TestPilotExperiment_getStudyMetadata use a try/catch in case the task method throws an exception?

Comment 5

[Jono Xia]

Attachment: Better patch, with exception handling

Yes it should.  Here's a patch with exception handling.