Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
86813: pkg/keys: implement SafeFormatter for roachpb.Key r=andreimatei a=abarganier

Currently, when a key is logged, the entirety of the pretty-printed
key is redacted, hindering observability when parsing through redacted
logs (something that will become more common with upcoming compliance
requirements).

For example, prior to this patch, a pretty-printed key would appear
in the following way for the unredacted/redacted cases, respectively:

	- unredacted: ‹/Table/42/1222/"index key"›
	- redacted:   ‹x›

This patch addresses this by implementing the SafeFormatter interface
for `roachpb.Key` and `roachpb.RKey`, yielding the following result
when looking at the same example above:

	- unredacted: /Table/42/1222/‹"index key"›
	- redacted:   /Table/42/1222/‹x›

While the index key itself remains redacted, the ability to see the
specific table, index, and in the case of tenant tables, the tenant
itself, provides much better observability into which table & index
a log line is referring to than before.

Note that this implementation is only partial. It currently only
supports keys that fall in the `/Table` keyspace for application
tenants and system tenants. Keyspaces such as Meta1, Meta2, Local,
etc. are not yet supported, but can be added with much more ease
in the future now that the example has been set.

Finally, we remove the `maxLen` and related truncation logic from
`StringWithDirs`, as this is no longer used. Furthermore, the
truncation was invalid as it could have truncated a utf-8
sequence in the wrong place, making the result invalid utf-8.

This PR is a continuation of the work originally done by `@kzh` in
#67065. See the original PR for some initial discussions. 

Release note (security update): redacted logs will now reveal
pretty-printed keys, except for the index key values themselves.
For example `/Table/42/1222/‹x›` will be shown instead of `‹x›`
(which was shown previously). This improved redaction is available
for the `/Table` keyspace for both system and application tenants.
Other keyspaces such as `/Meta1`, `/Meta2`, `/Local`, etc. are not
yet supported.

Release justification: low risk, high benefit observability changes

Addresses #86316

87494: authors: add leon.fattakhov to authors r=coolcom200 a=coolcom200

Release note: None
Release justification: non-production code change.

87516: gc: end to end integration test r=tbg a=aliher1911

Currently tests are covering various components, but there's no end to end flow test that would verify that GC will issue GC requests that would successfully remove all relevant data.
This commit adds a TestCluster based test to verify GC behaviour.

Release justification: test only change
Release note: None
Fixes #84988

87527: sql: add query level execution stats to sampled query log r=THardy98 a=THardy98

Partially addresses: #84729

This change adds:
- network bytes sent
- maximum memory usage
- maximum disk usage
- KV bytes read
- KV rows read
- network messages

fields to the `SampledQuery` telemetry log.

Release justification: low risk, high benefit changes to existing functionality

Release note (sql change): This change adds::
- network bytes sent
- maximum memory usage
- maximum disk usage
- KV bytes read
- KV rows read
- network messages fields to the `SampledQuery` telemetry log.

87617: workload/schemachange: propagate worker ID to log r=ajwerner a=ajwerner

The schemachange workload can run with concurrency. It logs what workers do. It was hard to understand what different workers are doing because we never populated the worker ID.

Release note: None

Co-authored-by: Alex Barganier <[email protected]>
Co-authored-by: Leon Fattakhov <[email protected]>
Co-authored-by: Oleg Afanasyev <[email protected]>
Co-authored-by: Thomas Hardy <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
  • Loading branch information
6 people committed Sep 8, 2022
6 parents 9e331bb + bafdd45 + cfadc61 + 968c845 + c546e33 + ffc14a9 commit 5b8a6e2
Show file tree
Hide file tree
Showing 23 changed files with 1,047 additions and 207 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ Lasantha Pambagoda <[email protected]>
Lasse Nordahl <[email protected]> <[email protected]>
Lauren Hirata <[email protected]> Lauren <[email protected]> lhirata <[email protected]>
Lee Reilly <[email protected]>
Leon Fattakhov <[email protected]>
Levon Lloyd <[email protected]>
Liam Gillies <[email protected]>
Lidor Carmel <[email protected]>
Expand Down
6 changes: 6 additions & 0 deletions docs/generated/eventlog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2607,6 +2607,12 @@ contains common SQL event/execution details.
| `ZigZagJoinCount` | The number of zig zag joins in the query plan. | no |
| `ContentionNanos` | The duration of time in nanoseconds that the query experienced contention. | no |
| `Regions` | The regions of the nodes where SQL processors ran. | no |
| `NetworkBytesSent` | The number of network bytes sent by nodes for this query. | no |
| `MaxMemUsage` | The maximum amount of memory usage by nodes for this query. | no |
| `MaxDiskUsage` | The maximum amount of disk usage by nodes for this query. | no |
| `KVBytesRead` | The number of bytes read at the KV layer for this query. | no |
| `KVRowsRead` | The number of rows read at the KV layer for this query. | no |
| `NetworkMessages` | The number of network messages sent by nodes for this query. | no |


#### Common fields
Expand Down
2 changes: 2 additions & 0 deletions pkg/keys/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ go_library(
"//pkg/util/encoding",
"//pkg/util/uuid",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_redact//:redact",
],
)

Expand All @@ -45,6 +46,7 @@ go_test(
"//pkg/util/uuid",
"@com_github_cockroachdb_apd_v3//:apd",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_redact//:redact",
"@com_github_stretchr_testify//require",
],
)
Expand Down
Loading

0 comments on commit 5b8a6e2

Please sign in to comment.