Hello Dan Burkert, Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5557
to review the following change.
Change subject: WIP: KUDU-1812. Redaction using thread-local boolean approach
......................................................................
WIP: KUDU-1812. Redaction using thread-local boolean approach
This takes a different approach to redaction compared to
https://gerrit.cloudera.org/#/c/5555/2 as follows:
- there's a new thread-local boolean which indicates whether ToString
functions should redact user data in the current thread. This defaults
to 'true', but it is always also combined with the configuration flag
to determine whether redaction will actually happen.
- A utility macro KUDU_DISABLE_REDACTION(...) can disable redaction
while evaluating a particular expression or block, useful in contexts
such as the web UI or tools where we don't want to redact.
- Like the other patch, a macro KUDU_REDACT(expr) replaces its argument
with '<redacted>' if redaction is enabled.
The main advantages of this approach is that we're now able to apply
KUDU_REDACT() at a very low level (the stringification functions in
types.cc which are used for stringifying all user data). This means that
we are by-default redacted anywhere that stringifies a row, rather than
having to look for all cases that may lead to this stringification.
Instead, we only have to find the places that explicitly want to disable
redaction, which should be the exception rather than the rule.
This patch is likely incomplete: there are probably some more tests that
need to inherit from KuduTest in order to gain the "disable redaction by
default in tests" behavior. Additionally, there are probably a few more
non-test code paths that need to have redaction disabled. But, I wanted
to post this for an early review on the approach.
In addition, there are a few cases where we log various buffers that may
encode user data. Dan's patch covered many of those, but this one
doesn't. If we decide to go this direction, we'll need to port over
those KUDU_REDACT calls from his patch.
Change-Id: I75392ae36e96c1742e478e82671fd1eac3a0edee
---
M src/kudu/common/column_predicate-test.cc
M src/kudu/common/partition-test.cc
M src/kudu/common/partition.cc
M src/kudu/common/partition.h
M src/kudu/common/scan_spec-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/types.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/util/flag_tags-test.cc
M src/kudu/util/logging-test.cc
M src/kudu/util/logging.cc
M src/kudu/util/logging.h
M src/kudu/util/test_util.cc
13 files changed, 187 insertions(+), 22 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/5557/1
--
To view, visit http://gerrit.cloudera.org:8080/5557
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I75392ae36e96c1742e478e82671fd1eac3a0edee
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
