amp-analytics: Added support for extraUrlParamsReplaceMap defined in a trigger block.

[dougscher]

This will add efficiency and better organization of param replacements that are only associated with specific triggers.

Similar use case as with: Added support for extraUrlParams in trigger blocks and URL vars. #3168

[aghassemi]

/to @zhouyx

[zhouyx]

@dougscher You are suggesting adding extraUrlParamsReplaceMap to trigger config as well as the extraUrlParams. What to do with global extraUrlParamsReplaceMap and trigger level extraUrlParamsReplaceMap? Shall we combine the values and override config level value with trigger level value? Could you please share your detail use case?

[dougscher]

Right now, we use extraUrlParamsReplaceMap to allow url params to be specified by the user as a meaningful name: articleId: 'artid',

So the user does not have to deal with the cryptic names that are actually used in the event url. This could be done with variables, but then we would be sending a lot of empty url params if they are not used, same example: &artid=""

Right now extraUrlParamsReplaceMap is limited to 16 entries for some reason. bumping that to maybe 40 would take us a long way.

Also allowing extraUrlParamsReplaceMap to be specified to a specific trigger would allow controlling these sets of parameters to only apply to specific events.

[zhouyx]

@rudygalfi Do you maybe have some context on why we put the 16 entries limit to extraUrlParamsReplaceMap. from #1932

[ampprojectbot]

This issue hasn't been updated in awhile. @zhouyx Do you have any updates?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[dougscher]

This issue hasn't been updated in awhile. @zhouyx Do you have any updates? This change would fix some challenges we have with our implementation.

[zhouyx]

I think expanding the limit from 16 to 40 is reasonable. I also think that extraUrlParamsReplaceMap should only be defined by analytics vendors. Maybe a good approach is to remove the count limit check, but disallow the configuration of extraUrlParamsReplaceMap inline.

cc @jeffjose

[micajuine-ho]

From a performance standpoint, increasing the limit from 16 to 40 doesn't seem like it will be immensely impactful.

However, removing inline support seems a little risky though since some pubs might have custom inline replace maps.

I would prefer to only add support for extraUrlParamsReplaceMap within a trigger block:

• If an extraUrlParamsReplaceMap is defined in a trigger block, then we will use it to preprocess the extraUrlParams within the trigger block
• Top level extraUrlParams will not be affected by trigger level extraUrlParamsReplaceMap

@dougscher Would this be sufficient for your use case?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.