Add ability to turn off telemetry in SignTool #7623
Conversation
| telemetry.SendEvents(); | ||
| if (!DisableTelemetry) | ||
| { | ||
| telemetry.SendEvents(); |
There was a problem hiding this comment.
This seems like a partial fix and adding an entire settable property for a partial fix isn't ideal. ie, if we are going to support the "DisableTelemetry" feature, then setting it to true should really disable the feature. Not just don't send events, but also, don't collect them.
I'm generally very anti-environment variables, but it seems like that would be a better solution here, particularly because we want the feature to be someone difficult to discover (a settable property makes it look like an acceptable practice rather than an exceptional case.
My preference would be that there is a "magic" env variable (only documented via comment in Telemetry.cs), which when set, turned telemetry.cs function calls into no-ops. This is a rare case where I prefer "magic". This would mean making the change entirely in Telemetry.cs rather than in SignToolTask.cs and the calling Sign.proj. Plus that telemetry class is already doing other magic with env variables, so it's not a huge stretch.
There was a problem hiding this comment.
Will work on adding a flag to disable entire telemetry. Thank you for suggesting this change.
There was a problem hiding this comment.
I think the real fix here is to find out why telemetry is not working just for Linux in the first place. I think Epsi has an issue tracking this, and it would be great if you could provide some guidance once we're able to focus on that issue. For now, we need to get this in with the hack....
There was a problem hiding this comment.
Yes, I think we're all saying the same thing. Perhaps my nomenclature was misleading. I meant his is an incomplete implementation of the "hack". Nobody believes that this work is an actual "fix".
There was a problem hiding this comment.
added this in a new PR #7638
|
created a new PR for this, so closing this one |
To double check:
Workaround for #7621
Adding turning off telemetry flag, to turn off telemetry in release pipeline for linux files.