Adar Dembo has posted comments on this change. Change subject: WIP: KUDU-1812. Redaction using thread-local boolean approach ......................................................................
Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/5557/2/src/kudu/common/column_predicate-test.cc File src/kudu/common/column_predicate-test.cc: Line 1012: ASSERT_EQ("`a` = <redacted>", ColumnPredicate::Equality(column_i32, &one_32).ToString()); I believe Dan's approach redacts the operator of the predicate as well as the predicate values, but IIRC, this was done for expediency, not because the operator is considered sensitive. Just wanted to point that out. http://gerrit.cloudera.org:8080/#/c/5557/2/src/kudu/util/flags-test.cc File src/kudu/util/flags-test.cc: Line 45: // command line flags and fails if an unsafe flag is set. Nit: or experimental http://gerrit.cloudera.org:8080/#/c/5557/2/src/kudu/util/logging.cc File src/kudu/util/logging.cc: Line 68: TAG_FLAG(log_redact_user_data, experimental); Why experimental? Line 82: __thread bool tls_redact_user_data = true; Would be nice to add a comment here justifying the default policy. http://gerrit.cloudera.org:8080/#/c/5557/2/src/kudu/util/logging.h File src/kudu/util/logging.h: Line 78: class ScopedDisableRedaction { I think it'd be nice to also enforce that there aren't nested calls to KUDU_DISABLE_REDACTION(). -- To view, visit http://gerrit.cloudera.org:8080/5557 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I75392ae36e96c1742e478e82671fd1eac3a0edee Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
