*: improve redaction

[tbg]

Describe the problem

On customer incidents, we regularly struggle with redacted logs which typically

To Reproduce

Pick any CockroachCloud Zendesk ticket, download the debug zip, and see for yourself.

Expected behavior

Redaction, to the extent that is reasonable, leaves non-PII untouched and allows redacted and unredacted logs to be used more or less interchangeably. In particular, keys could be redacted smartly, by preserving the /Table/descid/idxid part and at least printing the types for everything else (/Table/55/3/<int>/<string>#0 for example.

Lints would make redactability opt-out (a change from today's opt-in), that is, a linter would ensure that any new type added implements redactability. This is not true today. I also believe there are a few ways in which implementing redactability could be simplified, such as cockroachdb/redact#5.

There would clearly need to be some transition, such as adding the linting code but instead of ensuring the absence of non-redactable structs, we'd assert that the number of violations does not increase. This also provides a rough score of how well we are doing in terms of redactability.

Additional data / screenshots

Environment:

Additional context

The below issues deal with aspects of redactability across CRDB:

#58610
#53310
#55862
#50044
#50043

Epic: CRDB-6668

Jira issue: CRDB-7612

[tbg]

Wasn't able to post this on Wednesday due to GH outage, but here it is:

level 👶🏽:  implement redact.SafeFormatter on one or more simple structs that shows up as redacted during ./cockroach start --logtostderr, following the example, for example, here.
level 🏊🏽:  implement redact.SafeFormatter for roachpb.Key.
level ️🌶️: cockroachdb/redact#5 I think this would be super valuable before we even think about nudging all teams to whittle down their redaction numbers.
level 🌶️: implement the linter above. Some design work upfront would be needed here, but the lint passes in ./pkg/lint/passes are probably a good starting point. We could traverse the syntax trees and to investigate everything that is passed to a logging method (to see whether it implements redactability), but that may be incomplete (redaction might be important outside of logging). The easier, and perhaps also more helpful option, is to check all the types declared by cockroach code (i.e. inside of our repo and not in vendor) directly for whether the implement redaction. To get the linter in without having to boil the ocean, we allow exceptions (type foo struct {} // nomustredact) which would be implemented similar to here or here - I prefer the latter since I understand it; the former seems to misfire sometimes. We want two lint exceptions, nomustredact, and todomustredact. The idea is that to get the lint to pass, we add // todomustredact everywhere and count how many we added. We configure the linter to allow no more than that number of todomustredact tags, and also output the current number. This gives us a metric that we can optimize, and an easy way to find all the types that still need to be retrofitted. Thanks to code ownership (go run ./pkg/cmd/whoownsit), we could break that down on a per-team level as well (i.e. team that owns the file that declares the type owns the redactability of the type), for scheduling this work into the next release.

I bet @knz has ideas on the above and other ideas as well.

With proper mentorship, I think all of the above are suitable for an intern as while of varying complexity, the work is at the library level with clear dependencies + docs, and not a ton of prior context is necessary.

[dhartunian]

Tracking the projects above in separate tickets:

• log redaction: implement redact.SafeFormatter on a few structs logged on startup #66139
• log redaction: preserve non-pii in table keys #66136
• Make safe portions of structs and slices non-redactable redact#5 deps: bump errors  #66880
• log redaction: make redactability lints opt-out instead of opt-in #66138

[knz]

I'm doing the struct and array auto-redaction in cockroachdb/redact#19
Once this is in, the work for the first two points might be slightly simplified: we won't need to implement SafeFormat methods for structs when the "natural" struct representation is sufficient.

[tbg]

This umbrella issue has outlived its purpose, closing.