mozilla / ping-centre

INACTIVE - http://mzl.la/ghe-archive
Mozilla Public License 2.0
8 stars 10 forks source link

Making "payload" and "variants" work with redshift for TestPilotTest #34

Closed ncloudioj closed 7 years ago

ncloudioj commented 7 years ago

In the testpilottest.js, both "payload" and "variants" are defined as object fields in the schema. Unfortunately, Redshift doesn't support the nested object in the schema. To work around this limitation, we could consider using following options:

ncloudioj commented 7 years ago

FYI @6a68 @emtwo

emtwo commented 7 years ago

I do think UT is using JSON fields in general, but I'm not sure if in particular they're using it for payload and variants. I would still guess that the JSON fields are harder/slower to query and less ideal. A flat schema with an experiment_id type field would be my preference.

ncloudioj commented 7 years ago

After messing around the JSON support in Redshift, here is what I've found so far.

@emtwo So your guess is correct :). However, I think we could consider using JSON field in the following cases:

jaredhirsch commented 7 years ago

@ncloudioj @emtwo How about this alternative approach to the testpilotttest schema and associated Test Pilot experiment storage in ping centre Redshift:

Under this proposal, experiment author steps for Ping Centre integration would be:

  1. file a bug here (or against the onyx repo?) that includes the Redshift schema and their experiment addon ID
  2. integrate the testpilot-metrics repo in their addon

Does this seem reasonable?

emtwo commented 7 years ago

This seems reasonable to me. CC @tspurway to comment on the ETL portion. (though I believe he will be away until Wednesday)

jaredhirsch commented 7 years ago

Minor update to my last comment: it turns out we don't need a separate testpilottest schema, after making some changes to the testpilot schema in PR #36. The ETL step could split all Test Pilot pings on the addon_id field, both from the Test Pilot addon and the experiments.

ncloudioj commented 7 years ago

Closing this issue as @6a68 and @emtwo have sorted out the above game plan.

Thank you guys! Let's do this.