This repository has been archived by the owner. It is now read-only.
feat(metrics): emit a flow event for active experiments #4775
Conversation
|
Two things, the first a question (could metrics.logExperiment just call this.logFlowEvent), and the 2nd looks like a problem, I imagine the experiment should only be logged once. |
| @@ -36,7 +36,7 @@ define(function (require, exports, module) { | |||
| notifications: {}, | |||
|
|
|||
| /** | |||
| * The A/B testing group type, see https://en.wikipedia.org/wiki/A/B_testing for details. | |||
| * The A/B testing group type, usually either `'control'` or `'treatment'`. | |||
shane-tomlinson
Mar 2, 2017
Member
💯
app/scripts/lib/experiment.js
Outdated
| @@ -151,6 +151,9 @@ define(function (require, exports, module) { | |||
| if (initResult) { | |||
| this._activeExperiments[experimentName] = experiment; | |||
| this.metrics.logExperiment(experimentName, experiment._groupType); | |||
| this.notifier.trigger('flow.event', { | |||
shane-tomlinson
Mar 2, 2017
•
Member
Could metrics.logExperiment just invoke it's own logFlowEvent?
In addition, do you only want to log this once? If I keep switching between signin and signup, multiple experiment events are logged:
Could metrics.logExperiment just invoke it's own logFlowEvent?
In addition, do you only want to log this once? If I keep switching between signin and signup, multiple experiment events are logged:
philbooth
Mar 2, 2017
Author
Contributor
Oh crikey, didn't notice that, thanks. Will fix.
Oh crikey, didn't notice that, thanks. Will fix.
|
I give an ex post facto r+ |
Did this separate issue get filed? |
Good point, sorry, this had dropped off my radar temporarily. Opened in mozilla/fxa-activity-metrics#61. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Fixes #4290.
There is some additional monkeying around we can do in the import scripts, to make the experiment data available as metadata rather than a standalone event. But that can happen in a separate issue.
@mozilla/fxa-devs r?