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

telemetry: add more fields to data #8067

Merged
merged 1 commit into from
Feb 17, 2022
Merged

telemetry: add more fields to data #8067

merged 1 commit into from
Feb 17, 2022

Conversation

Pothulapati
Copy link
Contributor

@Pothulapati Pothulapati commented Feb 7, 2022

Description

This PR updates the installation-admin-controller to also retrieve
more data to send with telemetry. These are not part of the
installationAdminDb as we do not want to store this in the database
but lazily retrieve whenever a request is sent to /data endpoint
of the installation-admin express app unlike the uuid and settings
which need to be stored and updated.

The following fields are added:

  • totalUsers : specifies the total number of users in the instance
  • totalWorkspaces: specifies the total number of regular workspaces in the instance
  • totalInstances: specifies the total number of regular workspace instances in the gitpod instance

Related Issue(s)

Fixes #7866

How to test

Deploy via werft and do a /data GET on the 9000 port of server.

  1. Run yarn telepresence in /server
  2. Run kubectl port-forward svc/server 9000
  3. Run curl -H "Content-Type: application/json" http://localhost:9000/data

Sample Output:

{"installationAdmin":{"id":"79f821ea-eee8-42cb-adff-1828b75656dd","settings":{"sendTelemetry":true}},"totalUsers":3,"totalWorkspaces":1,"totalInstances":93}

Release Notes

[server]: Add `totalUsers`, `totalWorkspaces`, and `totalInstances` fields to telemetry data

Documentation

@Pothulapati Pothulapati requested a review from a team February 7, 2022 11:44
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Feb 7, 2022
@Pothulapati Pothulapati requested a review from a team February 7, 2022 11:44
@github-actions github-actions bot added the team: delivery Issue belongs to the self-hosted team label Feb 7, 2022
@@ -23,6 +23,11 @@ export interface InstallationAdmin {
settings: InstallationAdminSettings;
}

export interface Data {
installationAdmin: InstallationAdmin
totalUsers: number
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we add more telemetry data, We could continue adding them here thereby separating it from the InstallationAdmin (which integrates into the database).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same goes with the new Data struct for the golang side of things in installation-telemetry workspace

@codecov
Copy link

codecov bot commented Feb 7, 2022

Codecov Report

Merging #8067 (7c89847) into main (72b0017) will increase coverage by 0.65%.
The diff coverage is 4.91%.

❗ Current head 7c89847 differs from pull request most recent head 7547d11. Consider uploading reports for the commit 7547d11 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8067      +/-   ##
==========================================
+ Coverage   10.20%   10.86%   +0.65%     
==========================================
  Files          18       18              
  Lines        1009     1022      +13     
==========================================
+ Hits          103      111       +8     
- Misses        905      909       +4     
- Partials        1        2       +1     
Flag Coverage Δ
components-gitpod-cli-app 10.86% <4.91%> (+0.65%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/gitpod-cli/cmd/credential-helper.go 5.58% <4.91%> (+4.38%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2214706...7547d11. Read the comment docs.

@Pothulapati
Copy link
Contributor Author

Still there is work on wiring these metrics into the installation-telemetry job! Will update once that is done!

@roboquat roboquat added size/M and removed size/S labels Feb 7, 2022
Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Looks good. 👍

@Pothulapati Pothulapati changed the title telemetry: add totalUsers field to data telemetry: add more fields to data Feb 14, 2022
Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for the good work! 👍

@corneliusludmann
Copy link
Contributor

@Pothulapati Since we rebase and merge, it would be good when you can clean up your commits and squash them into one commit. ❤️

Fixes #7866

This PR updates the `installation-admin-controller` to also retrieve
more data to send with telemetry. These are not part of the
`installationAdminDb` as we do not want to store this in the database
but lazily retrieve whenever a request is sent to `/data` endpoint
of the `installation-admin` express app unlike the `uuid` and settings
which need to be stored and updated.

The following fields are added:
- `totalUsers` : specifies the total number of users in the instance
- `totalWorkspaces`: specifies the total number of **regular** workspaces in the instance
- `totalInstances`: specifies the total number of **regular** workspace instances in the gitpod instance

Signed-off-by: Tarun Pothulapati <[email protected]>
@Pothulapati Pothulapati force-pushed the tarun/telemetry-users branch from 7c89847 to 7547d11 Compare February 17, 2022 11:00
@Pothulapati
Copy link
Contributor Author

@corneliusludmann Done!

Copy link
Contributor

@corneliusludmann corneliusludmann left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks!

@corneliusludmann corneliusludmann removed the request for review from a team February 17, 2022 11:05
@roboquat roboquat merged commit 5f6114c into main Feb 17, 2022
@roboquat roboquat deleted the tarun/telemetry-users branch February 17, 2022 11:08
@Pothulapati Pothulapati mentioned this pull request Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note size/M team: delivery Issue belongs to the self-hosted team team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add number of users to the telemetry data
4 participants