-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[vscode] support TelemetryLogger #12453
[vscode] support TelemetryLogger #12453
Conversation
6bfba82
to
783274a
Compare
Thanks a lot for your review, @vince-fugnitto! I adressed most of the comments except the CQ one. I also rebased on the current master. |
783274a
to
90f459a
Compare
@vince-fugnitto The CQ has been approved, is this PR good to go from your POV or do you have other comments? |
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 👍
- the test plugin works as expected
- the code looks good, and feedback based on duplication from
@theia/core
has been addressed - the CQ has been approved
resolves eclipse-theia#12232 Contributed on behalf of STMicroelectronics Signed-off-by: Remi Schnekenburger <[email protected]>
Hi @vince-fugnitto, I rebased on latest master, and I fixed the 2 minor issues (missing new lines). |
Closes: eclipse-theia#12232 The commit adds support for the `TelemetryLogger` VS Code API in addition to other related functionality. Contributed on behalf of STMicroelectronics Signed-off-by: Remi Schnekenburger <[email protected]>
Closes: eclipse-theia#12232 The commit adds support for the `TelemetryLogger` VS Code API in addition to other related functionality. Contributed on behalf of STMicroelectronics Signed-off-by: Remi Schnekenburger <[email protected]>
Closes: eclipse-theia#12232 The commit adds support for the `TelemetryLogger` VS Code API in addition to other related functionality. Contributed on behalf of STMicroelectronics Signed-off-by: Remi Schnekenburger <[email protected]>
Closes: eclipse-theia#12232 The commit adds support for the `TelemetryLogger` VS Code API in addition to other related functionality. Contributed on behalf of STMicroelectronics Signed-off-by: Remi Schnekenburger <[email protected]>
Closes: eclipse-theia#12232 The commit adds support for the `TelemetryLogger` VS Code API in addition to other related functionality. Contributed on behalf of STMicroelectronics Signed-off-by: Remi Schnekenburger <[email protected]>
* vscode: add `TelemetryLogger` support (eclipse-theia#12453) Closes: eclipse-theia#12232 The commit adds support for the `TelemetryLogger` VS Code API in addition to other related functionality. Contributed on behalf of STMicroelectronics Signed-off-by: Remi Schnekenburger <[email protected]> --------- Signed-off-by: Remi Schnekenburger <[email protected]> Co-authored-by: Remi Schnekenburger <[email protected]>
* vscode: add `TelemetryLogger` support (eclipse-theia#12453) Closes: eclipse-theia#12232 The commit adds support for the `TelemetryLogger` VS Code API in addition to other related functionality. Contributed on behalf of STMicroelectronics Signed-off-by: Remi Schnekenburger <[email protected]> --------- Signed-off-by: Remi Schnekenburger <[email protected]> Co-authored-by: Remi Schnekenburger <[email protected]>
What it does
Fixes #12232
Add Telemetry support for extensions.
This adds the TelemetryLogger API and some additional types related to the TelemetryLogger. Telemetry is still declared as not enabled, so it won't change anything to the current behavior. However, extensions relying on Telemetry API will be able to run without any issues. As soon as the Telemetry API will be declared as mature enough, it will be possible to enable it with the support of preferences or a command line parameter (to be decided later).
TelemetrySender has no implementation, as this is really depending on the needs and the infrastructure of the extensions. This is basically the same for vscode, where an implementation using Application Insights is provided as an additional module.
Contributed on behalf of ST Microelectronics
How to test
Telemetry is still declared as not available according to: https://github.com/eclipsesource/theia/blob/bb71e70f6305b252aa5d663f0f73c1468b884821/packages/plugin-ext/src/plugin/telemetry-ext.ts#L20
So running the following actions won't produce anything until the flag is set to
true
.Log Telemetry event
. This will result on a notification on the test UI (directly displayed from the extension command before logging) and a log in the console if telemetry is activated. The log in the console is printed by the basic TelemetrySender provided in the sample. The logged data is cleaned as documented in the API (removal based on regex matching email, file path, Google key, etc.)Log Telemetry Exception
Note: No output is currently produced in the telemetry output channel as it is described in the vscode API. Goal of this task was only to initiate the main implementation.
Review checklist
Reminder for reviewers