Skip to content

[server] Track snapshot access requests, and whether they are granted or denied #8374

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

Merged
merged 1 commit into from
Feb 22, 2022

Conversation

jankeromnes
Copy link
Contributor

@jankeromnes jankeromnes commented Feb 22, 2022

Description

Track snapshot access requests, and whether they are granted or denied.

Related Issue(s)

Fixes #8340

How to test

  1. Verify that the analytics messages are being sent (but I'm not sure how to do this?)

Release Notes

NONE

Documentation

/werft analytics=segment|TEZnsG4QbLSxLfHfNieLYGF4cDwyFWoe

@jankeromnes jankeromnes requested a review from a team February 22, 2022 07:33
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Feb 22, 2022
@jakobhero
Copy link
Contributor

jakobhero commented Feb 22, 2022

/werft run

👍 started the job as gitpod-build-jx-track-snapshot-access.1

@codecov
Copy link

codecov bot commented Feb 22, 2022

Codecov Report

Merging #8374 (06af14b) into main (244b40f) will decrease coverage by 1.13%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8374      +/-   ##
==========================================
- Coverage   12.31%   11.17%   -1.14%     
==========================================
  Files          20       18       -2     
  Lines        1161      993     -168     
==========================================
- Hits          143      111      -32     
+ Misses       1014      880     -134     
+ Partials        4        2       -2     
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?

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

Impacted Files Coverage Δ
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go

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 244b40f...06af14b. Read the comment docs.

@jldec
Copy link
Contributor

jldec commented Feb 22, 2022

Thanks for adding this @jankeromnes

@roboquat roboquat merged commit ec19a3f into main Feb 22, 2022
@roboquat roboquat deleted the jx/track-snapshot-access branch February 22, 2022 07:54
@@ -836,6 +836,8 @@ export class GitpodServerImpl implements GitpodServerWithTracing, Disposable {
]);

if (SnapshotContext.is(context)) {
// TODO(janx): Remove snapshot access tracking once we're certain that enforcing repository read access doesn't disrupt the snapshot UX.
this.trackEvent(ctx, { event: "snapshot_access_request", properties: { snapshot_id: context.snapshotId } }).catch();
Copy link
Contributor

Choose a reason for hiding this comment

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

we try to bundle track calls that follow the same context in one call and have differences contained in the properties. in order to follow this here, it would make sense to give the same name to all three track calls (e.g. snapshot_access; we try to follow an object_verb naming convention from the perspective of the user, feel free to come up with something here that follows it more closely). then, you can add a property such as type that contains whether the event is requested, granted or denied so that we still capture the same level of information.

else, this looks good to me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note-none size/XS team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Track snapshot usage with success and access-error events
4 participants