Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Analytics] send requests to domain derived from variable #19759

Closed
tingyiwei opened this issue Dec 10, 2018 · 4 comments
Closed

[Analytics] send requests to domain derived from variable #19759

tingyiwei opened this issue Dec 10, 2018 · 4 comments

Comments

@tingyiwei
Copy link
Contributor

What's the issue?

Instead of sending AMP analytics requests to a fixed domain, we'd like to send to domain derived from am Analytics variable.
requests: {
endpoint: ${someVariable}
}
The issue is ${someVariable} always return an encoded string, while here an decoded string is needed.

How do we reproduce the issue?

<script type="application/json"> { "requests": { "pageview": "${documentReferrer}" }, "triggers": { "pageload": { "on": "visible", "request": "pageview" } } } </script>

In this case, https%3A%2F%2Fwww.example.com is returned instead of https://www.example.com

What browsers are affected?

ALL

Which AMP version is affected?

ALL

@zhouyx
Copy link
Contributor

zhouyx commented Dec 10, 2018

cc @lannka @calebcordry
AMP will by default encode vars to not break the outgoing url. But this sounds like a very solid use case.

We propose to add a new requestOrigin field to the top level analytics config. When specified, all requests defined will be treated as the url path. The requestOrigin part will not be encoded and the outgoing url will be consisted following the url constructor rule.
Example config

{
  'requestOrigin': 'https://${domain}',
  'requests': {
    'base': '/base',
    'click': '${base}?click'
  },
  'vars': {
    'domain': 'example.com'
  }

}

The outgoing request will be https://example.com/base and click: https://example.com/base?click

Let me know what do you think?

Other options that has been discussed, but not adopted.

  • Add origin field to the request object.

    • [Pro] Request object level origin provides more flexibility. However in most cases, there's only one origin to specify for every request.
    • [Con] Nested request expansion can be tricky. It is not clear which part of the request (baseUrl or origin) to replace. We could only replace baseUrl, but override origin, but that's still not very straightforward and requires some level of explanation.
    • We could consider adding request object level override upon further request.
  • Introduce DECODE macro

    • [Pro] Easy to add, will not affect current analytics design.
    • [Con] Not really a very element solution. It is AMP that encode vars by default, it could be confusing to ask developers to decode in this case.
    • [Con] This is tricky implementation wise. As encode happens after all macros resolve.
  • Ask Vendors to use REGEX

    • [Pro]: Temporary solution that requires no change from AMP's side
    • [Con]: Only a very temporary solution.

@calebcordry
Copy link
Member

Reviving this issue, since the original solution has been proposed the team feels that there might be greater utility in a solution to use unencoded values in other contexts. I know @tingyiwei is blocked on this.

@zhouyx Do you feel now feel that the DECODE macro is a better choice, and if so is this the right time for implementation?

cc/ @ampproject/wg-analytics

@calebcordry
Copy link
Member

Talked offline, @zhouyx's proposal still seems to be the best solution.

@zhouyx
Copy link
Contributor

zhouyx commented Jun 21, 2019

Closed by #22454

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants