Project

General

Profile

Feature #14922

Integrate download metrics in the new download page

Added by sajolida about 1 year ago. Updated 4 days ago.

Status:
In Progress
Priority:
Normal
Assignee:
Category:
Installation
Target version:
Start date:
11/04/2017
Due date:
% Done:

0%

Estimated time:
2.00 h
QA Check:
Pass
Feature Branch:
web/14922-download-metrics
Type of work:
Code
Blueprint:
Starter:
Affected tool:

successful-is-working.png View (113 KB) sajolida, 09/09/2018 01:02 PM

download-iso-is-not-working.png View (128 KB) sajolida, 09/09/2018 01:02 PM

0002-Ensure-that-failing-to-record-a-hit-does-not-cause-o.patch View (1.61 KB) lamby, 10/24/2018 09:52 PM

0001-Ensure-that-we-call-our-hit-counter-before-following.patch View (2.5 KB) lamby, 10/24/2018 09:52 PM

NetworkError on download and no refresh after installing extension.webm (1.07 MB) sajolida, 10/27/2018 03:49 PM

0002-Cachebust-hits-to-the-counter.-re.-14922.patch View (1.36 KB) lamby, 10/28/2018 10:58 PM

0001-Add-a-dummy-counter-file-to-ensure-that-regular-oper.patch View (687 Bytes) lamby, 10/28/2018 10:58 PM

0001-Use-relative-URI-for-download-counter.-re.-14922.patch View (1.13 KB) lamby, 10/29/2018 01:55 AM

NetworkError on download button.webm (501 KB) sajolida, 10/29/2018 02:16 PM

0002-Avoid-a-traceback-if-mirror-dispatcher.js-is-not-ava.patch View (929 Bytes) lamby, 10/29/2018 02:48 PM

0001-Use-window.open-over-setting-window.location-to-avoi.patch View (1.1 KB) lamby, 10/29/2018 02:48 PM


Related issues

Related to Tails - Bug #12127: Make "notes" more structured in the description of our mirror pool Confirmed 01/10/2017
Blocks Tails - Feature #16080: Core work 2018Q4 → 2019Q2: User experience Confirmed 10/29/2018

History

#1 Updated by sajolida 12 months ago

  • Parent task deleted (#12328)

I won't have time to do this on the budget for #12328.

#2 Updated by u 11 months ago

@sajolida: could you explain what you want to do on this ticket please? thanks!

#3 Updated by sajolida 11 months ago

Sure. With the new structure of the download page, I thought we could maybe hit a counter using JavaScript when an ISO image is downloaded or verified. That would give us a quite good download metric.

#4 Updated by sajolida 11 months ago

  • Related to Bug #12127: Make "notes" more structured in the description of our mirror pool added

#5 Updated by sajolida 10 months ago

  • Target version set to Tails_3.9

I want to have some data before 3.11, so let's target 3.9.

#6 Updated by sajolida 5 months ago

  • Target version changed from Tails_3.9 to Tails_3.10.1

#7 Updated by sajolida 4 months ago

  • Assignee changed from sajolida to intrigeri
  • QA Check set to Ready for QA
  • Feature Branch set to web/14922-download-metrics

Here is a branch that does that. I tested it on a counter for which I could see the logs of the web server.

I managed hit the counter on the result of the verification.

I didn't manage to hit it when the download button is clicked (ISO or BitTorrent).

My understanding is that it's because when the download button is clicked the browser opens the link that points to the download and JavaScript on the page is interrupted at that time. Someone with a better understanding of the internals of JavaScript in browsers might be able to fix that. Until then, people doing the OpenPGP verification only or skipping the verification won't be counted.

I'm also passing along:

  • The scenario, so we know the base OS of people downloading Tails
  • The version, if that becomes interesting for some reason

intrigeri: Do you mind helping me identify who would be a suitable worker to review this (u doesn't want to do that kind of stuff anymore)? They will be paid on my UX budget. See fundraising.git:drafts/ISC/1/unofficial.mdwn.

#8 Updated by intrigeri 4 months ago

  • Assignee changed from intrigeri to sajolida

I didn't manage to hit it when the download button is clicked (ISO or BitTorrent). My understanding is that it's because when the download button is clicked the browser opens the link that points to the download and JavaScript on the page is interrupted at that time.

Indeed, to fix that we would need to open the download URL via JS too, after hitting the counter. Should not be too hard except once you add real world constraints such as our use-mirror-pool handling and graceful fallback when JS is disabled.

intrigeri: Do you mind helping me identify who would be a suitable worker to review this

Happy to help with that!

  • Short-term: I see nobody else than me with all the info in mind to review this so I'll do it as part of reviewing code contributions that are on nobody else's plate
  • Long-term: as we've noticed when doing the budget forecasting last year, you need a team-mate for this sort of things. I think Cody is the person that works closest to this area (website, IA) so I would say best is to somehow get him up-to-speed so he can do this kind of reviews.

So in this case, I propose you get a first review by Cody (so we make progress on solving the root cause of the problem) and then I'll take a last quick look before merging (let's call it a safety net). Works for you?

#9 Updated by sajolida 3 months ago

ignifugo offered me to help with that. She doesn't have an account so for the time being I'll only add more info on how to test this but I can't reassign it to her:

1. Build a local build of the website on my branch (web/14922-download-metrics). See https://tails.boum.org/contribute/build/website/.
2. Change counter_url in wiki/src/install/inc/js/download.js to an URL for which you can see the server's log.
3. Uncomment showVerificationResult("successful"); at the bottom of wiki/src/install/inc/js/download.js.
4. Visit /install/download on your local build.

  • You should see a hit on the counter_url with status=successful.
  • But if you click on the "Download Tails" button, you don't get a hit with status=download-iso.

See also the screenshots in attachment.

#10 Updated by sajolida about 2 months ago

  • Target version changed from Tails_3.10.1 to Tails_3.11

No news from ignifugo since the summit despite 2 pings, I'm asking Chris if he's interested.

#11 Updated by lamby about 2 months ago

I'm asking Chris if he's interested.

I'm happy to review this as well as try and get the "normal" download to work as you write in note-7 above. Please assign back to me if you'd like me to go ahead, including the maximum number of hours I should exert on this (if I need more, I'll get back to you). Thanks!

#12 Updated by sajolida about 2 months ago

  • Assignee changed from sajolida to lamby

Excellent, thanks for taking this one!

I spent myself 2.5 hours on the code (I'm slow and have a very poor training at JS). So I guess that 1 hour of review should be enough and maybe another hour to try to fix the normal download. Total 2 hours max.

#13 Updated by lamby about 2 months ago

ACK, on my TODO.

#14 Updated by lamby about 2 months ago

Please find the following attached patches:

0001-Ensure-that-we-call-our-hit-counter-before-following.patch fixes the issue where the hit counter was not being recorded as outlined in c9d24f225310073a95e28da773b4c051ef033ff5 or in #note-7.

0002-Ensure-that-failing-to-record-a-hit-does-not-cause-o.patch improves the robustness of the hit counter, ensuring that if we fail to record the hit (eg. if viewing the documentation locally and one's content policy prevents the "ping" hitting an external server, or it fails to parse the url etc. etc.) then no "important" code fails to execute.

#15 Updated by sajolida about 2 months ago

  • QA Check changed from Ready for QA to Pass

Excellent! I applied your patches and merged. Exception handling is still out of my skillset, so thanks for spotting that!

lamby: How much time did you spend on this?

I'm leaving this ticket open to remind me to compute some first stats in some weeks to make sure this works as intended in production.

#16 Updated by sajolida about 2 months ago

Oops, actually, this doesn't work on the production website. See screencast in attachment.

When click the download button I get a NetworkError instead of hiting the counter. I don't get this error when I click the "I already downloaded Tails" link.

I reverted the whole branch because I initially thought that it also triggered #16078 but it's not the case. So if you need it for debugging, I could merged again the branch.

I also added 4563af94cf on top of your commits since the console is disabled elsewhere. But I think it's unrelated to this new issue...

#17 Updated by lamby about 2 months ago

What exactly do you think is the issue here? :) We get a NetworkError 404 as the https://tails.boum.org/install/download/counter location does not actually exist; this was the expected behaviour as I understood it from your comment in note-7. Marking as Info Neeeded to match.

However, I can see that that is not ideal and a little misleading, therefore please find 0001-Add-a-dummy-counter-file-to-ensure-that-regular-oper.patch attached which adds such a file.

(In addition, we should probably cachebust these HTTP GET requests. For this, please see 0002-Cachebust-hits-to-the-counter.-re.-14922.patch.)

lamby: How much time did you spend on this?

2 hours total.

#18 Updated by lamby about 2 months ago

Please also find the following 0001-Use-relative-URI-for-download-counter.-re.-14922.patch attached:

From 99012ca495e5ab2e74c79488baf6adf35b429915 Mon Sep 17 00:00:00 2001
From: Chris Lamb <chris@chris-lamb.co.uk>
Date: Sun, 28 Oct 2018 21:54:17 -0400
Subject: [PATCH] Use relative URI for download counter. (re. #14922)

This will let it work over Tor (etc. as well as to ease local development)
due to cross-site request prevention / CORS, etc. etc.
---
 wiki/src/install/inc/js/download.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

#19 Updated by sajolida about 2 months ago

What exactly do you think is the issue here? :) We get a NetworkError 404 as the https://tails.boum.org/install/download/counter location does not actually exist; this was the expected behaviour as I understood it from your comment in note-7. Marking as Info Neeeded to match.

I thought about that as well but I discarded this hyopthesis because I
was not getting the same NetworkError when clicking on the "I already
downloaded" link. Still, #16078 is unrelated and the NetworkError
doesn't prevent download, I applied your new patcher.

On the production website, I still have a NetworkError (and no GET in
the network activity) when clicking the download button, while I have a
GET and no NetworkError when clicking the "I already downloaded" link.

See screencast in attachment.

Since I don't have real-time access to the server log, I can't check
right-now if the GET is actually performed when clicking the download
button. I guess it's not but I could download the logs tomorrow if
that's helpful.

#20 Updated by sajolida about 2 months ago

#21 Updated by lamby about 2 months ago

Ahhh it's a nice race condition that doesn't occur locally for me due to network, etc. This should be fixed in 0001-Use-window.open-over-setting-window.location-to-avoi.patch

Please also find 0002-Avoid-a-traceback-if-mirror-dispatcher.js-is-not-ava.patch which (unrelated) prevents an ugly error when viewing the site locally due to the lack of mirror-dispatcher.js.

#22 Updated by sajolida about 2 months ago

  • Status changed from Confirmed to Resolved
  • QA Check changed from Ready for QA to Pass

Excellent! It works now :)

I'll wait to apply 0002-Avoid-a-traceback-if-mirror-dispatcher.js-is-not-ava.patch until we have to modify page/template.tmpl for something else since it will trigger a full rebuild of the website on the server.

Marking this ticket as Pass but not closing it so I remember to computer some first stats in some weeks to make sure that the result is fine.

#23 Updated by lamby about 2 months ago

Excellent! It works now :)

Silly race conditions... grr.

Marking this ticket as Pass but not closing it so I […]

ACK.

#24 Updated by sajolida about 2 months ago

  • Status changed from Resolved to In Progress

#25 Updated by sajolida 15 days ago

  • Blocks Feature #16080: Core work 2018Q4 → 2019Q2: User experience added

#26 Updated by sajolida 4 days ago

  • Target version changed from Tails_3.11 to Tails_3.12

Also available in: Atom PDF