Amphtml: Support LINKER_PARAM in the vars section of amp-analytics

Created on 14 Feb 2019  路  21Comments  路  Source: ampproject/amphtml

We need to save a query param (gclid) in a cookie in our amp-analytics tag.
Since cookies can't be saved in cached AMP pages, we use the linker to pass the param to the next page.
This works well if the next page is either a non-cached AMP page or a regular HTML page.
However, if the next page is another cached AMP page this solution doesn't work, because the LINKER_PARAM function that is used to extract the param from the linker is only supported in the cookies section (which doesn't work in a cached page).
If the LINKER_PARAM function was supported in the vars section we could pass the param from one cached AMP page to another until we reach either a non-AMP page or a non-cached AMP page.

We need something like this:

<amp-analytics>
    <script type="application/json">
        {
            "vars": {
                "myLinkerParam": "LINKER_PARAM(linker, param)"
                "myParam": "QUERY_PARAM(param, ${myLinkerParam})"               
            },
                                    :
            "linkers": {
                "linker" : {
                    "ids" : {
                        "param": "${myParam}"
                    },
                    "enabled": true
                }
            },
            "cookies": {
                "enabled": true,
                "my_param_cookie": {
                    "value": "${myParam}"
                }
            }
        }
    </script>
</amp-analytics>
Feature Request analytics

Most helpful comment

@ifatgv4

If it's a regular HTML page, you can write your own logic to handle the linker param, or you are more than welcome to refer to the AMP's cookie writer implementation.

You can refer to the linker param receiving doc here. @calebcordry can help you with any question there.

All 21 comments

Triaging to @zhouyx , feel free to re-assign 馃槃

@ifatgv4 Thank you, I can see that's this use case is not supported today and it is a very reasonable request.

to @lannka @calebcordry instead

LINKER_PARAM removes the param from the window.location. We need to somehow make sure that LINKER_PARAM is called only once for a given id.

Thank you @zhouyx for the quick response!

I also have a question. If we navigate to a regular HTML page from the AMP page, how do you recommend we extract the param value from the linker?
Should we copy the linker code, or is there a way to import the relevant linker file(s)?

Thanks

@ifatgv4

If it's a regular HTML page, you can write your own logic to handle the linker param, or you are more than welcome to refer to the AMP's cookie writer implementation.

You can refer to the linker param receiving doc here. @calebcordry can help you with any question there.

We also have some test cases here that you can use to double-check your logic. It may be helpful to review AMP's implementation as well.

Thanks! I will check it out.

@calebcordry
What if you change linker's version? Will it impact us when extracting the parameters?

@chcohen I dont think we will be changing the version any time soon. If we do, we would definitely give plenty of warning

@calebcordry Is it going to be implemented? Is there an ETA?
Thanks

@ifatgv4 working on it now. Targeting next week's canary

@calebcordry Wonderful! Thank you

@calebcordry, thank you for handling this.

When will the changes be available?

@ifatgv This hit canary yesterday. Should be in prod next week.

@calebcordry thanks

Hi @calebcordry,

I tried using the new code and I am having some issues.

When I add:

"myLinkerParam": "LINKER_PARAM(linker, param)"

to vars is works well. But when I try to do either:

"myLinkerParam": "LINKER_PARAM(linker, param)"
"myParam": "QUERY_PARAM(param, ${myLinkerParam})"

or

"myParam": "QUERY_PARAM(param, LINKER_PARAM(linker, param))"

I get an error in the console that says that:

LINKER_PARAM requires two params, name and id

I tried debugging it and it seems that the parser parses the LINKER_PARAM wrong and the values it sends to the function get are:

name: "linker%2C%20param"
id: ""

Should I do it another way?

Also, from what I see, the cookie-writer class was not changed. It still only accepts QUERY_PARAM and LINKER_PARAM. It doesn't accept a variable. So if I want to save the one of them that is not empty there is no way to do that in the same cookie. Or am I missing something?

@ifatgv4 thanks for reporting I will look into this.

So if I want to save the one of them that is not empty there is no way to do that in the same cookie. Or am I missing something?

I think that you should be able to use the $IF macro here. Something like $IF(QUERY_PARAM(foo), QUERY_PARAM(foo) , LINKER_PARAM(foo)) but it may have the same problem as the bug you are reporting. I will look into this use case as well.

@calebcordry Thanks!

Regarding the cookie, no, you can't use $IF. I checked and you get:

cookie value $IF(QUERY_PARAM(param), QUERY_PARAM(param), LINKER_PARAM(linker, param)) not supported. Only QUERY_PARAM and LINKER_PARAM is supported

If on the other hand you try to set the cookie value to:

QUERY_PARAM(param, ${myParam})

The cookie is created with the value:

%24%7BmyParam%7D

(no variable expansion)

OK @ifatgv4 I did some investigation. There are a few things going on here.

  1. Using nested macros with the ${} syntax. This has nothing to due with linker but is a bug in our getNameArgs logic. I created #21751 to track. This manifests itself here with the weird encoding bugs you are seeing. It is unable to extract the args correctly, so it wrongly encodes them and breaks.

  2. Using vars inside of cookies, this is something that we actually do not want to allow, as it would open up any macro to be used inside of the cookies config which is a security hole.

  3. Allow $IF to be used inside of cookies. This is something we do want to allow but unfortunately is blocked by the same bug as (1). We need to get this fixed, I would suggest to follow along on that issue.

Let me know if any questions.

@calebcordry Thank you. I will follow it

@calebcordry Should I open a feature request for supporting $IF in cookies?

Was this page helpful?
0 / 5 - 0 ratings