Report A4A experiment to CSI. #8958
Conversation
| this.ampexp_ = ''; | ||
|
|
||
| // Add RTV version as experiment ID, so we can slice the data by version. | ||
| this.addEnabledExperiment(getMode(this.win).rtvVersion); |
There was a problem hiding this comment.
So, if I'm reading the code right, the ampexp_ field (and, therefore, your ampexp CSI message) will look something like this:
'canary,experiment_foo,0012355768,experiment_bar'where '0012355768' is the AMP runtime version, right? My question is how you will recognize the runtime version field during analysis, later? As long as it's the only numeric field, you can find it with a regex or something. But I'm concerned that somebody may insert another numeric experiment name, which would make it hard to separate out the version string. Would it be reasonable to add a prefix or something to make it more identifiable? rtv_0012355768 or something?
There was a problem hiding this comment.
good suggestion. i can do it in a separate PR since this is already in prod, and it will change prod behavior.
test/functional/test-performance.js
Outdated
| perf.flush(); | ||
| expect(viewerSendMessageStub).to.be.calledWith('sendCsi', { | ||
| ampexp: getMode(win).rtvVersion, | ||
| ampexp: getMode(win).rtvVersion + ',experiment-a,experiment-b', |
There was a problem hiding this comment.
Depends on the order in which Object.keys() returns the keys, I think. Is that guaranteed in JavaScript? In most languages it's not for a Map.
There was a problem hiding this comment.
Good point. I improved the asserts. PTAL
* Report A4A experiment to CSI. * Address comments, improve test asserts.
* Report A4A experiment to CSI. * Address comments, improve test asserts.
experiments.jsa bit to exposeaddEnabledExperimentmethodaddEnabledExperimentmethodlegacy-cdn-domainexperiment