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

fix: add telemetry reporting #100

Merged
merged 4 commits into from
Apr 17, 2024

Conversation

dgolovin
Copy link
Collaborator

@dgolovin dgolovin commented Apr 9, 2024

Fix #43.

@dgolovin dgolovin requested a review from a team as a code owner April 9, 2024 04:27
src/extension.ts Outdated Show resolved Hide resolved
@dgolovin dgolovin requested a review from slemeur April 9, 2024 16:04
src/podman-cli.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
src/extension.ts Outdated Show resolved Hide resolved
src/extension.ts Show resolved Hide resolved
}

TelemetryLogger.logUsage('signin', telemetryData);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we're missing lot of data in telemetryData

here it's only successful yes/no

I would expect the time it has taken, in case of failure, which step succeed/failed.

did we have to restart and how many restart

at which step the user stopped the flow

for example it's different if user aborted if no podman machine was running vs error in doing the task
did we have a PodmanMachineRunning ?

was isSubscriptionManagerInstalled already installed ?

Was it registryAccess part, vmActivation part

for each step, the time it took as well.

Then about the session, probably need some data (about the user being authenticated
I don't know what data we have but maybe number about some fields of the user (counter)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@benoitf @slemeur would be possible to fix all of that as follow up issues? I have added part of it and working on tests right now, but it will take some time to implement.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that's fair. We had a meeting on the telemetry issue (#43 (comment)) where these details did not come up. So I guess we under-specified the requirements initially.

Merging the PR and trying to get as far as possible until the deadline is OK for me.

Signed-off-by: Denis Golovin <[email protected]>
@dgolovin dgolovin merged commit c95a793 into redhat-developer:main Apr 17, 2024
4 checks passed
@dgolovin dgolovin deleted the i43-send-telemetry branch April 17, 2024 23:32
@dgolovin
Copy link
Collaborator Author

@benoitf

I think we're missing lot of data in telemetryData
I would expect the time it has taken, in case of failure, which step succeed/failed.

did we have to restart and how many restart

It is always one restart if needed at all, I can send start and stop command outcome.

at which step the user stopped the flow

User cannot stop the flow right now, it keeps going until it is finished with failure or success.

for example it's different if user aborted if no podman machine was running vs error in doing the task
did we have a PodmanMachineRunning ?

I have added error messages to telemetry for registry configuration and subscription activation

was isSubscriptionManagerInstalled already installed ?

Follow up issue #113 created

Was it registryAccess part, vmActivation part

added field with error message for each part

for each step, the time it took as well.

Follow up issue #114 created

Then about the session, probably need some data (about the user being authenticated
I don't know what data we have but maybe number about some fields of the user (counter)

I need clarification for this. What king of tracking we want to do here. Number of logins for specific user? Session duration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Send telemetry when creating container image registry service accounts and subscription activation keys
5 participants