Add support for Nielsen analytics #9356
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
@avimehta, can you help identify what the Travis error is? It says there is an unexpected token in the examples file, but I don't see anything wrong with the formatting. |
| }, | ||
| 'request': 'cloudapi', | ||
| }, | ||
| 'transport': { |
There was a problem hiding this comment.
you're missing an closing } here
There was a problem hiding this comment.
Thanks for catching this!
| 'request': 'cloudapi', | ||
| }, | ||
| 'transport': { | ||
| 'beacon': false, |
There was a problem hiding this comment.
just want to check that your server does not support sendBeacon, which is recommended.
There was a problem hiding this comment.
Yes, this was on purpose. Our server only accepts GET requests.
| 'on': 'visible', | ||
| 'request': 'session', | ||
| }, | ||
| 'visible': { |
There was a problem hiding this comment.
you can merge this with init:
'request': ['session', 'cloudapi'],
| 'segC': 'NA', | ||
| }, | ||
| 'requests': { | ||
| 'session': 'https://uaid-linkage.imrworldwide.com/cgi-bin/gn?prd=session&c13=asid,P${apid}&sessionId=${clientId(imrworldwide)},&pingtype=4&enc=false&c61=createtm,${timestamp}&rnd=${random}', |
There was a problem hiding this comment.
for readability, can you also extract clientId to a var?
| 'cloudapi': 'https://cloudapi.imrworldwide.com/nmapi/v2/${apid}/${clientId(imrworldwide)}/a?b=%7B%22devInfo%22%3A%7B%22devId%22%3A%22${clientId(imrworldwide)}%22%2C%22apn%22%3A%22${apn}%22%2C%22apv%22%3A%22${apv}%22%2C%22apid%22%3A%22${apid}%22%7D%2C%22metadata%22%3A%7B%22static%22%3A%7B%22type%22%3A%22static%22%2C%22section%22%3A%22${section}%22%2C%22assetid%22%3A%22${pageViewId}%22%2C%22segA%22%3A%22${segA}%22%2C%22segB%22%3A%22${segB}%22%2C%22segC%22%3A%22${segC}%22%2C%22adModel%22%3A%220%22%2C%22dataSrc%22%3A%22cms%22%7D%2C%22content%22%3A%7B%7D%2C%22ad%22%3A%7B%7D%7D%2C%22event%22%3A%22playhead%22%2C%22position%22%3A%22${timestamp}%22%2C%22data%22%3A%7B%22hidden%22%3A%22${backgroundState}%22%2C%22blur%22%3A%22${backgroundState}%22%2C%22position%22%3A%22${timestamp}%22%7D%2C%22type%22%3A%22static%22%2C%22utc%22%3A%22${timestamp}%22%2C%22index%22%3A%22${requestCount}%22%7D', | ||
| }, | ||
| 'triggers': { | ||
| 'init': { |
| @@ -890,6 +890,45 @@ export const ANALYTICS_CONFIG = /** @type {!JSONType} */ ({ | |||
| }, | |||
| }, | |||
|
|
|||
| 'nielsen': { | |||
| 'vars': { | |||
| 'apid': '', | |||
There was a problem hiding this comment.
no need to specify. it's by default empty string
|
please fix the travis build. and also follow the googlebot to sign CLA |
|
@googlebot, CLA signed for Nielsen |
Try this. |
ghost
mentioned this pull request
|
I signed it! |
|
|
@lannka this was signed under kale.smith@nielsen.com on Nielsen's behalf. |
|
It should be either the code author or the corp sign it. Can you either sign it yourself or ask Kale to sign it using corp's name? |
|
@lannka I am Kale and I signed the contract with Nielsen's name...I'm not sure why it's not getting picked up by the bot |
|
I will investigate with CLA on this. |
|
@bpaduch The CLA should have been signed by the appropriate party now. Thanks! |
|
Can we get this one merged? |
|
Yes, we've received the signed copy of the CLA, so I don't see anything holding this back anymore. @bpaduch, is there anything I can do to help push this along? |
|
@uniquelygeneric - I'm waiting on response from the CLA team on status before proceeding. As soon as I get details, I'll advise. |
|
Hi @uniquelygeneric, the CLA for Nielsen is registered under the following group: |
|
@bpaduch I have been added to that list, but it could take 24-48 hours for your systems to update. Is there anything that can be done to ensure this doesn't delay the merge by another week? |
|
@erwinmombay: Can you force merge this? |
|
CLAs look good, thanks! |
ghost
merged commit 
In reference to ampproject/amphtml issue #3337.