internetarchive / wayback-machine-firefox

Reduce annoying 404 pages by automatically checking for an archived copy in the Wayback Machine. Learn more about this Test Pilot experiment at https://testpilot.firefox.com/
GNU Affero General Public License v3.0
53 stars 17 forks source link

Add Telemetry support? #9

Open pdehaan opened 8 years ago

pdehaan commented 8 years ago

(Before I lose this link)

Currently looks like we'd be blocked by https://github.com/mozilla/testpilot/issues/433

lmorchard commented 8 years ago

We're probably not going to have Telemetry support for WebExtensions add-ons in the very near future. Next best thing will be to send HTTP POST requests to a metrics ping resource on the Test Pilot server, once we get that pull request merged & deployed on the Test Pilot side.

ghost commented 7 years ago

Thanks to Les, there is an example of how to implement the metrics pings from a webextension at https://github.com/mozilla/testpilot/tree/master/docs/examples/webextension

I've also asked for a review of the metrics proposal to get some more eyes on it.

rchrd2 commented 7 years ago

Hi @wresuolc The above example has the telemetry in background.js. Do you know if these calls will also work on the webpage itself via the inject client.js file? 3 of the 4 calls need to be done in the client.

Client:

Background:

lmorchard commented 7 years ago

Short answer is, no you cannot make the telemetry calls directly from an injected script. Only your background.js can send those pings over the BroadcastChannel to the main Test Pilot add-on. So, you need to somehow have cilent.js communicate with background.js and have background.js send the pings. I'm not sure off the top of my head about how to do that, so will have to do some research.

bwinton commented 7 years ago

It looks like https://developer.chrome.com/extensions/messaging describes how to communicate from client.js to background.js. (In particular, the client calls chrome.runtime.sendMessage, and the background calls chrome.runtime.onMessage.addListener.)

lmorchard commented 7 years ago

Maybe something like this helps?

https://developer.chrome.com/extensions/messaging#external-webpage

rchrd2 commented 7 years ago

Thanks for the tips. I'll try this messaging approach.

rchrd2 commented 7 years ago

Hello. This approach is working. There's another question. When a user clicks the <a> tag to view the archive page, I have setup a hander to send the telemetry like this:

          createEl("a", function(el) {
            el.onclick = function(e) {
              sendTelemetry("viewed");
            };
         }

This ultimately makes it's way to background.js which sends the metrics using testpilotPingChannel.postMessage.

Do you konw if this process is synchronous and will always sucessfully send the data before the web page changes? Or do I need to handle this asynchronously and use a complete handler with the telemetry to make sure it goes through before the page changes?

I worked on a solution like this, but wanted to check, because maybe it's not necessary:

            el.onclick = function(e) {
              // We want to send telemetry that it was clicked and wait for that
              // to go through. But if it takes too long, go to the URL anyway.
              e.preventDefault();
              archiveLinkWasClicked = true;
              var telemetryTimeout = setTimeout(function() {
                window.location.href = wayback_url;
              }, 3000);
              sendTelemetry("viewed", function() {
                clearTimeout(telemetryTimeout);
                window.location.href = wayback_url;
              });
              return false;
            };

You can view/download/try the code here: https://github.com/internetarchive/FirefoxNoMore404s/pull/25/files

Thanks.

rchrd2 commented 7 years ago

Also, here's a page you can test on: http://rchrd.net/using-polling-to-automatically-update-ember-models/

Edit: And to run the code, you simple have to run npm run dev after you run npm install

rchrd2 commented 7 years ago

Since I had not heard back, I went ahead and merged it. I will create a release. If the click isn't tracked, I can put a patch.

ghost commented 7 years ago

Thanks. I installed this last week and looked at my telemetry pings today (in about:telemetry) and didn't see any from this add-on, but I'm also not super familiar with that system (eg. if I didn't generate enough of them, maybe they are still in a queue). I asked our metrics folks to look for them on the server side and haven't heard back yet.

rchrd2 commented 7 years ago

I just made small change to the telemetry. I was worried this might be the issue: I moved the 'version' field out of the root and into the 'payload':

See the change here: https://github.com/internetarchive/FirefoxNoMore404s/commit/babbe50dbd0937d24f904c4d820315f0cbebcefe#diff-b1b4f182c9eab8c964d68b709e2e6a25R79