server: remove KeepRedactable logs rpc option#56878
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Nov 20, 2020
Merged
server: remove KeepRedactable logs rpc option#56878craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Member
|
This change is |
knz
approved these changes
Nov 19, 2020
Contributor
knz
left a comment
There was a problem hiding this comment.
Reviewed 6 of 6 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained
5d79a42 to
db25bc2
Compare
This patch removes the KeepRedactable option from the Logs and LogFile RPCs. The option existed just for backwards compatibility with pre-20.2 clients. Even for such old clients, I don't think there's real consequences to removing this option. This patch retains the behvior of the option being set. The option, when coupled with the Redact=false option, made the RPCs keep the log entries exactly as they are in the log file - i.e. redactable, with sensistive data markers. With this patch, the logs returned by the RPCs are either redacted, or redactable. "Flattening" them (i.e. not redacting, but stripping the markers) is no longer an option. The Logs RPC is used by the AdminUI, which doesn't use either Redact or KeepRedactable - so, with this patch, the UI will show logs with markers in them. I think that's OK. The LogFile RPC is used by the cli getting a debug zip. Since 20.2, the client is setting KeepRedactable, and so nothing changes. Older clients will get log entries with markers in them. This change is motivated by the fact that I'd like to replace the LogFile RPC with a more efficient version that returns a raw file, not a proto-encoded list of log entries. In fact we already have that RPC in the form of GetFiles, which we may be able to use. This KeepRedactable option was in my way. Release note: None
db25bc2 to
891b299
Compare
andreimatei
commented
Nov 19, 2020
Contributor
Author
andreimatei
left a comment
There was a problem hiding this comment.
bors r+
Reviewable status:
Contributor
|
Build failed: |
Contributor
Author
|
flaked on #53724 bors r+ |
Contributor
|
Build succeeded: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch removes the KeepRedactable option from the Logs and LogFile
RPCs. The option existed just for backwards compatibility with pre-20.2
clients. Even for such old clients, I don't think there's real
consequences to removing this option. This patch retains the behvior of
the option being set.
The option, when coupled with the Redact=false option, made the RPCs
keep the log entries exactly as they are in the log file - i.e.
redactable, with sensistive data markers. With this patch, the logs
returned by the RPCs are either redacted, or redactable. "Flattening"
them (i.e. not redacting, but stripping the markers) is no longer an
option.
The Logs RPC is used by the AdminUI, which doesn't use either Redact or
KeepRedactable - so, with this patch, the UI will show logs with markers
in them. I think that's OK.
The LogFile RPC is used by the cli getting a debug zip. Since 20.2, the
client is setting KeepRedactable, and so nothing changes. Older clients
will get log entries with markers in them.
This change is motivated by the fact that I'd like to replace the
LogFile RPC with a more efficient version that returns a raw file, not a
proto-encoded list of log entries. In fact we already have that RPC in
the form of GetFiles, which we may be able to use. This KeepRedactable
option was in my way.
Release note: None