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

Add telemetry support #498

Merged
merged 10 commits into from
Feb 11, 2021
Merged

Add telemetry support #498

merged 10 commits into from
Feb 11, 2021

Conversation

sudhirverma
Copy link
Contributor

@sudhirverma sudhirverma commented Feb 8, 2021

Fix: #470

@sudhirverma sudhirverma changed the title Add telemetry support Add telemetry support [WIP] Feb 8, 2021
@sudhirverma sudhirverma changed the title Add telemetry support [WIP] Add telemetry support Feb 8, 2021
src/extension.ts Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Feb 8, 2021

Codecov Report

Merging #498 (d2b9440) into master (3282692) will increase coverage by 0.19%.
The diff coverage is 95.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #498      +/-   ##
==========================================
+ Coverage   67.20%   67.39%   +0.19%     
==========================================
  Files          79       80       +1     
  Lines        5327     5365      +38     
  Branches      934      941       +7     
==========================================
+ Hits         3580     3616      +36     
- Misses       1747     1749       +2     
Impacted Files Coverage Δ
src/extension.ts 90.36% <91.66%> (+0.22%) ⬆️
src/telemetry.ts 100.00% <100.00%> (ø)
src/util/tknversion.ts 100.00% <100.00%> (ø)

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 3282692...d2b9440. Read the comment docs.

@evidolob
Copy link
Collaborator

evidolob commented Feb 9, 2021

@sudhirverma I got this error when try to launch extension in dev mode:
Screenshot 2021-02-09 at 11 05 11
Screenshot 2021-02-09 at 11 04 52

@sudhirverma
Copy link
Contributor Author

@sudhirverma I got this error when try to launch extension in dev mode:
Screenshot 2021-02-09 at 11 05 11
Screenshot 2021-02-09 at 11 04 52

I have launch extension in my mac and ubuntu system it is working fine for me.

@evidolob
Copy link
Collaborator

evidolob commented Feb 9, 2021

@sudhirverma Try to delete redhat.telemetry.enabled in preferences and relaunch extension

@sudhirverma
Copy link
Contributor Author

redhat.telemetry.enabled

I think we can't delete the redhat.telemetry.enabled from preferences -> settings

@evidolob
Copy link
Collaborator

evidolob commented Feb 9, 2021

Why? I try to launch extension with telemetry first time, so I do not have that in my preferences

@sudhirverma
Copy link
Contributor Author

Why? I try to launch extension with telemetry first time, so I do not have that in my preferences

I had remove and install it again. I didn’t get the error.

@evidolob
Copy link
Collaborator

evidolob commented Feb 9, 2021

That strange, I rebuild everything again and it start working

src/extension.ts Outdated
telemetryProps.error = result.error.toString();
sendTelemetry('command', telemetryProps);
} else {
if (commandId === 'kubectl.version') {
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 we need same for tkn versions, I mean that we need to store each of tkn cli, tekton version and triggers versions as separate values, that will help us to analyze used versions more easy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when we store tkn version it looks like Client version: 0.15.0 Pipeline version: v0.16.3 Triggers version: v0.8.1 which is similar to what I am doing with kubectl.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, that what I was thinking, if we store all tekton related versions in one row. The in case if we want to measure how much of our users use some specific Pipeline version it may be a bit difficult.

Copy link
Contributor Author

@sudhirverma sudhirverma Feb 10, 2021

Choose a reason for hiding this comment

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

You mean we should send the tkn related version separately? for Client, Pipeline, and trigger.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not 100% sure, I think you can ask Fred how to do it better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fbricon What's your opinion on sending data for tkn version it contains Client, Pipeline, and trigger version for example see in image. Should we send them separately?

Screenshot 2021-02-10 at 1 40 36 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, that's 3 properties

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.

Add telemetry reporting
4 participants