add telemetry appender that writes to log #53603
Conversation
| @@ -4,6 +4,7 @@ | |||
| "applicationName": "code-oss", | |||
| "dataFolderName": ".vscode-oss", | |||
| "win32MutexName": "vscodeoss", | |||
| "enableTelemetry": true, | |||
There was a problem hiding this comment.
maybe debatable but it helps with local development
There was a problem hiding this comment.
I would leave this out, and enable it on demand.
| } | ||
|
|
||
| log(eventName: string, data: any): void { | ||
| this._logService.trace(`telemetry/${eventName}`, data); |
There was a problem hiding this comment.
@jrieken My concern on using trace for the telemetry events is that the logs will now get very noisy. This is due to the data part of the event having a lot of "common" properties.
Anybody using the log level of trace is sure to get annoyed by all the noise.
An alternative is to
- Do what you did here, but log only the event name + data without the "common" parts of the data. This can help in the development/debugging process
- Have a dedicated output channel for the telemetry events with event name + all data. This can help in our attempt in being transparent.
Thoughts? cc @kieferrm
Below is a comparison between all data and data without the common parts
There was a problem hiding this comment.
Yeah, I am all in for a dedicated output channel or, even better, a dedicated log file which then shows as output channel. If we have dedicated channel we might also consider other places that send telemetry, like the main process and other windows.
There was a problem hiding this comment.
generally, don't feel obligated to stick to my changes/approch. it was just an experiment
|
@jrieken Made some changes as per #54001 (comment). Can you take a look? cc @joaomoreno, @sandy081 |
|
|
||
| export class LogAppender implements ITelemetryAppender { | ||
|
|
||
| private commonProperties = ['sessionID', 'version', 'timestamp']; |
There was a problem hiding this comment.
Also not sure about this... I understand the noise reduction but we aren't all honest with what we send when we filter stuff that goes into the log
There was a problem hiding this comment.
@jrieken What goes into the individual log files for processes (and so the output channel) is filtered. But the dedicated file telemetry.log will contain the complete data.
I think it's important - to see you changes in effect and to see the impact telemetry has, e.g us sending a telemetry event on type while suggestion show... |
With the changes in this PR, even when the Having it this way, lets us have what @jrieken is saying above. |
|
Thanks for making that clear, @ramya-rao-a. Looks good! 👍 |
This PR adds a telemetry appender that uses our log service. Its purpose is to make it easy to know what we log as telemetry (helps with transparency and development)
Edit by Ramya:
This PR implements #54001 (comment)