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

Replace canonicalUrl with ampdocUrl in Nielsen analytics #26278

Merged
merged 3 commits into from
Jan 10, 2020

Conversation

uniquelygeneric
Copy link
Contributor

Updated Nielsen analytics to utilize ampdocUrl in query param.

@uniquelygeneric
Copy link
Contributor Author

@alanorozco Could you please restart the Travis job? Thanks!

@alanorozco
Copy link
Member

@uniquelygeneric Restarted shards. Current reviewer @calebcordry also has restart powers FYI

@calebcordry
Copy link
Member

@uniquelygeneric the test failures are real, can you also update

"nielsen": {
"session": "https://!prefixuaid-linkage.imrworldwide.com/cgi-bin/gn?prd=session&c13=asid,P!apid&sessionId=_client_id___page_view_id_&pingtype=4&enc=false&c61=createtm,_timestamp_&rnd=_random_",
"cloudapi": "https://!prefixcloudapi.imrworldwide.com/nmapi/v2/!apid/_client_id___page_view_id_/a?b=%7B%22devInfo%22%3A%7B%22devId%22%3A%22_client_id___page_view_id_%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_page_view_id_%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_background_state_%22%2C%22blur%22%3A%22_background_state_%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%221%22%2C%22pageURL%22%3A%22_canonical_url_%22%7D"
},

This should fix the broken test. Thanks!

@uniquelygeneric
Copy link
Contributor Author

@calebcordry whoops, forgot to update there, too. Thanks!

Are we good now?

Copy link
Member

@calebcordry calebcordry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for the continued maintenance of this impl!

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

Successfully merging this pull request may close these issues.

None yet

5 participants