-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Segment integration with pageview tracking #367
Conversation
Pull Request Test Coverage Report for Build 662851225
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sentry doesn't support domain filters, so there was nothing to update there, but I did verify that events from this application were being received.
Sentry does support environment tags so that you can view errors that come from development, staging, and production environments separately. We have implemented that in pulse, but it required removing Sentry from the Segment integration and implementing it directly. I really like the additional detail that we're getting from the errors now that we have more control over them, but it is more code...
yeah I was trying to get that work but eventually stopped short of removing it from Segment and just left it as is instead. I think maybe we'd be better off configuring separate Segment "sources" for our different environments (that seems to be what Segment wants you to do, and we are arguably jumping through additional hoops as a result of not doing that) but I wasn't really prepared to mess with that configuration as part of the scope of this ticket so I just left it alone. Spotlight doesn't generate a lot of errors anyway and virtually all of them are transient fetch failures about which we do nothing so I'm not really that worried about it for now? |
Description of the change
Includes the Segment snippet and integrates pageview tracking with our routing logic (which includes updating the page titles to be more meaningful). Because of our in-page navigation features this couldn't be quite as simple as "log a pageview whenever the URL changes" but I don't think the end result is too unreasonable (and it is tested).
Not visible in code but also done as part of this task was updating some inbound domain filters in Google Analytics to keep staging data out of the metrics, and cleaning up some GA projects that were auto-generated with the various Spotlight Firebase projects but weren't actually being used. (Sentry doesn't support domain filters, so there was nothing to update there, but I did verify that events from this application were being received.)
Type of change
Related issues
Checklists
Development
These boxes should be checked by the submitter prior to merging:
Code review
These boxes should be checked by reviewers prior to merging: