Skip to content
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

Send media downloads to analytics #6063

Merged
merged 2 commits into from Aug 16, 2019

Conversation

@davidfischer
Copy link
Contributor

@davidfischer davidfischer commented Aug 9, 2019

While data will be sampled, this will give us an idea of how frequently media (zips, PDFs, ePubs) are downloaded.

@davidfischer davidfischer requested a review from Aug 9, 2019
Copy link
Member

@ericholscher ericholscher left a comment

💯 Great start to at least getting the data.

Loading

event_action=f'Download {type_}',
event_label=str(version),
ua=request.META.get('HTTP_USER_AGENT'),
uip=get_client_ip(request),
Copy link
Member

@ericholscher ericholscher Aug 9, 2019

Choose a reason for hiding this comment

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

Do we want/need to send this?

Loading

Copy link
Contributor Author

@davidfischer davidfischer Aug 9, 2019

Choose a reason for hiding this comment

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

If we don't care about it, we could definitely drop it. It is anonymized but it also isn't really useful. I'd be fine dropping it.

Loading

Copy link
Member

@ericholscher ericholscher Aug 9, 2019

Choose a reason for hiding this comment

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

Up to you. I'm 👍 on dropping it if we don't need it though.

Loading

Copy link
Contributor Author

@davidfischer davidfischer Aug 10, 2019

Choose a reason for hiding this comment

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

I'll update. Let's drop it. Seems cleaner that way especially since we don't need it. I'll just verify that GA doesn't silently drop requests without them.

Loading

Copy link
Contributor Author

@davidfischer davidfischer Aug 12, 2019

Choose a reason for hiding this comment

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

In my testing, without sending something for the user agent and IP address, the event doesn't show up in the realtime reporting. I'll give it a day to see if it does show up in the aggregated reporting but we may need to send some value for those settings. That could be the server's IP address.

Loading

Copy link
Member

@ericholscher ericholscher Aug 12, 2019

Choose a reason for hiding this comment

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

Alright, let's just ship it and let GA anonymize it, we're already doing it elsewhere.

Loading

Copy link
Contributor Author

@davidfischer davidfischer Aug 13, 2019

Choose a reason for hiding this comment

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

After looking at GA's regular reporting, only the events with an IP and UA were counted.

Loading

@ericholscher ericholscher merged commit 9f92834 into master Aug 16, 2019
3 checks passed
Loading
@ericholscher ericholscher deleted the davidfischer/media-download-analytics branch Aug 16, 2019
@ericholscher
Copy link
Member

@ericholscher ericholscher commented Aug 16, 2019

👍

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants