cfpb / consumerfinance.gov

Django project protecting American consumers
https://www.consumerfinance.gov/
Creative Commons Zero v1.0 Universal
273 stars 110 forks source link

Should we be using Google Analytics and Google Tag Manager snippets concurrently? #496

Closed anselmbradford closed 8 years ago

anselmbradford commented 9 years ago

In https://github.com/cfpb/cfgov-refresh/blob/flapjack/_layouts/base.html#L113-L129 we have both Google Analytics (GA) and the Google Tag Manager (GTM) code snippets.

I noticed in https://support.google.com/tagmanager/answer/2574370 the following:

A container snippet is provided for each container. Copy and paste this snippet immediately after the opening tag on every page on the site. Be sure to remove tags (e.g. Google Analytics, AdWords Conversion Tracking, etc) from the site pages at the same time.

I'm not super familiar with GTM, but according to http://stackoverflow.com/questions/23640645/what-is-the-difference-between-google-tag-manager-and-google-analytics we can output GA code from GTM.

Scotchester commented 9 years ago

/cc @bethannenc

bethannenc commented 9 years ago

That is true - once we have GTM fully implemented we can remove the old GA code and rely just on GTM. However, we have not removed the GA code as of yet because we wanted to let the two accounts run in parallel to make sure GTM was implemented correctly. We are almost at that point where we would feel comfortable removing the code - most likely by June 1. For new content, we are comfortable just implementing GTM and not adding the GA code snippet.

I hope that helps - let me know if you have any other questions.

anselmbradford commented 9 years ago

@bethannenc Do you know why GTM has a jQuery dependency and can that be removed/replaced with native code? http://www.googletagmanager.com/gtm.js?id=GTM-KMMLRS

Scotchester commented 9 years ago

/cc @wernerc

bethannenc commented 9 years ago

GTM itself does not have a jQuery dependency. However, some of the tags that we have written are written in jQuery and they will not work if jQuery is not on the page. We could fix that by translating all of our tags in jQuery into JS but that would be a major pain. @wernerc - thoughts?

KimberlyMunoz commented 9 years ago

Thanks for asking this question. I had the same question for eRegs.

cc @ascott1

mistergone commented 9 years ago

My goal is to try to have all Tag Manager code fit the following:

Most of the GTM code is just copied from old Analytics code, then modified to work with GTM. Most of the old Analytics code used jQuery. We're now in a world where jQuery is not used everywhere on our pages, and sometimes, even when it is, it's not exposed to the window scope (not to mention $). That shouldn't stop our Analytics code from working on all of our pages, so some overhaul is now necessary.

cfarm commented 9 years ago

@virginiacc

jimmynotjim commented 9 years ago

Bumping this issue because we're seeing lots'o jQuery undefined errors related to GTM and I'm now wondering if we're missing analytics capturing when that happens. See the conversation on #585

jimmynotjim commented 9 years ago

To continue the conversation in #585, the error pops up on pages where jQuery is used, but not all the time. The issue as I understand it from @anselmbradford is that GTM loads asynchronously sometimes beating out the loading of jQuery.

anselmbradford commented 9 years ago

Yeah, for context look at the source code for the live www site:

<head … >

…

<script type="text/javascript" async="" src="http://www.google-analytics.com/analytics.js"></script>
<script async="" src="//www.googletagmanager.com/gtm.js?id=GTM-KMMLRS"></script>
<script src="//www.google-analytics.com/ga.js"></script>

…

<script type="text/javascript" src="http://www.consumerfinance.gov/wp-content/themes/cfpb_nemo/_/js/jquery-1.9.1.min.js?ver=v4.2.10v4.2.24"></script>

…

<!-- H5BP's Asynchronous Google Analytics snippet. -->
<script>
  var _gaq=[['_setAccount','UA-20466645-3'],['_setDomainName', '.consumerfinance.gov'],['_trackPageview']];
  (function(d,t){var g=d.createElement(t),s=d.getElementsByTagName(t)[0];
  g.src=('https:'==location.protocol?'//ssl':'//www')+'.google-analytics.com/ga.js';
  s.parentNode.insertBefore(g,s)}(document,'script'));
</script>   

…

</head>

That's the head with everything but the tracking code and jQuery excluded. You'll notice the script order is:

Looking at this without much understanding of how GTM works under ideal circumstances it looks to me like this is happening:

Under less ideal circumstances this could happen as far as I can see:

cfgov-refresh has moved scripts around and jQuery loads in the footer now, this refactoring certainly could have introduced issues regarding load order, but the outcome should be the same as the live site and cfgov-refresh are both starting jQuery downloading after GTM. Ideally, GTM would handle it's own dependency-management and dynamically inject a script for jQuery if it wasn't available—that is if it really needs jQuery at all and couldn't be refactored.

jimmynotjim commented 9 years ago

You're forgetting the GTM script in the <body> on L:85 of cf.gov. I don't think GTM does anything till it gets to there, so jQuery has already been loaded at that point.

The issue on the demo appears to be that jQuery isn't loaded until the footer. I believe the reason it's a hit or miss issue is because the main script file is being cached by the browser and loading faster on subsequent page loads.

mistergone commented 9 years ago

This can easily be fixed by me playing with a few things in GTM. Sadly, I am on vacation, so this comment is all I can do until June. :D

anselmbradford commented 9 years ago

@jimmynotjim Ah! Good catch, yup see that now, so the body script is dynamically creating the GTM script in the head, so should be good on live. Updated order is:

jimmynotjim commented 9 years ago

Right, but the generation scripts aren't adding/loading the dynamic scripts until the browser has parsed them, and by that point it should have already started loading jQuery. In this case page order does not entirely equal loading order. Load order is something more like:

  1. Download jQuery
  2. HTML5 Boilerplate (H5BP) script inject GA script url to head and begins downloading it
  3. GTM generation script injects GTM and GA script url to head and begins downloading them

There's still a chance of the GTM source beating out jQuery on cf.gov, but it's far less (and I've yet to replicate it) than on the demo sites since jQuery isn't loaded until almost last on them.

Edit, here's the timeline showing this in action

screen shot 2015-05-22 at 3 26 56 pm

anselmbradford commented 9 years ago

@jimmynotjim Yeah, that order I listed was just order the scripts appear in the source, not load order.

jimmynotjim commented 9 years ago

What order they're in the source doesn't really matter though right? They could be injected anywhere and the loading order would still be the same.

jimmynotjim commented 9 years ago

Here's the problem on the demos, common.min.js (where jQuery lives) is so bloated that it's not even done downloading till much later than GTM

screen shot 2015-05-22 at 3 37 57 pm

Also, I found if I disable caching in the Console Settings I get the error every time. When cache is enabled it's a bit better.

screen shot 2015-05-22 at 3 42 32 pm

anselmbradford commented 9 years ago

What order they're in the source doesn't really matter though right?

That is right. Sorry I should I have been clearer—I was just updating the first list in https://github.com/cfpb/cfgov-refresh/issues/496#issuecomment-104714299.

So as a stop-gap this is relatively straightforward fix I think. We can place a CDN to jQuery in the head and use https://github.com/thlorenz/browserify-shim#a-expose-global-variables-via-global in browserify. It probably wouldn't be a bad idea to load jQuery from a CDN anyway, so this would be a fine direction to go I think. Eventually though it would be better to load jQuery in the footer to avoid a blocking script in the head. Opened https://github.com/cfpb/cfgov-refresh/issues/587. Comment there if the approach I wrote seems off.

anselmbradford commented 8 years ago

Looks like this can be closed. I just realized GA was removed awhile ago https://github.com/cfpb/cfgov-refresh/commit/7e8191ea3a9f87ebe8351f09921763f2891f03e7 @bethannenc Are you'll okay with it just being GTM for launch (that would be our preference, since it's Google best-practice). Otherwise, closing this because standalone GA is gone and the jquery CDN is in the head with a TODO comment.

bethannenc commented 8 years ago

@anselmbradford Yep - we should only be using GTM at this point. No need to keep any old GA code.