[GTM + GA4] Add `tagmanager` datastore partial to determine connected GA4 entities about site-kit-wp HOT 11 CLOSED

1. From what I can see, we cannot add GA4 tags to an AMP container within the TagManager Admin UI. So am I correct in assuming we won't be supporting AMP containers in our Tag Manager integration?

@jimmymadon I wasn't aware of this, but it's not entirely surprising since GA4 support for AMP was added sometime around June, right before the start of the UA sunset. I think we'll need to ask internally if support will not be coming for it, otherwise I think we should keep that part but we'd only check for the tag in web containers, does that make sense? Essentially trying to keep it the same as today.

However, it is possible to attach multiple GA4 tags with different measurement IDs to a container. Should we cater for this and refactor the above selectors for this?

I believe this was possible with UA as well but we only check for one, correct? This is similar to existing tags where we only "capture"/report about the first one we find. Having multiple GA tags in GTM should be an edge case and trying to account for it would likely be a large lift that probably isn't worth it.

3. I can see that the UI allows for two types of GA4 tags, one is standard Google Tag (googtag) and one is a GA4 Event tag (gaawe). Should we look for both of these types when finding linked measurement IDs?

We should only be concerned with the Google Tag. The event tag is for custom events that are set up, and managed in GTM.

from site-kit-wp.

Hi @jimmymadon, apologies but while reviewing #7990 I had the realisation that we haven't quite got the IB right for the implementation of getLiveContainerAnalytics4MeasurementID().

Where I've steered us slightly wrong is talking about the tagId as though it's always a measurement ID. It's actually a Google tag ID - which might happen to be a measurement ID for the automatically created tags, but there's no guarantee of that being the case.

Here's an updated set of screenshots to illustrate this:

So, the implication here is that we actually need to do an additional container lookup in order to determine the measurement ID from the Google tag ID. We probably want to determine the measurement ID from the container using similar logic to that used in GoogleTagIDMismatchNotification, but should presumably fall back to the first valid measurement ID if none of them exist in the available Analytics accounts.

Note that I've screenshotted the using-a-variable scenario, but even without a variable we'd still need to do a container lookup.

Please can you update the IB accordingly, and drop me a line if you want to discuss this further?

from site-kit-wp.

@nfmohit @aaemnnosttv

1. In our current implementation, we have to cater for multiple "property IDs" because we cater for for ua and ua_amp tag types based on the usage context of the containers (either web or amp) linked to an account. From what I can see, we cannot add GA4 tags to an AMP container within the TagManager Admin UI. So am I correct in assuming we won't be supporting AMP containers in our Tag Manager integration?

2. If yes, then we wouldn't need all the remaining selectors other than getLiveContainerAnalytics4Tag and getLiveContainerAnalyticsPropertyID. All the remaining selectors solely exist to cater for the potential of there being a UA and and an AMP tag. However, it is possible to attach multiple GA4 tags with different measurement IDs to a container. Should we cater for this and refactor the above selectors for this?

3. Finally, I can see that the UI allows for two types of GA4 tags, one is standard Google Tag (googtag) and one is a GA4 Event tag (gaawe). Should we look for both of these types when finding linked measurement IDs?

from site-kit-wp.

Hey @jimmymadon, the AC here LGTM. ✅

The IB also looks good; there's just one aspect that could use clarification. Considering getLiveContainerAnalytics4MeasurementID(), I can see a question arising at execution time as to whether we should provide support for using variables, as we currently do for UA. (Actually, on reflection this could just as well be an AC-level question, but we should figure it out one way or the other :)).

Providing full support for variable use would clearly be out of scope, but it might be worth adding support for constant variables which seem to be the most straightforward case.

Alternatively, we could defer variable support and explore it more fully in a separate issue. Either way, it would be useful to make it clear what approach to take in the IB. What do you think?

Additionally, a minor point - there are a couple of typos in the IB with getLiveContainerAnalyticsMeasurementID and getAnalyticsMeasurementIDs both missing the 4 after Analytics.

from site-kit-wp.

@techanvil My apologies - I had just assumed that fetching the propertyID via variables was legacy behaviour. On doing some more research, I can now confirm that it is not possible to set a GA4 measurement ID as part of a gaSettings variable anywhere in the GTM UI.

We can set Google Tag configuration settings but these do not allow to set a measurementID parameter anywhere.

So should we drop support for variables all together? (UPDATE: I have updated the AC and IB to reflect this decision for now.)

from site-kit-wp.

Hey @jimmymadon, while we can't use gaSettings, and Google Tag configuration settings doesn't look applicable, we can still use variables to specify the measurement ID. The simple example I noted is the Constant variable type - I've given this a test in the wild and it does work.

So, I was thinking we might want to provide support for constant variables and then review other potential cases later... Or, as mentioned simply defer the whole topic. But considering we do currently provide support for variables, it does feel like it would be nice to retain some support in the initial migration.

from site-kit-wp.

@techanvil I did try this out too but was just wondering how we would "support" this. There is no indication that the constant variable above would be a measurement ID (even if we can make out from the format of the string). I don't think we can simply assume that this string, even if it is a measurement ID, is one that needs to be tracked within this GTM container. I don't think that using variables to store the measurement ID is prescribed anywhere by GTM and is good practise (we have a Google Tag for this precise reason). The reason we already have variable support is purely because the "Google Analytics Settings" variable would always have a property ID set and we would be certain about its existence there. So IMO, dropping variable support here fully is potentially the right way to proceed unless we want to capture any other data from variables in the future.

from site-kit-wp.

Thanks @jimmymadon. I do think that it would be straightforward to support in the simple case where the variable reference is specified as the tagId value, which is easy to setup via the UI.

I don't think there's necessarily a question of good practice here, the user-defined variables are there to support specific user requirements - I think using a variable to share a measurement ID across multiple tags would be a pretty legitimate use case, as, for example both Google Tag and GA4 Event tags have a measurement ID field.

However, I do take your point that there can be more complex uses e.g. multiple variables, or the variable itself can point to other variables. Given it's a bit of an arbitrary isolated case to support, let's go ahead and drop the variable support for now, we can take a more comprehensive look for a future iteration.

It is, though, not correct to say that it's no longer possible to set the measurement ID via a variable - as mentioned I tested out setting it via a constant and it worked fine, correctly tracking events, so please can you tweak the AC accordingly? I'll send it back to AC for the purpose but the IB looks good to go.

from site-kit-wp.

Thanks for the update @jimmymadon!

As discussed on Slack we've gone ahead with adding support for variables of type constant.

AC ✅
IB ✅

from site-kit-wp.

@jimmymadon This is going to be scheduled for Sprint 120 – can you please prioritize making these IB updates? Thanks!

from site-kit-wp.

As summarised in Slack Thread 1 and Slack Thread 2, it is not possible to have an exact 1:1 mapping between selectors mentioned in the AC of this issue.

The selectors being adapted here were originally planned to be used in #7990 which attempted to adapt the following two UA-GTM integration screens that were shown to the user when setting up Google Tag Manager:

1. When GA is not connected, but the selected Tag Manager container had a tag that was a valid UA trackingId (UA Property ID):
   

   This was achieved using the getLiveContainerAnalyticsPropertyID selector which fetched the Property ID from the selected GTM container's live version:
   

   site-kit-wp/assets/js/modules/tagmanager/datastore/versions.js

   Lines 197 to 201 in bf2219e

   	if ( analyticsTag?.parameter ) {
   	// Check if property ID is provided directly on the tag first.
   	let propertyID = analyticsTag.parameter.find(
   	( { key } ) => key === 'trackingId'
   	)?.value;

   As the IB of this issue mentions, we can fetch the googtag from the current GTM Container. To determine if and which GA4 "destinations" (i.e. web data streams) this Google Tag is pointing to, we need the tag's own unique accountId and containerId which gets created when the Google Tag is created. Then we could use getGoogleTagContainerDestinations(accountId, containerId) which uses the container-destinations endpoint to fetch the destinations. However, the accountId and containerId we get when we do the getLiveContainerVersion query returns the GTM accountId and containerId which isn't the tag's unique IDs that we are looking for. So there is no direct API support for us to fetch a Google Tag's information.

   Result of `getLiveContainerVersion`

   

   We could potentially loop through all of the current user's accounts and containers and list all possible destinations, and check if any of those destinations is connected to the Google Tag that is found above. This would be very inefficient and unnecessary for the purpose here which is to inform the user that their currently selected GTM config contains a GA4 property.

   So we have decided to modify the AC of #7990 such that, if we find a Google Tag in the current GTM container, we can simply validate it to check if the format matches a valid Google Tag. If it does, we can tell the user that their configuration contains a Google Tag which could be connected to a GA4 property - so they should consider "Connecting Analytics". We will not check if the Google Tag points to an actual GA4 web data stream until perhaps direct API support is available to find these details from a Google Tag ID.

2. When GA is already connected, and the selected Tag Manager container had a tag that was a valid UA trackingId which matched the existing propertyId saved in Analytics settings:
   

   In this case, the user was informed that only the GTM snippet would be inserted and the Analytics snippet would not be inserted. This was to prevent duplicate events from firing. However, GA4 prevents duplicate events from firing regardless of another snippet with the same config being inserted. Also, inserting the GA4 snippet is coupled with inserting code that enables the tracking of custom dimensions introduced with the News Key Metrics feature. This means it is essential that this snippet is inserted until we manage to decouple these two behaviours.

   Thus it makes no sense to inform users about this scenario anymore. Even if the GA4 config overlaps with a tag from their GTM container, both snippets can be introduced to their site safely without any adverse effects. So in issue #7990, we will no longer require this screen to be displayed. In fact, we will remove functionality which prevented the duplicate snippets being inserted in issue #7991.

So in summary:

• #7989 (this issue) will be closed and the adaptation of selectors will be incorporated into 7990 which actually uses them. Any selectors no longer required should not be adapted.
• #7990 will handle the displaying of a screen that prompts the user to Connect Analytics if a Google Tag was detected within their GTM config.
• #7991 will be repurposed closed and a new issue #8196 has been created to remove the prevention of duplicate snippets being inserted.
• #7992 will remove any other code no longer being used in #7990.

from site-kit-wp.