x/telemetry/cmd/gotelemetry: umbrella issue for cleanup

[hyangah]

• implement gotelemetry clean
• move gotelemetry set on/off to gotelemetry on/off
• make gotelemetry on print the legal notice
• hide or mark as experimental - gotelemetry dump and gotelemetry cvs. (the current thought is gotelemetry json...)
• gotelemetry help and gotelemetry -h

[gopherbot]

Change https://go.dev/cl/530515 mentions this issue: cmd/gotelemetry: replace 'set' with 'on' and 'off'

[gopherbot]

Change https://go.dev/cl/530555 mentions this issue: cmd/gotelemetry: revamp command line interface

[findleyr]

https://go.dev/cl/530555 implements everything except clean.

What exactly should clean do?

CC @pjweinb

[hyangah]

https://research.swtch.com/telemetry-design
"A command like go clean -telemetry would delete all of these."

I think clean is rm -rf local upload.

[pjweinb]

i'm not sure it should remove the directories, but maybe just empty them.

…

On Fri, Sep 22, 2023 at 5:33 PM Hyang-Ah Hana Kim ***@***.***> wrote: https://research.swtch.com/telemetry-design "A command like go clean -telemetry would delete all of these." I think clean is rm -rf local upload. — Reply to this email directly, view it on GitHub <#63142 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABJIAIYEZDYVYPJ5DVBGAILX3X72VANCNFSM6AAAAAA5BZCGNE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[findleyr]

Yeah, we shouldn't delete the 'weekends' file -- does that suggest that 'weekends' should perhaps belong in a different directory?

[findleyr]

Ok, while working on implementing gotelemetry clean (#63142), I started to look into the consequences of deleting state from the telemetry config dir. Here are a few observations:

• Each counter file, when it is created, looks for the presence of a report file (.json), to detect whether the user is 'new'. If no report file is found, the counter expiry is increased by a week.
• When creating reports to upload, they are collected by expiry, and a local report is created for any non-empty expiry before today. An upload report is created at the same time, if the telemetry mode is "on".
• When the upload pass is run, it collects any upload reports that haven't been uploaded, and uploads them.

I see several problems with this:

• If a user runs gotelemetry clean it will essentially reset their state to that of a new user, and new counters will skip an upload week (their expiry will be set far into the future).
• If a user runs gotelemetry off I don't see anything preventing old upload reports from being uploaded, though I may have missed something. That's a problem: we should never upload anything if the Mode is off.
• If a user runs gotelemetry on after a local report has been created (e.g. a few weeks after local collection started), we can potentially find local counters that expire the next day, not at least 7 days into the future as we suggest.
• If a user runs gotelemetry off and then subsequently runs gotelemetry on, we'll potentially upload new counter files sooner than expected, due to the presence of historical reports.

If I'm not misunderstanding, I think these are all bugs that need to be fixed, and I don't think we can fix them with the current data model, due to the complicated state interactions:

• counter expiry reflects the state of the local dir at the time the counter was created.
• upload reports reflect the state of the mode at the time the report was created.

I think the simplest and best fix is to associate an explicit date with the mode file, recording when the mode was set to "on". Then we can easily guarantee that we won't upload data before that date, and won't submit the first upload before T+7.

[pjweinb]

I need some time to think about this. It seems to me that we've added (perhaps justifiably) a bunch of complexity, and now have to cope with it. My model was that users watch what telemetry we collect locally, and then someday decide to turn telemetry on. At that point, we start uploading stuff, because they had time to see what was being collected and are apparently ok with it. If they then turn telemetry off, the right response is to remove all the counter files and the non-uploaded (non-local) reports. The go telemetry commands should support a simple user-focused model. Turning it on again afterwards is not the same as turning it on for the first time. (If they really want to start over, removing the local and upload dirs is the way)

I wouldn't claim this is the only way to think about it. Probably all have various drawbacks.

[findleyr]

I extracted the comment above into https://go.dev/issue/63218. I'll work on this.

[findleyr]

My model was that users watch what telemetry we collect locally, and then someday decide to turn telemetry on. At that point, we start uploading stuff, because they had time to see what was being collected and are apparently ok with it.

FWIW, when the user enabled telemetry, we say that we won't upload anything for 7 days, but that's not true if telemetry has been running (disabled) for some time: we could start uploading reports tomorrow if there are counters that expire tomorrow (which could easily be the case if the user has been running telemetry-enabled binaries for ~weeks.