Skip to content
This repository was archived by the owner on Dec 15, 2021. It is now read-only.

Update config.toml#252

Closed
omerbensaadon wants to merge 1 commit into
knative:mainfrom
omerbensaadon:patch-1
Closed

Update config.toml#252
omerbensaadon wants to merge 1 commit into
knative:mainfrom
omerbensaadon:patch-1

Conversation

@omerbensaadon
Copy link
Copy Markdown

@omerbensaadon omerbensaadon commented Feb 18, 2021

Changing Analytics to a knative.team owned account

Changing Analytics to a knative.team owned account 

Please DO NOT MERGE until we are certain we cannot transfer ownership of the existing Google Analytics account. 👍🏼
@google-cla google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label Feb 18, 2021
@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign markusthoemmes after the PR has been reviewed.
You can assign the PR to them by writing /assign @markusthoemmes in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 18, 2021
@omerbensaadon
Copy link
Copy Markdown
Author

/hold

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 18, 2021
@omerbensaadon
Copy link
Copy Markdown
Author

/unhold

@knative-prow-robot knative-prow-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Feb 19, 2021
@omerbensaadon
Copy link
Copy Markdown
Author

Closes knative/community#89 (comment)

@omerbensaadon
Copy link
Copy Markdown
Author

/unhold

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 19, 2021
@evankanderson
Copy link
Copy Markdown
Member

Is this ready to be approved?

/assign @evankanderson

@omerbensaadon
Copy link
Copy Markdown
Author

omerbensaadon commented Feb 23, 2021 via email

@RichieEscarez RichieEscarez added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 23, 2021
@RichieEscarez
Copy link
Copy Markdown
Contributor

Sorry I missed the meeting where this was all discussed but I believe (from my exchange with April) that we need to have both Google Analytics accounts tied to the site.

@vaikas
Copy link
Copy Markdown
Contributor

vaikas commented Feb 23, 2021

@RichieEscarez @thisisnotapril what does that mean in practical terms? Just trying to understand the need for it as well as what impact does this have? Is this akin to mirroring the data into Google analytics account (for account restrictions, or something I'd reckon?). Also does this mean that we wouldn't lose the old data? Again, just trying to understand :)

@RichieEscarez
Copy link
Copy Markdown
Contributor

To move forward at this point, someone would need to test and get Analytics working for both account IDs.

WRT restrictions: Basically, external access to the internal Google tool is not allowed. That includes exporting all data from there. I can however extract anonymous data and share that like ive done for past requests. Here are those reports: https://drive.google.com/corp/drive/folders/16ozmyon22Uz2K0il32-gTEaLR4xyv7zM

Per the Knative.dev privacy policy (the same standard Google Privacy policy), the current internal Analytics account only contains data for the last 14 mns (everything before that has been automatically deleted): https://support.google.com/analytics/answer/7667196?hl=en

Do let me know if there are other metrics that you want from the last 14 mns and I can upload that report to the shared drive.

Per the two different Google Analytics accounts, I think that was something that came out of the TOC - add a separate account for Knative members.

@omerbensaadon
Copy link
Copy Markdown
Author

Ack on all above, will reach out to you w/ requests sometime in the next few weeks...

I was under the impression that it was to move the analytics from a Google-owned account to a community-owned account...?

@csantanapr
Copy link
Copy Markdown
Member

My impression was the same that there was going to be a single analytics account owned by the community under Knative team Google account and not longer Google owned

this way we have access to the analytics tools and we can get to the data directly fro. The dashboard makes it easier to filter and analyze with a web app instead of screenshots

@vaikas
Copy link
Copy Markdown
Contributor

vaikas commented Feb 26, 2021

@thisisnotapril @csantanapr was there anything else that we needed here except add both IDs?

@RichieEscarez
Copy link
Copy Markdown
Contributor

RichieEscarez commented Feb 27, 2021

Adding status/notes here incase it might help.

After spending a little time researching this. It looks like default Hugo/Docsy will give us this today (using this config.yaml file):

<script type="application/javascript">
 var doNotTrack = false;
 if (!doNotTrack) {
 window.ga=window.ga\|\|function(){(ga.q=ga.q\|\|[]).push(arguments)};ga.l=+new Date;
 ga('create', 'UA-148338258-1', 'auto');
 ga('send', 'pageview');
 }
 </script>
 <script async src='https://www.google-analytics.com/analytics.js'></script>

Per the instructions here: https://developers.google.com/analytics/devguides/collection/analyticsjs/display-features#using-multiple-trackers

We need to modify our site template to give us this instead (both IDs and the require elements too):

// Google ID: UA-148338258-1
ga('create', 'UA-148338258-1', {name: 'google'});
ga('google.require', 'displayfeatures');
ga('google.send', 'pageview');

// Knative ID: G-YRMNFYE32R
ga('create', 'G-YRMNFYE32R', {name: 'knative'});
ga('knative.require', 'displayfeatures');
ga('knative.send', 'pageview');

Looking at how Hugo injects the Id in the config.yaml, we might need to create our own custom shortcode to be able to specify the required parameters for using multiple ids (ga('google.require', 'displayfeatures'); and ga('knative.require', 'displayfeatures');).

Reference: https://gohugo.io/templates/internal/#google-analytics

@csantanapr
Copy link
Copy Markdown
Member

@vaikas what we need is to include the 2 account IDs to Hugo

thanks @RichieEscarez

@csantanapr
Copy link
Copy Markdown
Member

@RichieEscarez I proposed that we close this PR and that you do the changes to add the 2 Google analytics IDs.

Based on what you shared it looks like you have the knowledge on what needs to be done
#252 (comment)

Adding status/notes here incase it might help.

After spending a little time researching this. It looks like default Hugo/Docsy will give us this today (using this config.yaml file):

<script type="application/javascript"> var doNotTrack = false; if (!doNotTrack) { window.ga=window.ga\|\|function(){(ga.q=ga.q\|\|[]).push(arguments)};ga.l=+new Date; ga('create', 'UA-148338258-1', 'auto'); ga('send', 'pageview'); } </script> <script async src='https://www.google-analytics.com/analytics.js'></script>

Per the instructions here: https://developers.google.com/analytics/devguides/collection/analyticsjs/display-features#using-multiple-trackers

We need to modify our site template to give us this instead (both IDs and the require elements too):

// Google ID: UA-148338258-1
ga('create', 'UA-148338258-1', {name: 'google'});
ga('google.require', 'displayfeatures');
ga('google.send', 'pageview');

// Knative ID: G-YRMNFYE32R
ga('create', 'G-YRMNFYE32R', {name: 'knative'});
ga('knative.require', 'displayfeatures');
ga('knative.send', 'pageview');
Looking at how Hugo injects the Id in the config.yaml, we might need to create our own custom shortcode to be able to specify the required parameters for using multiple ids (ga('google.require', 'displayfeatures'); and ga('knative.require', 'displayfeatures');).

Reference: https://gohugo.io/templates/internal/#google-analytics

@RichieEscarez
Copy link
Copy Markdown
Contributor

I can prioritize this over the "docs contributor" docs but schedule wise, I won't have bandwidth until next week. When I do start, ill mark this close and send out a PR.

(FYI, our Hugo template site generator is Golang based so that might help other folks here if interested, and if someone does get to this first, here is the page about creating shortcodes - also see our other Knative custom shortcodes.)

Base automatically changed from master to main March 8, 2021 17:38
@RichieEscarez
Copy link
Copy Markdown
Contributor

Replacing this PR with #262

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes Indicates the PR's author has signed the CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants