Skip to content
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

log redaction: make redactability lints opt-out instead of opt-in #66138

Closed
dhartunian opened this issue Jun 7, 2021 · 1 comment
Closed
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) no-issue-activity X-stale

Comments

@dhartunian
Copy link
Collaborator

dhartunian commented Jun 7, 2021

Is your feature request related to a problem? Please describe.
Currently, we do not enforce redactability on new types. This reduces the fidelity of redactable logs in CRDB.

Describe the solution you'd like
via @tbg on #65453:

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.

This is part of #65453

Jira issue: CRDB-7897

@dhartunian dhartunian added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-observability-inf labels Jun 7, 2021
@github-actions
Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) no-issue-activity X-stale
Projects
None yet
Development

No branches or pull requests

1 participant