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

clean up analytics in footer #22998

Merged
merged 1 commit into from
Aug 13, 2020

Conversation

kbhawkey
Copy link
Contributor

@kbhawkey kbhawkey commented Aug 6, 2020

See issue #22819 .

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 6, 2020
@k8s-ci-robot k8s-ci-robot added sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 6, 2020
@kbhawkey kbhawkey changed the title clean up analytics in footer [wip]clean up analytics in footer Aug 6, 2020
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 6, 2020
@netlify
Copy link

netlify bot commented Aug 6, 2020

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 0898f1c

https://deploy-preview-22998--kubernetes-io-master-staging.netlify.app

@sftim
Copy link
Contributor

sftim commented Aug 6, 2020

/area web-development

@k8s-ci-robot k8s-ci-robot added the area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes label Aug 6, 2020
@tengqm
Copy link
Contributor

tengqm commented Aug 7, 2020

At this stage, if the link target is not working properly, commenting the link out might be the best option.
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 7, 2020
@kbhawkey kbhawkey changed the title [wip]clean up analytics in footer clean up analytics in footer Aug 8, 2020
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 8, 2020
@kbhawkey
Copy link
Contributor Author

kbhawkey commented Aug 8, 2020

I'd like more information about the purpose of this pixel with regard to the site analytics.

@tengqm
Copy link
Contributor

tengqm commented Aug 8, 2020

@kbhawkey, I may be wrong but my theory is that this link is used to count page visits. On the link target, you can sum up the visits by checking the page referrer.

@sftim
Copy link
Contributor

sftim commented Aug 8, 2020

/approve

/hold
Feel free to /unhold

Is this the tool in use? https://github.com/igrigorik/ga-beacon

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 8, 2020
@kbhawkey
Copy link
Contributor Author

kbhawkey commented Aug 9, 2020

I don't know about the "ga-beacon" tool, but the description of the tool fits.
Could investigate updating the tool to Go 1.12+ or find another way/tool?

@shuuji3
Copy link
Contributor

shuuji3 commented Aug 9, 2020

It seems that this update has already done on June 24 by this PR: igrigorik/ga-beacon/pull/69. So just fetching source code and redeploy it to App Engine is enough?

@sftim
Copy link
Contributor

sftim commented Aug 9, 2020

@celestehorgan do you have any context about that beacon and its operation?

@kbhawkey
Copy link
Contributor Author

kbhawkey commented Aug 9, 2020

It seems that this update has already done on June 24 by this PR: igrigorik/ga-beacon/pull/69. So just fetching source code and redeploy it to App Engine is enough?

Great! @zacharysarah @jimangel @kbarnard10 do you have insight into updating the docs code to GAE?

@celestehorgan
Copy link
Contributor

@sftim no I don't, unfortunately. Judging by its position (not in the footer but specifically in .issue-button-container) I wonder if this is specifically tracking click-through's to GitHub?

I don't have access to the Analytics so I can't say more.

@jimangel
Copy link
Member

I went deep down this rabbit hole... It looks like something that was implemented around 2015 as a way to track analytics of GitHub markdown files (the literal files when viewed on GitHub). You can read more about ga-beacon in the repo linked above. Also, looking at the file's git history and following it back to the earliest file, it came over as part of the Jekyll migration.

From everything I can find, this was somewhat an early attempt at gaining analytics that are not commonly known on GitHub.

Given that we still implement the Google Analytics JS over the entire site and the fact that this GAE app is broken (and not intended to improve the website). I think it's safe to remove.

I would prefer the entire section gets deleted vs. commented out.

/cc @zacharysarah

@celestehorgan
Copy link
Contributor

+1 to removing this completely. Thanks for the research, @jimangel!

@kbhawkey
Copy link
Contributor Author

That's fine. I will update.

@kbhawkey kbhawkey force-pushed the kb-analytics-footer-pixel branch from eb176f4 to 0898f1c Compare August 12, 2020 23:01
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 12, 2020
@celestehorgan
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 12, 2020
@jimangel
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jimangel, sftim

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

The pull request process is described here

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

@jimangel
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 13, 2020
@k8s-ci-robot k8s-ci-robot merged commit c8e43db into kubernetes:master Aug 13, 2020
@kbhawkey kbhawkey deleted the kb-analytics-footer-pixel branch June 23, 2021 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/web-development Issues or PRs related to the kubernetes.io's infrastructure, design, or build processes cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. 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.

7 participants