Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

fix(metrics): send events from /metrics-flow to amplitude #6517

Merged
merged 1 commit into from Sep 6, 2018

Conversation

philbooth
Copy link
Contributor

Fixes mozilla/fxa-amplitude-send#74.

There were two problems preventing these events from showing up in Amplitude. Firstly, as pointed out by @irrationalagent, the amplitude view events are keyed off the raw screen... content server events rather than the flow.xxx.view events. But also, more fundamentally, the route handler wasn't actually sending any events to Amplitude.

This PR addresses both issues.

@mozilla/fxa-devs r?

@philbooth philbooth self-assigned this Sep 5, 2018
@philbooth philbooth requested a review from a team September 5, 2018 12:37
@@ -4,14 +4,16 @@

'use strict';

const amplitude = require('../amplitude');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

const flowMetrics = require('../flow-metrics');
const logFlowEvent = require('../flow-event').logFlowEvent;
const logger = require('../logging/log')('server.get-metrics-flow');

module.exports = function (config) {
const FLOW_ID_KEY = config.get('flow_id_key');
const FLOW_EVENT_NAME = 'flow.begin';
const FLOW_ENTER_EMAIL_EVENT_NAME = 'flow.enter-email.view';
const ENTER_EMAIL_SCREEN_EVENT_NAME = 'screen.enter-email.view';
const ENTER_EMAIL_FLOW_EVENT_NAME = 'flow.enter-email.view';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@vladikoff vladikoff merged commit 408f61d into master Sep 6, 2018
@ghost ghost removed the waffle:review label Sep 6, 2018
@vladikoff vladikoff deleted the pb/amplitude-74 branch September 6, 2018 02:24
@shane-tomlinson shane-tomlinson added this to the FxA-0: quality milestone Sep 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants