Amphtml: Add filter support for analytics vars

[cramforce]

Updated by @lannka on 4/3/2017:

Use cases:

• Adobe Analytics does not like - (dashes) in client ids, which could be enforced with such a mechanism.
• Google Analytics has cookie value in format of GA1.1.12345.54321, and we want to extract the 12345.54321 part to report as Client ID.

V1

• Introduce a regex filter, which would address most requirements.

V2

• Introduce other filters, such as

 Not (negation)
 Trim
 JSON.stringify
 Base64
 Default()
 ToLowerCase
 ToUpperCase

Task break down:

• [ ] Finalize the design of filter syntax.
• [ ] Implement rule parser
• [ ] Implement filter

[ryanlombardo]

+1 for this. We have a similar formatting tool in our internal analytics library at BuzzFeed.

[rudygalfi]

@avimehta Have you given any further thought to implementing this?

Moving to Backlog since we haven't touched this in awhile.

[avimehta]

Here are a few things that I could think of implementing. Feel free to add more to the list. wdyt?

• [ ] Not (negation)
• [ ] Trim
• [ ] JSON.stringify
• [ ] Base64
• [ ] Default()
• [ ] ToLowerCase
• [ ] ToUpperCase

[rudygalfi]

Looks like a good list.

From a format point of view, we should review the pipe (|) syntax and make sure we definitely want to go that way. How would nesting work?

Doe any of the items on the list address @cramforce's original suggestion of stripping dashes?

[dvoytenko]

We should also definitely consider function syntax, such as not(...) or base64(...)

[avimehta]

none of the items so far listed take care of stripping dashes. The filters should be cheap and simple functions though and can be implemented for such needs.

For Dima's comment, I agree about the function syntax. So a variable could be something like:

${clientId(amp_id)|toLowerCase()|default(emptyCid)}

${jsonVariable|json()|base64()}

We could also make () optional when there are no parameters. That way the syntax gets a little easier:

${clientId(amp_id)|toLowerCase|default(emptyCid)}

${jsonVariable|json|base64}

As for the | syntax, I have not looked into exact details and don't know what could go wrong. An alternative would be to consider nested functions like ${base64(json(jsonVariable))} . Not sure if I like it though.

[dvoytenko]

@avimehta What about ${toLowerCase(clientId(amp_id))}?

[avimehta]

@dvoytenko Is this the same as the nested functions that I talk about in the last paragraph? I like the readability of this. It has slightly more involved parsing though. Do you like this more over the other options?

[dvoytenko]

Having some concerns over additional syntax.

[georgecrawford]

I originally raised https://github.com/ampproject/amphtml/issues/2476, and I'm wondering what the latest is on this? I'd like to send a boolean value in my analytics submission based on an access decision made in the client (e.g. barrier: 'NOT AUTHDATA(access)', as described in the other ticket). How close are we to being able to do this?

[rudygalfi]

@avimehta has been working on this and could provide updates

[taymonbeal]

This is good! Can we please have a hash function as one of the filters? A publisher has requested this functionality.

[jridgewell]

Reverted in #6542.

[rudygalfi]

@avimehta Any updates on this?

[jridgewell]

Closed by https://github.com/ampproject/amphtml/pull/6560.

[avimehta]

sorry. Reopening. The feature is behind a flag and needs to be turned on. I think this should be ready in next two weeks.

[rudygalfi]

@avimehta Is this released yet?

[jridgewell]

Nope.

[rudygalfi]

What's the next step here?

[jridgewell]

We flip the flag in canary or prod. @lannka?

[rudygalfi]

Do we have documentation? I couldn't find any. @avimehta

[rudygalfi]

Updates:

Work still not done:

• https://github.com/avimehta/amphtml/tree/filters contains the filter changes that need to happen.
• Tests need to be added for the filter related changes, for regex filter and need to make sure that similar changes are made in other places where variableService is used

[lannka]

Decided to fix #5761 in a different way. So downgrade this to P2.

[jesuskelisto]

Hi

I'm trying to track to Mixpanel who expect an encoded json in base64, basically I'm doing this:

      "requests": {
        "pageview": "https://api.mixpanel.com/track/?data=${mp|json|base64}&ip=1&verbose=1"
      },
      "vars":{
        "mp":{
          "event": "Some Event",
          "properties":{
            "token": "MyToken",
            "distinct_id": "CLIENT_ID(mp_mixpanel)",
            "app": "MyApp",
            "amp": "true"
          }
        }
      },

The issue is that CLIENT_ID is resolved in the url after all the filters are applied.
Could be variables in vars be resolved before applying filters?.
Is there any other way to do this kind of tracking?
Thank you

[jridgewell]

@jesuskelisto: we still don't fully support filters. Once it is supported, it will work as expected.

[rudygalfi]

@lannka Can you modify the initial entry to include some subtasks so that we can track this as a multi-sprint effort?

[dtwmixpanel]

I work at Mixpanel, and we have clients who use AMP and want to integrate more fully between AMP and Mixpanel. The success of that integration hinges on this feature fully being released, including the base64 filter. Is this going to be released soon?

[jridgewell]

/cc @lannka

[avimehta]

/cc @zhouyx as well. Lets discuss this early next week.

[zhouyx]

Sounds good

[dtwmixpanel]

Any update after you discussed this issue last week?

[rohanprasad]

Hey guys, any update regarding this ?

[a-warner]

hey all, any update here? As big mixpanel and AMP users this is a big one for us

[lannka]

I think the decision was to go with the original design. @avimehta @zhouyx corrected me if I'm wrong.

I also chat with @choumx to see if there's any design conflict with amp-bind, and seems not.

There is already some code checked in for this feature long time ago. However I tested it seems not fully functioning yet. @calebcordry can you take a look what's missing?

[a-warner]

should @jesuskelisto's amp-analytics example from above work now, or should we be using a different syntax? even after adding the meta tag to enable url-replacement-v2 I cannot get that example to work

for simplicity I'll paste the example I'm referring to here:

"requests": {
        "pageview": "https://api.mixpanel.com/track/?data=${mp|json|base64}&ip=1&verbose=1"
      },
      "vars":{
        "mp":{
          "event": "Some Event",
          "properties":{
            "token": "MyToken",
            "distinct_id": "CLIENT_ID(mp_mixpanel)",
            "app": "MyApp",
            "amp": "true"
          }
        }
      },

[calebcordry]

@a-warner there are a couple things you should know:

1. Since this just got merged yesterday it will probably be about a week until you are able to test the experiment when using the code from our CDN. If you want to start immediately you could clone the repo and start experimenting that way.

2. There is a new syntax (docs are coming). For your example I think you'll want this

"requests": {
        "pageview": "https://api.mixpanel.com/track/?data=BASE64(JSON(${mp}))&ip=1&verbose=1"
      },
    ...

Feel free to ping me on our slack channel if you want more help with your specific case.

[a-warner]

thanks @calebcordry this is really helpful, just a bit eager to get going 😃. I'll check it out next week

[rudygalfi]

@calebcordry Is there an issue tracking adding documentation?

[lannka]

Remaining tasks to launch the feature is tracked here: #13371