doc: add Google Analytics tracking. #6601

phillipj · 2016-05-05T18:13:42Z

Checklist

tests and code linting passes
a test and/or benchmark is included
documentation is changed or added
the commit message follows commit guidelines

Affected core subsystem(s)

tools, doc

Description of change

Original changes has been discarded after discussions below.

This adds Google Analytics tracking to the bottom of all doc pages if ~~we're building for Release~~ make is run with DOCS_ANALYTICS variable present.

Any thoughts about this so far? Would you consider using build type like this sufficient to avoid tracking locally or other scenarios when tracking isn't wanted?

Refs nodejs/nodejs.org#709

/cc @nodejs/documentation

phillipj · 2016-05-05T18:17:02Z

tools/doc/html.js

@@ -107,6 +120,10 @@ function render(lexed, filename, template, cb) {
  });
 }

+function gaScriptContent() {
+  return fs.readFileSync(gaScriptPath).toString();


Allowed my self to use Sync here as this is a build script, where doing async doesn't do too much for performance, other than increasing code complexity.

Fishrock123 · 2016-05-05T18:27:43Z

The build is release by default... I don't really want this in local docs tbh

phillipj · 2016-05-05T19:14:36Z

@Fishrock123 yeah, although this provides a way to opt-out of tracking, it's not ideal as Release is the default...

In a perfect world the tracking script would only be included when actually cutting new releases. I don't have access to the release build jobs so I haven't been able to see what exactly is run when building new versions - is there anything unique about those builds we could pick up automatically?

jasnell · 2016-05-06T17:07:03Z

A flag could be added to configure to selectively enable this and that flag could be switched on in the release CI.

Fishrock123 · 2016-05-06T17:41:14Z

I'd like thoughts from @nodejs/build

rvagg · 2016-05-10T01:46:25Z

Here's my thoughts on where this needs to go:

we use the doc-upload target for building docs for nodejs.org, we're going to need to rejig that a bit because it depends on tar to get the docs to build so I suggest we make it depend on docs and have it do what tar does on top of that
use a make variable rather than build type to indicate this, so it's only ever done by CI and people who have a clue about how to turn it on. I suggest DOCS_ANALYTICS and maybe pass in the GA # via that but leave it blank by default so that when the build script get the arg (--analytics?) and it's empty it simply leaves it out.
the rest of the GA stuff can be included in the script, hard-wired, no need to pull it in from a separate file IMO, we can leave the code configurable and that can come via Jenkins, the rest can be hard-wired and appear if a code is supplied.
we have to make sure that the GA stuff doesn't appear inside the docs in the tarball while we're doing all of this

phillipj · 2016-05-10T19:36:07Z

Thanks @rvagg, a lot of good tips I wasn't aware of there. I'll look into using a make variable instead.

phillipj · 2016-05-16T22:02:15Z

@rvagg PTAL now.

Couldn't see any reason doc-upload should depend on tar, so I simply changed it to be dependant on doc directly.

jbergstroem · 2016-05-16T23:45:55Z

doc/template.html

@@ -45,5 +45,6 @@
  <script src="assets/sh_main.js"></script>
  <script src="assets/sh_javascript.min.js"></script>
  <script>highlight(undefined, undefined, 'pre');</script>
+  __TRACKING__


can we perhaps comment this?  -- that way we don't have to process any files if tracking is disabled.

@jbergstroem it gets replaced w/empty string when tracking is disabled, which is default, so it shouldn't be needed to enclose in comments?

yeah was just suggesting we could avoid it altogether (replacing with empty) by making it a html comment.

Following up on this. Why waste cpu cycles if we don't have to?

@jbergstroem sorry this slipped. I just pushed an update using a comment as you've suggested, PTAL.

ChALkeR · 2016-05-18T20:19:16Z

Personally, I don't like the idea of including GA to the docs.
My opinion here is that if we really need to track how users use the docs, it should be done with a solution that is:

Self-hosted.
Respects DNT setting.

Piwik looks as a possible solution to me (judging only by description, I have not tested it).
Also I would prefer all the depersonalized data to be either publically accessible, or accessible to everyone on the org.

I don't want to block GA though if others agree that we need it, so that's a -0. /cc @nodejs/ctc

bnoordhuis · 2016-05-19T09:40:39Z

I second what @ChALkeR said. The motivation in nodejs/nodejs.org#709 is knowing what parts of the documentation are visited more. It should be fairly straightforward to pull that information from the server logs, right?

saghul · 2016-05-19T16:25:56Z

Agreed with @ChALkeR. I use Piwik and it gets the job done. In case it helps, here is a screenshot from my own blog's stats: https://www.dropbox.com/s/cj7vnjbzn70gp8n/Screenshot%202016-05-19%2017.24.32.png?dl=0

phillipj · 2016-05-25T16:24:35Z

@mikeal as you initially tried to add GA tracking, what's your thoughts on using Piwik instead?

eljefedelrodeodeljefe · 2016-05-25T16:54:53Z

I have some points.

GA can be anonymized to some extent.
having two tracking systems is not unusual
tracking on the server manually is not a road we want to do down.
No matter GA or Piwik, having the dataset open without any or loose constraints opens up alls sorts of legal problems I assume. Using email addresses of just some people, who then pass down aggregated data is likely better. Maybe we could also use APIs for that.
I haven't used Piwik yet, but a grew fond of GA in mobile development contexts, since it allows really nice screenflow analytics, which I could reproduce hardly with other tracking solutions out-of-the-box
Currently we use a lot of JavaScript on page load already and load every page separately(, right?)
I am refactoring the doctool ATM and would integrate that rather after it. Since I wanted to look at more elegant ways, maybe templating.

So I am rather for deferring this to some later point and rather for GA (and additional others.)

mikeal · 2016-05-25T20:33:10Z

We are using Google Analytics in several other places already and it would not be ideal to use something else for this as it makes things far more difficult. For instance, if we want to know the number of unique users across the website including the API docs this would make that impossible.

ChALkeR · 2016-05-27T04:37:24Z

@eljefedelrodeodeljefe

GA can be anonymized to some extent.

I don't think so, it will still store and use unanonymized data.

No matter GA or Piwik, having the dataset open without any or loose constraints opens up alls sorts of legal problems I assume.

Could you elaborate? I was speaking only about anonymized data only. I'm not sure how does sharing the data with a third party not open similar issues then.

Using email addresses of just some people, who then pass down aggregated data is likely better. Maybe we could also use APIs for that.

That might be significantly more complex.

@mikeal

We are using Google Analytics in several other places already

I expect docs to be significantly more visited than any other places, so for anyone who views third-party unanonymized tracking as being harmful/unacceptable, including GA to the docs will make the situation considerably worse. /cc @nodejs/inclusivity, btw.

and it would not be ideal to use something else for this as it makes things far more difficult. For instance, if we want to know the number of unique users across the website including the API docs this would make that impossible.

Perhaps we should to add self-hosted tracking everywhere where GA is currently used, then? It should not harm to use them together for a time, I suppose.

phillipj · 2016-05-30T20:31:16Z

Rebased on top of master. As it doesn't seem to be consensus about using GA for docs tracking, should this be scheduled for the next CTC-meeting?

/cc @nodejs/ctc

rvagg · 2016-05-31T05:04:27Z

It should be fairly straightforward to pull that information from the server logs, right?

No, CloudFlare serves the majority of our doc requests. Although, we do apparently have a path to getting full logs from CF (it's an Enterprise feature but we've discussed it with them already). Log processing is pretty far from optimal for what's being asked for here.

Those who are concerned about being tracked are probably already blocking GA, either explicitly or with blocking tools (I have two of them installed in my current browser!). I'm not a huge fan of giving more data to Google about the entire world but weighing that against the useful information that the docs WG could glean from this puts me well into the +1 for this.

Also, in case anyone's reading this and misinterprets: this is only enabling it for nodejs.org, not self-hosted or even in the tarball we ship.

Someone in @nodejs/build, maybe me, will need to confirm that the Makefile changes are going to be OK with our workflow before this can land.

ChALkeR · 2016-05-31T05:34:38Z

@rvagg

the useful information that the docs WG could glean from this

Have you considered self-hosted tracking? Why is GA viewed as the only option to achieve that here?

phillipj · 2017-01-25T20:59:29Z

Landed in 01b90ee

phillipj · 2017-01-28T12:12:17Z

FYI / later reference: while merging this, I read through the COLLABORATOR_GUIDE.md and wanted to try the trick mentioned there, getting the PR status "merged" rather than "closed" by running git push --force-with-lease origin master:ga-docs-tracking.

That didn't work as expected. (1) Status was shown as "Closed" and (2) the UI shows this PR has 0 file changes. Not getting the "merged" status doesn't matter, but 0 file changes are really misleading tho!

Adds Google Analytics tracking script to all doc pages when `DOCS_ANALYTICS` is set when running `make`: ```bash $ DOCS_ANALYTICS=<GOOGLE ANALYTICS ID> make ``` By default (when `DOCS_ANALYTICS` is not set), no tracking scripts are included. It respects "Do Not Track" settings end users might have in their browser. Also changes make target `doc-upload` from depending on the `$(TARBALL)` target, to only depend on `doc` directly. PR-URL: #6601 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

gibfahn · 2017-01-28T13:11:38Z

@phillipj did you push to nodejs/master before you pushed to phillipj/ga-docs-tracking? The PR only counts as merged if the head of your fork branch has the same SHA as the commit you push to master, so if you change the commit message then push to master, Github will say there are 0 changes, because only the commit messages (and hashes) have changed.

If you force push to your branch before updating node/master, that should work.

phillipj · 2017-01-28T15:34:23Z

Aaah thanks for elaborating, that explained what I did wrong. I pushed to master node/master first.

…

On Sat, 28 Jan 2017 at 14:12, Gibson Fahnestock ***@***.***> wrote: @phillipj <https://github.com/phillipj> did you push to nodejs/master before you pushed to phillipj/ga-docs-tracking? The PR only counts as merged if the head of your fork branch has the same SHA as the commit you push to master, so if you change the commit message then push to master, Github will say there are 0 changes, because only the commit messages (and hashes) have changed. If you force push to your branch before updating node/master, that should work. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#6601 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABLLExYSygPcJPMrlUcR2tMY8Jzq6GfOks5rWz6ugaJpZM4IYPrE> .

Adds Google Analytics tracking script to all doc pages when `DOCS_ANALYTICS` is set when running `make`: ```bash $ DOCS_ANALYTICS=<GOOGLE ANALYTICS ID> make ``` By default (when `DOCS_ANALYTICS` is not set), no tracking scripts are included. It respects "Do Not Track" settings end users might have in their browser. Also changes make target `doc-upload` from depending on the `$(TARBALL)` target, to only depend on `doc` directly. PR-URL: nodejs#6601 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

Adds Google Analytics tracking script to all doc pages when `DOCS_ANALYTICS` is set when running `make`: ```bash $ DOCS_ANALYTICS=<GOOGLE ANALYTICS ID> make ``` By default (when `DOCS_ANALYTICS` is not set), no tracking scripts are included. It respects "Do Not Track" settings end users might have in their browser. Also changes make target `doc-upload` from depending on the `$(TARBALL)` target, to only depend on `doc` directly. PR-URL: #6601 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

ChALkeR · 2017-07-05T21:25:19Z

@phillipj @mikeal @chrisdickinson

Several months have passed with this landed. What information were we able to collect and how exactly is can we use it to improve things?

phillipj · 2017-07-06T08:38:16Z

@ChALkeR thanks for following up 👍

Although this was merged a long time ago, it was activated a couple of weeks ago: nodejs/nodejs.org#1268 (comment). By checking the docs deployed to production, v8.1.3 docs looks like the first set of docs that has GA tracking enabled.

In other words I think it's a little early to answer your question about what the stats collected has provided of actual value yet.

Maybe @feross has some future plans?

ChALkeR · 2018-08-30T15:29:24Z

Any updates?

The Google Analytics tracking wasn't wholly uncontroversial and hasn't been used in practice. Remove it. Refs: nodejs#6601 Fixes: nodejs#22652

The Google Analytics tracking wasn't wholly uncontroversial and hasn't been used in practice. Remove it. PR-URL: #23083 Fixes: #22652 Refs: #6601 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

phillipj added the doc Issues and PRs related to the documentations. label May 5, 2016

phillipj reviewed May 5, 2016
View reviewed changes

addaleax added the tools Issues and PRs related to the tools directory. label May 5, 2016

phillipj force-pushed the ga-docs-tracking branch 3 times, most recently from 8044926 to dc3e6ca Compare May 16, 2016 21:58

phillipj force-pushed the ga-docs-tracking branch 2 times, most recently from 27e1a70 to 005cf5b Compare May 16, 2016 22:07

jbergstroem reviewed May 16, 2016
View reviewed changes

phillipj force-pushed the ga-docs-tracking branch 2 times, most recently from 356224b to 34bf996 Compare May 30, 2016 20:26

Fishrock123 added the ctc-agenda label May 30, 2016

phillipj force-pushed the ga-docs-tracking branch from 34bf996 to 0851ad1 Compare May 31, 2016 08:32

nodejs-github-bot added lts-watch-v6.x labels Jan 25, 2017

phillipj deleted the ga-docs-tracking branch January 25, 2017 20:59

phillipj mentioned this pull request Jan 28, 2017

Include $DOCS_ANALYTICS when building docs #11050

Closed

italoacasas mentioned this pull request Jan 29, 2017

v7.5.0 proposal #11062

Merged

MylesBorins mentioned this pull request Mar 9, 2017

v6.10.1 proposal #11759

Merged

MylesBorins mentioned this pull request Mar 9, 2017

v4.8.1 proposal #11760

Merged

MylesBorins removed lts-watch-v4.x labels Nov 14, 2017

ChALkeR mentioned this pull request Aug 30, 2018

doc: Remove 'dnt_helper.js' #22595

Closed

4 tasks

ChALkeR mentioned this pull request Sep 2, 2018

doc: is Google Analytics tracking actually useful? #22652

Closed

bnoordhuis added a commit to bnoordhuis/io.js that referenced this pull request Sep 25, 2018

doc: remove GA tracking

4e652d0

The Google Analytics tracking wasn't wholly uncontroversial and hasn't been used in practice. Remove it. Refs: nodejs#6601 Fixes: nodejs#22652

bnoordhuis mentioned this pull request Sep 25, 2018

doc: remove GA tracking #23083

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: add Google Analytics tracking. #6601

doc: add Google Analytics tracking. #6601

phillipj commented May 5, 2016 •

edited

Loading

phillipj May 5, 2016

Fishrock123 commented May 5, 2016

phillipj commented May 5, 2016

jasnell commented May 6, 2016

Fishrock123 commented May 6, 2016 •

edited

Loading

rvagg commented May 10, 2016

phillipj commented May 10, 2016

phillipj commented May 16, 2016

jbergstroem May 16, 2016

phillipj May 18, 2016

jbergstroem May 18, 2016

jbergstroem May 31, 2016

phillipj May 31, 2016

ChALkeR commented May 18, 2016 •

edited

Loading

bnoordhuis commented May 19, 2016

saghul commented May 19, 2016

phillipj commented May 25, 2016

eljefedelrodeodeljefe commented May 25, 2016

mikeal commented May 25, 2016

ChALkeR commented May 27, 2016

phillipj commented May 30, 2016

rvagg commented May 31, 2016

ChALkeR commented May 31, 2016 •

edited

Loading

phillipj commented Jan 25, 2017

phillipj commented Jan 28, 2017

gibfahn commented Jan 28, 2017

phillipj commented Jan 28, 2017 via email

ChALkeR commented Jul 5, 2017

phillipj commented Jul 6, 2017 •

edited

Loading

ChALkeR commented Aug 30, 2018

doc: add Google Analytics tracking. #6601

doc: add Google Analytics tracking. #6601

Conversation

phillipj commented May 5, 2016 • edited Loading

Checklist

Affected core subsystem(s)

Description of change

phillipj May 5, 2016

Choose a reason for hiding this comment

Fishrock123 commented May 5, 2016

phillipj commented May 5, 2016

jasnell commented May 6, 2016

Fishrock123 commented May 6, 2016 • edited Loading

rvagg commented May 10, 2016

phillipj commented May 10, 2016

phillipj commented May 16, 2016

jbergstroem May 16, 2016

Choose a reason for hiding this comment

phillipj May 18, 2016

Choose a reason for hiding this comment

jbergstroem May 18, 2016

Choose a reason for hiding this comment

jbergstroem May 31, 2016

Choose a reason for hiding this comment

phillipj May 31, 2016

Choose a reason for hiding this comment

ChALkeR commented May 18, 2016 • edited Loading

bnoordhuis commented May 19, 2016

saghul commented May 19, 2016

phillipj commented May 25, 2016

eljefedelrodeodeljefe commented May 25, 2016

mikeal commented May 25, 2016

ChALkeR commented May 27, 2016

phillipj commented May 30, 2016

rvagg commented May 31, 2016

ChALkeR commented May 31, 2016 • edited Loading

phillipj commented Jan 25, 2017

phillipj commented Jan 28, 2017

gibfahn commented Jan 28, 2017

phillipj commented Jan 28, 2017 via email

ChALkeR commented Jul 5, 2017

phillipj commented Jul 6, 2017 • edited Loading

ChALkeR commented Aug 30, 2018

phillipj commented May 5, 2016 •

edited

Loading

Fishrock123 commented May 6, 2016 •

edited

Loading

ChALkeR commented May 18, 2016 •

edited

Loading

ChALkeR commented May 31, 2016 •

edited

Loading

phillipj commented Jul 6, 2017 •

edited

Loading