This repository has been archived by the owner. It is now read-only.
feat(CAD): Add flow metrics for connect another device. #4787
Conversation
I'm not totally happy with this solution, there's a lot of duplication between this and the experiment metrics, it seems like we should be able to unify. That's for another day. fixes #4783
| }); | ||
|
|
||
| it('shows a sign in button with the appropriate link, logs appropriately', () => { | ||
| assert.lengthOf(view.$('#signin'), 1); | ||
| testIsNotified('connectAnotherDevice.signedin.false'); | ||
| testIsNotified('connectAnotherDevice.signin.eligible'); | ||
| testIsNotified('connectAnotherDevice.signin_from.fx_desktop'); | ||
| testIsFlowEventLogged('signedin.false'); | ||
| testIsFlowEventLogged('signin.eligible'); | ||
| testIsFlowEventLogged('signin_from.fx_desktop'); |
vladikoff
Mar 7, 2017
Contributor
Do we need to update the flow even docs to include these new events?
Do we need to update the flow even docs to include these new events?
philbooth
Mar 7, 2017
Contributor
Those event docs are next in the line of fire on my auto-generate-all-the-docs mission.
Those event docs are next in the line of fire on my auto-generate-all-the-docs mission.
| @@ -64,7 +64,7 @@ | |||
| {{/isFirefoxIos}} | |||
| {{/canSignIn}} | |||
| <div class="links"> | |||
| <a href="/connect_another_device/why">{{#t}}Why is this required?{{/t}}</a> | |||
| <a data-flow-event="link.why" href="/connect_another_device/why">{{#t}}Why is this required?{{/t}}</a> | |||
vladikoff
Mar 7, 2017
Contributor
I wonder if are overloading the flow event pipeline. I think before we were talking about how flowEvents are only a selected number of events. If we keep adding more and more then queries will be even slower. thoughts on this @philbooth ?
I wonder if are overloading the flow event pipeline. I think before we were talking about how flowEvents are only a selected number of events. If we keep adding more and more then queries will be even slower. thoughts on this @philbooth ?
philbooth
Mar 7, 2017
Contributor
But it's THE CLOUD! 😄
I see your point but I think we're good. At some point soon, we'll implement automated expiry for the flow data like we have for the activity events and that should address most of the performance concerns. As long as they're useful events, the more the merrier.
But it's THE CLOUD!
I see your point but I think we're good. At some point soon, we'll implement automated expiry for the flow data like we have for the activity events and that should address most of the performance concerns. As long as they're useful events, the more the merrier.
vladikoff
Mar 7, 2017
Contributor
Ok! sounds good
Ok! sounds good
|
@shane-tomlinson, this looks good to me! r+ |
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.
I'm not totally happy with this solution, there's a lot of duplication
between this and the experiment metrics, it seems like we should
be able to unify. That's for another day.
fixes #4783
@philbooth - r?