✨Add requestCount, maxScrollDepth, and totalTime variables to Chartbeat vendor config #22849

joshschwartz · 2019-06-14T22:52:12Z

This PR adds three new variables to the Chartbeat vendor config to keep it in sync with our non-AMP scripts.

joshschwartz · 2019-06-14T22:54:44Z

Tagging @ampproject/wg-analytics for review

calebcordry

LGTM. I will try to keep an eye on travis but feel free to ping me when it finishes.

torch2424 · 2019-06-17T23:24:16Z

@calebcordry Restarted what may have been a flaky test for you 😄

calebcordry · 2019-06-18T19:24:18Z

extensions/amp-analytics/0.1/test/vendor-requests.json

@@ -54,7 +54,7 @@
  },
  "chartbeat": {
    "host": "https://ping.chartbeat.net",
-    "basePrefix": "/ping?h=!domain&p=_canonical_path_&u=_client_id_&d=_canonical_host_&g=!uid&g0=!sections&g1=!authors&g2=!zone&g3=!sponsorName&g4=!contentType&c=120&x=_scroll_top_&y=_scroll_height_&o=_scroll_width_&w=_viewport_height_&j=!decayTime&R=1&W=0&I=0&E=_total_engaged_time_&r=_document_referrer_&t=_page_view_id__client_id_&b=_page_load_time_&i=_title_&T=_timestamp_&tz=_timezone_&C=2",
+    "basePrefix": "/ping?h=!domain&p=_canonical_path_&u=_client_id_&d=_canonical_host_&g=!uid&g0=!sections&g1=!authors&g2=!zone&g3=!sponsorName&g4=!contentType&c=_total_time_&x=_scroll_top_&m=_max_scroll_depth_&y=_scroll_height_&o=_scroll_width_&w=_viewport_height_&j=!decayTime&R=1&W=0&I=0&E=_total_engaged_time_&r=_document_referrer_&t=_page_view_id__client_id_&b=_page_load_time_&i=_title_&T=_timestamp_&tz=_timezone_&sn=_request_count_&C=2",


_total_time_ => !totalTime
_max_scroll_depth_ => !maxScrollDepth
_request_count_ => 1

calebcordry

Looks like this test needs a few changes to pass. Here is the full config that I got working.

"chartbeat": {
    "host": "https://ping.chartbeat.net",
    "basePrefix": "/ping?h=!domain&p=_canonical_path_&u=_client_id_&d=_canonical_host_&g=!uid&g0=!sections&g1=!authors&g2=!zone&g3=!sponsorName&g4=!contentType&c=!totalTime&x=_scroll_top_&m=!maxScrollDepth&y=_scroll_height_&o=_scroll_width_&w=_viewport_height_&j=!decayTime&R=1&W=0&I=0&E=_total_engaged_time_&r=_document_referrer_&t=_page_view_id__client_id_&b=_page_load_time_&i=_title_&T=_timestamp_&tz=_timezone_&sn=1&C=2",
    "baseSuffix": "&_",
    "interval": "https://ping.chartbeat.net/ping?h=!domain&p=_canonical_path_&u=_client_id_&d=_canonical_host_&g=!uid&g0=!sections&g1=!authors&g2=!zone&g3=!sponsorName&g4=!contentType&c=!totalTime&x=_scroll_top_&m=!maxScrollDepth&y=_scroll_height_&o=_scroll_width_&w=_viewport_height_&j=!decayTime&R=1&W=0&I=0&E=_total_engaged_time_&r=_document_referrer_&t=_page_view_id__client_id_&b=_page_load_time_&i=_title_&T=_timestamp_&tz=_timezone_&sn=1&C=2&_",
    "anchorClick": "https://ping.chartbeat.net/ping?h=!domain&p=_canonical_path_&u=_client_id_&d=_canonical_host_&g=!uid&g0=!sections&g1=!authors&g2=!zone&g3=!sponsorName&g4=!contentType&c=!totalTime&x=_scroll_top_&m=!maxScrollDepth&y=_scroll_height_&o=_scroll_width_&w=_viewport_height_&j=!decayTime&R=1&W=0&I=0&E=_total_engaged_time_&r=_document_referrer_&t=_page_view_id__client_id_&b=_page_load_time_&i=_title_&T=_timestamp_&tz=_timezone_&sn=1&C=2&_"
  }

calebcordry · 2019-06-18T19:26:28Z

extensions/amp-analytics/0.1/test/vendor-requests.json

@@ -54,7 +54,7 @@
  },
  "chartbeat": {
    "host": "https://ping.chartbeat.net",
-    "basePrefix": "/ping?h=!domain&p=_canonical_path_&u=_client_id_&d=_canonical_host_&g=!uid&g0=!sections&g1=!authors&g2=!zone&g3=!sponsorName&g4=!contentType&c=120&x=_scroll_top_&y=_scroll_height_&o=_scroll_width_&w=_viewport_height_&j=!decayTime&R=1&W=0&I=0&E=_total_engaged_time_&r=_document_referrer_&t=_page_view_id__client_id_&b=_page_load_time_&i=_title_&T=_timestamp_&tz=_timezone_&C=2",
+    "basePrefix": "/ping?h=!domain&p=_canonical_path_&u=_client_id_&d=_canonical_host_&g=!uid&g0=!sections&g1=!authors&g2=!zone&g3=!sponsorName&g4=!contentType&c=_total_time_&x=_scroll_top_&m=_max_scroll_depth_&y=_scroll_height_&o=_scroll_width_&w=_viewport_height_&j=!decayTime&R=1&W=0&I=0&E=_total_engaged_time_&r=_document_referrer_&t=_page_view_id__client_id_&b=_page_load_time_&i=_title_&T=_timestamp_&tz=_timezone_&sn=_request_count_&C=2",
    "baseSuffix": "&_",
    "interval": "https://ping.chartbeat.net/ping?h=!domain&p=_canonical_path_&u=_client_id_&d=_canonical_host_&g=!uid&g0=!sections&g1=!authors&g2=!zone&g3=!sponsorName&g4=!contentType&c=120&x=_scroll_top_&y=_scroll_height_&o=_scroll_width_&w=_viewport_height_&j=!decayTime&R=1&W=0&I=0&E=_total_engaged_time_&r=_document_referrer_&t=_page_view_id__client_id_&b=_page_load_time_&i=_title_&T=_timestamp_&tz=_timezone_&C=2&_",


need to add these new vars to interval here and anchorClick below.

mrjoro · 2019-08-08T18:39:39Z

@joshschwartz, please make the test update @calebcordry described so we'll be able to get this in. Thanks!

micajuine-ho · 2019-12-23T19:48:26Z

extensions/amp-analytics/0.1/test/vendor-requests.json

-  },
+    "interval": "https://ping.chartbeat.net/ping?h=!domain&p=_canonical_path_&u=_client_id_&d=_canonical_host_&g=!uid&g0=!sections&g1=!authors&g2=!zone&g3=!sponsorName&g4=!contentType&c=!totalTime&x=_scroll_top_&m=!maxScrollDepth&y=_scroll_height_&o=_scroll_width_&w=_viewport_height_&j=!decayTime&R=1&W=0&I=0&E=_total_engaged_time_&r=_document_referrer_&t=_page_view_id__client_id_&b=_page_load_time_&i=_title_&T=_timestamp_&tz=_timezone_&sn=1&C=2&_",
+    "anchorClick": "https://ping.chartbeat.net/ping?h=!domain&p=_canonical_path_&u=_client_id_&d=_canonical_host_&g=!uid&g0=!sections&g1=!authors&g2=!zone&g3=!sponsorName&g4=!contentType&c=!totalTime&x=_scroll_top_&m=!maxScrollDepth&y=_scroll_height_&o=_scroll_width_&w=_viewport_height_&j=!decayTime&R=1&W=0&I=0&E=_total_engaged_time_&r=_document_referrer_&t=_page_view_id__client_id_&b=_page_load_time_&i=_title_&T=_timestamp_&tz=_timezone_&sn=1&C=2&_"
+  }


micajuine-ho

Hi @joshschwartz. We are currently in the process of migrating vendor's javascript files to json.

Can you add these changes to extensions/amp-analytics/0.1/vendors/chartbeat.json as well?

googlebot · 2020-01-14T15:48:01Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2020-01-14T16:07:16Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

tommcgurl · 2020-01-14T16:07:51Z

@googlebot I fixed it.

googlebot · 2020-01-14T16:38:51Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot · 2020-01-14T16:39:26Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

…new keys, add totalTime to c key

…s to new keys, add totalTime to c key

tommcgurl · 2020-01-14T21:53:51Z

Thanks for the review/help @calebcordry and @micajuine-ho 👍 . I've pushed the updates and it looks like everything is passing 🎉

tommcgurl · 2020-01-16T12:42:44Z

Tagging @micajuine-ho for review

joshschwartz · 2020-01-16T14:57:37Z

Sorry this has been open for so long! Should be all set, so tagging @ampproject/wg-analytics for review

googlebot added the cla: yes label Jun 14, 2019

calebcordry self-requested a review June 14, 2019 23:01

calebcordry added the WG: analytics label Jun 14, 2019

calebcordry approved these changes Jun 14, 2019

View reviewed changes

calebcordry reviewed Jun 18, 2019

View reviewed changes

calebcordry self-requested a review June 18, 2019 19:24

calebcordry requested changes Jun 18, 2019

View reviewed changes

calebcordry reviewed Jun 18, 2019

View reviewed changes

joshschwartz force-pushed the chartbeat_add_timer branch from 89fa70f to d1fa48e Compare September 17, 2019 14:18

micajuine-ho self-requested a review December 23, 2019 19:48

micajuine-ho suggested changes Dec 23, 2019

View reviewed changes

micajuine-ho self-requested a review January 9, 2020 19:31

micajuine-ho suggested changes Jan 9, 2020

View reviewed changes

tommcgurl force-pushed the chartbeat_add_timer branch from 9ec5e19 to c08afce Compare January 14, 2020 15:47

googlebot added cla: no and removed cla: yes labels Jan 14, 2020

tommcgurl force-pushed the chartbeat_add_timer branch 3 times, most recently from fe24855 to 0707572 Compare January 14, 2020 16:07

googlebot added cla: yes and removed cla: no labels Jan 14, 2020

googlebot added cla: no and removed cla: yes labels Jan 14, 2020

tommcgurl force-pushed the chartbeat_add_timer branch from bdea199 to 90cb938 Compare January 14, 2020 16:39

googlebot added cla: yes and removed cla: no labels Jan 14, 2020

tommcgurl force-pushed the chartbeat_add_timer branch from 90cb938 to d33a5c2 Compare January 14, 2020 16:56

micajuine-ho approved these changes Jan 14, 2020

View reviewed changes

Josh Schwartz and others added 5 commits January 14, 2020 14:57

Chartbeat vendor config: add totalTime and requestCount variables to …

74451ec

…new keys, add totalTime to c key

update tests to reflect new chartbeat vendor config keys

d16f3b4

fix tests for chartbeat config

0123841

fixup for tests

e379040

Chartbeat vendor json config: add totalTime and requestCount variable…

10c49f5

…s to new keys, add totalTime to c key

tommcgurl force-pushed the chartbeat_add_timer branch from d33a5c2 to f3fd874 Compare January 14, 2020 19:57

Update chartbeat.json

1bfdf83

tommcgurl force-pushed the chartbeat_add_timer branch from f3fd874 to 1bfdf83 Compare January 14, 2020 20:44

calebcordry approved these changes Jan 16, 2020

View reviewed changes

micajuine-ho merged commit e9a5c5a into ampproject:master Jan 16, 2020

ampprojectbot added the PR use: In Beta / Experimental label Jan 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨Add requestCount, maxScrollDepth, and totalTime variables to Chartbeat vendor config #22849

✨Add requestCount, maxScrollDepth, and totalTime variables to Chartbeat vendor config #22849

joshschwartz commented Jun 14, 2019

joshschwartz commented Jun 14, 2019

calebcordry left a comment

torch2424 commented Jun 17, 2019

calebcordry Jun 18, 2019

calebcordry left a comment

calebcordry Jun 18, 2019

mrjoro commented Aug 8, 2019

micajuine-ho Dec 23, 2019

micajuine-ho left a comment

googlebot commented Jan 14, 2020

googlebot commented Jan 14, 2020

tommcgurl commented Jan 14, 2020

googlebot commented Jan 14, 2020

googlebot commented Jan 14, 2020

tommcgurl commented Jan 14, 2020

tommcgurl commented Jan 16, 2020 •

edited

joshschwartz commented Jan 16, 2020

✨Add requestCount, maxScrollDepth, and totalTime variables to Chartbeat vendor config #22849

✨Add requestCount, maxScrollDepth, and totalTime variables to Chartbeat vendor config #22849

Conversation

joshschwartz commented Jun 14, 2019

joshschwartz commented Jun 14, 2019

calebcordry left a comment

Choose a reason for hiding this comment

torch2424 commented Jun 17, 2019

calebcordry Jun 18, 2019

Choose a reason for hiding this comment

calebcordry left a comment

Choose a reason for hiding this comment

calebcordry Jun 18, 2019

Choose a reason for hiding this comment

mrjoro commented Aug 8, 2019

micajuine-ho Dec 23, 2019

Choose a reason for hiding this comment

micajuine-ho left a comment

Choose a reason for hiding this comment

googlebot commented Jan 14, 2020

googlebot commented Jan 14, 2020

tommcgurl commented Jan 14, 2020

googlebot commented Jan 14, 2020

googlebot commented Jan 14, 2020

tommcgurl commented Jan 14, 2020

tommcgurl commented Jan 16, 2020 • edited

joshschwartz commented Jan 16, 2020

tommcgurl commented Jan 16, 2020 •

edited