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

fix(metrics): include full version information in event data #2356

Merged
merged 3 commits into from Mar 22, 2018

Conversation

philbooth
Copy link
Contributor

@philbooth philbooth commented Mar 21, 2018

Fixes mozilla/fxa-amplitude-send#58.

The user-agent parser was originally written with synthesized device names in mind, so it didn't always return the full version string for browsers and operating systems. Since then, we started using the same code for our event data, meaning that we're getting an incomplete picture of the browser/os that our users are on.

This change tweaks the user-agent code so that it returns full version info, and tweaks the code for synthesizing device names so that it remains consistent(ish) with its old behaviour.

@mozilla/fxa-devs r?

@ghost ghost assigned philbooth Mar 21, 2018
@ghost ghost added the waffle:active label Mar 21, 2018
Fixes mozilla/fxa-amplitude-send#58.

The user-agent parser was originally written with synthesized device
names in mind, so it didn't always return the full version string for
browsers and operating systems. Since then, we started using the same
code for our event data, meaning that we're getting an incomplete
picture of the browser/os that our users are on.

This change tweaks the user-agent code so that it returns full version
info, and tweaks the code for synthesizing device names so that it
remains consistent with its old behaviour.
Copy link
Contributor

@vbudhram vbudhram left a comment

Choose a reason for hiding this comment

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

@philbooth LGTM 👍, will there be any charts/dashboards that will need updating after this change?

@philbooth
Copy link
Contributor Author

will there be any charts/dashboards that will need updating after this change?

Lol, probably although I can't think of any specific cases off the top of my head. We can fix them as we find them...

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

2 participants