Add filter support for analytics vars #2198

Open
cramforce opened this Issue Feb 22, 2016 · 13 comments

Projects

None yet

8 participants

@cramforce
Member

Something like
${clientId(amp_id)|toLowerCase}

E.g. Adobe Analytics does not like - (dashes) in client ids, which could be enforced with such a mechanism.

@cramforce cramforce self-assigned this Feb 22, 2016
@rudygalfi rudygalfi added this to the Up Next milestone Mar 4, 2016
@ryanlombardo

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

@cramforce cramforce removed their assignment May 23, 2016
@rudygalfi
Contributor

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

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

@rudygalfi rudygalfi modified the milestone: Backlog, Next Aug 25, 2016
@avimehta
Collaborator
avimehta commented Aug 25, 2016 edited

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
@avimehta avimehta self-assigned this Aug 25, 2016
@rudygalfi
Contributor

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
Collaborator

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

@avimehta
Collaborator

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
Collaborator

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

@avimehta
Collaborator

@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
Collaborator

Having some concerns over additional syntax.

@georgecrawford

I originally raised #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
Contributor

@avimehta has been working on this and could provide updates

@taymonbeal
Collaborator

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

@avimehta avimehta added a commit to avimehta/amphtml that referenced this issue Oct 14, 2016
@avimehta avimehta Added initial version of amp-anlaytics variable filters.
Fixes #2198
e479159
@avimehta avimehta added a commit to avimehta/amphtml that referenced this issue Oct 14, 2016
@avimehta avimehta Added initial version of amp-anlaytics variable filters.
Fixes #2198
a1e612d
@avimehta avimehta added a commit to avimehta/amphtml that referenced this issue Nov 18, 2016
@avimehta avimehta Added initial version of amp-anlaytics variable filters.
Fixes #2198
f9644eb
@avimehta avimehta added a commit to avimehta/amphtml that referenced this issue Nov 18, 2016
@avimehta avimehta Added initial version of amp-anlaytics variable filters.
Fixes #2198
16ccc7a
@avimehta avimehta added a commit to avimehta/amphtml that referenced this issue Nov 21, 2016
@avimehta avimehta Added initial version of amp-anlaytics variable filters.
Fixes #2198
77a0c95
@avimehta avimehta added a commit to avimehta/amphtml that referenced this issue Nov 21, 2016
@avimehta avimehta Added initial version of amp-anlaytics variable filters.
Fixes #2198
0191721
@avimehta avimehta added a commit to avimehta/amphtml that referenced this issue Dec 1, 2016
@avimehta avimehta Added initial version of amp-anlaytics variable filters.
Fixes #2198
304536b
@avimehta avimehta closed this in #5621 Dec 7, 2016
@avimehta avimehta added a commit that referenced this issue Dec 7, 2016
@avimehta avimehta Added initial version of amp-anlaytics variable filters. (#5621)
* Added initial version of amp-anlaytics variable filters.

Fixes #2198
65177f8
@jridgewell jridgewell reopened this Dec 8, 2016
@jridgewell
Member

Reverted in #6542.

@Lith Lith added a commit to Lith/amphtml that referenced this issue Dec 22, 2016
@avimehta @Lith avimehta + Lith Added initial version of amp-anlaytics variable filters. (#5621)
* Added initial version of amp-anlaytics variable filters.

Fixes #2198
8f9f715
@Lith Lith added a commit to Lith/amphtml that referenced this issue Dec 22, 2016
@avimehta @Lith avimehta + Lith Added initial version of amp-anlaytics variable filters. (#5621)
* Added initial version of amp-anlaytics variable filters.

Fixes #2198
02f7b4c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment