Closed (fixed)
Project:
Google Tag
Version:
7.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
19 Mar 2019 at 18:35 UTC
Updated:
11 Dec 2019 at 15:09 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
sthomen commentedThis patch doesn't apply cleanly (to 7.x-1.4+5-dev), on line 187 the line
should be:
Comment #3
fernando iglesias commentedThanks for reviewing. Here's a reroll against latest dev.
Comment #4
solotandem commented@Fernando Iglesias Your patch files indicate:
A few questions.
Would you edit the issue summary to include the rationale for this request?
The 7.x module supports creating multiple containers in the context of variable realms. In this sense, each container may have separate configuration settings (e.g. container ID, insertion conditions, etc.). However, the page response would only include the container for the primary realm:key on that response. Does this feature support your use case?
Is your use case supported by the environment concept offered by Google Tag Manager?
Comment #5
t00lie commented@solotandem responding on behalf of Fernando:
We manage several hundred discrete websites at a large academic institution. Most of those sites report to at least two GA codes, but many of the sites do not use the same two codes. As platform owners, one is an internal code for aggregate tracking. The other codes are for sharing discretely with other groups in our organization.
We have a similar situation with GTM containers. The majority of sites use the same stable of base event tags, but there are a handful of sites that require additional event tagging that the hundreds of other sites don't need to load. In some cases we inherit a site with a legacy container that they are used to managing themselves, but they shouldn't have access to our core container.
A container-ception strategy in GTM could certainly work in theory, but managing (and mismanaging) all of those conditions could lead to problems, plus it adds a ton of bloat to every site that uses that one container.
If we were to start from scratch, we might do things differently but this is the architecture we have and it hasn't let us down yet!
Comment #6
fernando iglesias commentedTo add, doing multiple containers in this manner is a strategy that google supports but the module does not; see here: https://developers.google.com/tag-manager/devguide#multiple-containers
Comment #7
solotandem commentedThanks for the details. Do you happen to use your patch in production? If not, what are you doing in the interim?
Comment #8
fernando iglesias commentedYes, we've been using this in production for the past few months. We've got it on about 100+ sites using 1-3 containers on each. If you pop open your browser's dev tools network tab and filter on gtm, you can see it in action here: https://home.dartmouth.edu
Comment #9
solotandem commented@Fernando-Iglesias Would you be able to test the 7.x-2.x branch which has support for multiple containers (in the same way as the 8.x branch)? For each container, you can define the container ID as well as all the other values (data layer, insertion conditions, etc.). There is an update hook to run that will migrate your container from variables to config table. You would need to run this without your custom patch.
Comment #10
fernando iglesias commentedThe 2.x version sounds great and would definitely take care of our use case. Unfortunately, we're not really willing to run unstable/no-security-support modules in prod, so we'll be sticking to 1.x with the above patch. Once 2.x gets a stable release though, I'll definitely check it out.
Comment #11
solotandem commentedI was not asking you to run 2.x in prod but on a dev environment. Your feedback would be useful before making the first release.
Comment #12
adanbouzoua commentedI tested it and I am getting this error
Comment #13
solotandem commentedThanks for testing this.
Latest commit removes the type hints from the interface file. Would you test again?
Curious, what PHP version are you running with this?
Comment #14
fernando iglesias commentedFor anyone who needs, here's a reroll of the 7.x-1.x patch against the latest release.
Comment #15
solotandem commented@Fernando Iglesias Take it with a grain of salt, but I am going to poke you on this. You file an issue asking for multiple container support. I add it and ask you to test the changes (implied on a test environment). You miss the point and, even after clarification, seem unwilling to participate in the open source process and test what you asked for. Am I missing something here or are you missing the concept of open source? community? etc.? How about it? [Granted the dev release does not have security support, but to claim it is unstable is a bit much, especially from someone who has not even tried it.]
@t00lie You remark that
managing (and mismanaging) all of those conditions could lead to problems. With the 2.x branch you configure the default settings once, then create the individual containers (which inherit the default settings) and set the container ID. Where is the management difficulty in this? As to your claimit adds a ton of bloat to every site that uses that one container, what exactly are referring to?Comment #16
fernando iglesias commented@solotandem:
I didn't ask for anything, I added a feature that was missing from 7.x-1.x, and provided a patch for anyone else who might need it. If you want to commit it, go ahead. If you don't, that's fine too.
I didn't miss the point, I clearly told you that I'm not interested in helping you QA a 2.x release. I don't work for you and I don't owe you a thing. I'm only interested in my use case, which is multiple container support in the supported version of the module. Why exactly does it need to be left out of 1.x again?
Relax with the ego. You've got some nerve telling someone that bothered to contribute a patch to your work that they're missing the point of open source and community. I just hope I was able to help someone else that was in the same boat.
Have a nice thanksgiving!
Comment #17
solotandem commented@Fernando Why exactly does it need to be left out of 1.x? Because it is a kluge.
Comment #18
fernando iglesias commentedIt's really not, its a pretty basic non-breaking extension to the module. But agree to disagree. If you have any thoughts on how you'd prefer to see multiple container support added to 1.x, then I'm also happy to work with you there.