Skip to content

Commit

Permalink
execinfrapb: finer grain redactability for ComponentStats
Browse files Browse the repository at this point in the history
Before:
```
event:‹component:... net_rx:...›
```

After:
```
event:ComponentStats{ID: {‹6e92072b-31ed-4b90-b323-b55d2d2fe0a3› PROCESSOR 0 1}, execution time: 1.5s, batches output: 1, rows output: 1}
```

We can also later consider exposing the FlowID as well, with a
suitable typedef and the `SafeValue` interface.

Release note: None
  • Loading branch information
knz committed May 29, 2023
1 parent 5060026 commit 09ba34a
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 13 deletions.
3 changes: 3 additions & 0 deletions pkg/base/node_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,9 @@ func (s SQLInstanceID) String() string {
return strconv.Itoa(int(s))
}

// SafeValue implements the redact.SafeValue interface.
func (s SQLInstanceID) SafeValue() {}

// SQLIDContainer is a variant of NodeIDContainer that contains SQL instance IDs.
type SQLIDContainer NodeIDContainer

Expand Down
1 change: 1 addition & 0 deletions pkg/sql/execinfrapb/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ go_library(
"//pkg/util/uuid",
"@com_github_cockroachdb_errors//:errors",
"@com_github_cockroachdb_logtags//:logtags",
"@com_github_cockroachdb_redact//:redact",
"@com_github_dustin_go_humanize//:go-humanize",
"@com_github_gogo_protobuf//types",
"@org_golang_google_grpc//:go_default_library",
Expand Down
20 changes: 20 additions & 0 deletions pkg/sql/execinfrapb/component_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/util/optional"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/cockroach/pkg/util/tracing/tracingpb"
"github.com/cockroachdb/redact"
"github.com/dustin/go-humanize"
"github.com/gogo/protobuf/types"
)
Expand Down Expand Up @@ -78,6 +79,25 @@ func (s *ComponentStats) StatsForQueryPlan() []string {
return result
}

// String implements fmt.Stringer and protoutil.Message.
func (s *ComponentStats) String() string {
return redact.StringWithoutMarkers(s)
}

var _ redact.SafeFormatter = (*ComponentStats)(nil)

// SafeValue implements redact.SafeValue.
func (ComponentID_Type) SafeValue() {}

// SafeFormat implements redact.SafeFormatter.
func (s *ComponentStats) SafeFormat(w redact.SafePrinter, _ rune) {
w.Printf("ComponentStats{ID: %v", s.Component)
s.formatStats(func(key string, value interface{}) {
w.Printf(", %s: %v", redact.SafeString(key), value)
})
w.SafeRune('}')
}

// formatStats calls fn for each statistic that is set.
func (s *ComponentStats) formatStats(fn func(suffix string, value interface{})) {
// Network Rx stats.
Expand Down
2 changes: 2 additions & 0 deletions pkg/sql/execinfrapb/component_stats.proto
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ message ComponentID {
// Depending on the component, not all statistics apply. For all fields, the zero
// value indicates that the particular stat is not available.
message ComponentStats {
option (gogoproto.goproto_stringer) = false;

optional ComponentID component = 1 [(gogoproto.nullable) = false];

optional NetworkRxStats net_rx = 2 [(gogoproto.nullable) = false];
Expand Down
29 changes: 16 additions & 13 deletions pkg/sql/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,25 +114,28 @@ func TestTrace(t *testing.T) {
// Check that stat collection from the above SELECT statement is output
// to trace. We don't insert any rows in this test, thus the expected
// num tuples value plus one is 1.
rows, err := sqlDB.Query(
"SELECT count(message) FROM crdb_internal.session_trace " +
"WHERE message LIKE '%component%num_tuples%value_plus_one:1%'",
)
rows, err := sqlDB.Query("SELECT message FROM crdb_internal.session_trace")
if err != nil {
t.Fatal(err)
}
if !rows.Next() {
t.Fatal("unable to retrieve count")
}

var count int
if err := rows.Scan(&count); err != nil {
var trace strings.Builder
if err := func() error {
for rows.Next() {
var msg string
if err := rows.Scan(&msg); err != nil {
return err
}
fmt.Fprintln(&trace, msg)
}
return rows.Close()
}(); err != nil {
t.Fatal(err)
}
if err := rows.Close(); err != nil {
t.Fatal(err)
t.Logf("trace:\n%s", trace.String())
if trace.Len() == 0 {
t.Fatalf("empty trace")
}
if count == 0 {
if !strings.Contains(trace.String(), "ComponentStats") {
t.Fatalf("no stat messages found")
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/testutils/lint/passes/redactcheck/redactcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
"NodeIDContainer": {},
"SQLIDContainer": {},
"StoreIDContainer": {},
"SQLInstanceID": {},
},
"github.com/cockroachdb/cockroach/pkg/cli/exit": {
"Code": {},
Expand Down Expand Up @@ -134,6 +135,9 @@ func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
"IndexDescriptorVersion": {},
"MutationID": {},
},
"github.com/cockroachdb/cockroach/pkg/sql/execinfrapb": {
"ComponentID_Type": {},
},
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgwirebase": {
"FormatCode": {},
},
Expand Down

0 comments on commit 09ba34a

Please sign in to comment.