-
Notifications
You must be signed in to change notification settings - Fork 1.3k
installation-telemetry: add customer ID #10629
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
Conversation
This looks like the telemetry data provider needs to be relocated into |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great! Great work Adrien! 🤩
Hey @adrienthebo, we originally discussed two implementation paths here - one with the license ID, and one with a newly added customer ID as a (hidden) field in the replicated license. We had plan was to use the customer ID (reasoning was outlined in this comment) but I had failed to transfer this state into the main issue description (Sorry!). Is it possible to switch the implementation to use customer id instead of the license ID? |
3dbbc1d
to
1d1007d
Compare
started the job as gitpod-build-alt-10183-sh-telem-license.7 because the annotations in the pull request description changed |
started the job as gitpod-build-alt-10183-sh-telem-license.8 because the annotations in the pull request description changed |
started the job as gitpod-build-alt-10183-sh-telem-license.9 because the annotations in the pull request description changed |
This looks good - two things that are missing as per the original ticket:
|
Just my 2 cents: Given that the customer ID is included only when we explicitly add it to the license (and we could ask the customers for consent before we do this), in my option we could add this in a separate PR (especially when we want to give George some time to give his input). What do you think? |
536cdbd
to
f48df1f
Compare
6394f9d
to
85f898f
Compare
@adrienthebo I was looking to review, but couldn't because the build seems to fail. 😕 |
@geropl I'm rebasing onto |
85f898f
to
0fcb0b9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tested and works as expected 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/unhold |
Came back a bit late for your comment[1] @lucasvaltl as I was offline for a week, but left some feedback in a relevant discussion (internal). FWIW, using a second checkbox sounds like a good minimal viable change. Also, thanks @adrienthebo for shipping this and including a helpful section description[1] below the section title that also provides some visual balance. ✨ |
Description
This pull request exposes the GitPod Customer ID field in installation telemetry.
Related Issue(s)
This issue resolves #10183.
How to test
The
customerID
field will be available in the Gitpod Admin/Settings page, displayed in the installation-telemetry job output, and will be present in the segment.io telemetry.Telemetry Disabled
Dashboard: Admin/Settings:
Installation telemetry job:
Telemetry enabled, Customer ID disabled
Dashboard: Admin/Settings:
Installation Telemetry job:
Telemetry enabled, Customer ID enabled, empty customer ID
Dashboard: Admin/Settings:

Installation Telemetry job:
Installation telemetry enabled, customer ID enabled, customer ID set
Dashboard: Settings/Admin:
Installation telemetry job:
Release Notes
Documentation
This change is user facing, and when enabled adds information that removes the pseudononymous guarantees in our documentation. We'll amend that in a separate pull request against the website.