Skip to content

Commit

Permalink
Merge #66427 #67913
Browse files Browse the repository at this point in the history 
66427: util/log,cli: new TELEMETRY channel r=maryliag a=knz

Supersedes #64218


Informs #63815.

See discussion in #63815: we would like to leverage the logging
subsystem, in particular network logging, as a new way
to channel diagnostic reports.

This work will operate in multiple phases. This commit is only
about the first phase: creating a new logging channel TELEMETRY.

(see PRs #67809 and #67507 for example follow-ups)

Release note (cli change): CockroachDB now supports a new logging
channel called TELEMETRY. This will be used in later versions to
report diagnostic events useful to Cockroach Labs for product
analytics. (At the time of this writing, no events are defined for the
TELEMETRY channel yet.)

When no logging configuration is specified, this channel is connected
to file output, with a maximum retention of 1MiB.

To also produce the diagnostic output elsewhere, one can define a new
sink that captures this channel.
For example, to see diagnostics reports on the standard error, one can
use: `--log='sinks: {stderr: {channels: TELEMETRY, filter: INFO}}'`

When configuring file output, the operator should be careful to apply a
separate maximum retention for the TELEMETRY channel from other file
outputs, as telemetry data can be verbose and outcrowd other logging
messages. For example:

`--log='sinks: {file-groups: {telemetry: {channels: TELEMETRY,
max-group-size: 1MB}, ...}}`


67913: roachtest: remove mention of 'roachstress' r=tbg a=knz

The proposed 'roachstress' script linked from the repro steps at
https://gist.github.com/tbg/fc4409e06b4ddc9abc58838cf5c0fe8b is
utterly broken: it confuses local with remote execution.

The correct steps must clarify that `roachprod` and `roachtest`
binaries must be built for the local platform, and the `cockroach` and
`workload` binaries must be built for the remote platform. Someone
trying to run this on macos, or with a linux without the old glibc,
gets inscrutable error messages with the current script.

Additionally, the proposed roachstress script uses a `--cpu-quota`
parameter that's incoherent with `roachtest`'s own default.

Finally, I (@knz) directly witnessed a new hire who was utterly
confused by the amount of automation and assumptions made in the
roachtest script. (Should they install caffeinate? Why? How to
customize the roachtest flags? How to change the number of runs? Can
the test run locally  on my computer?)

In summary, the `roachstress` script is just too magical and actively
hostile to newcomers.

This patch removes it and replaces it by clearer instructiosn.

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
  • Loading branch information
@knz
craig[bot] and knz committed Jul 22, 2021
3 parents 5989de8 + dfc7f2b + 22c7022 commit 5c56090
Show file tree
Hide file tree
Showing 10 changed files with 498 additions and 67 deletions.
6 changes: 6 additions & 0 deletions docs/generated/logging.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,9 @@ helping developers of CockroachDB itself. It exists as a separate
channel so as to not pollute the `SQL_PERF` logging output with
internal troubleshooting details.

### `TELEMETRY`

The `TELEMETRY` channel reports telemetry events. Telemetry events describe
feature usage within CockroachDB and anonymizes any application-
specific data.

18 changes: 17 additions & 1 deletion pkg/cli/log_flags.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,8 @@ func addPredefinedLogFiles(c *logconfig.Config) {
if prefix == "sql-audit" && cliCtx.deprecatedLogOverrides.sqlAuditLogDir.isSet {
dir = &cliCtx.deprecatedLogOverrides.sqlAuditLogDir.s
}
m[prefix] = &logconfig.FileSinkConfig{

sinkConfig := &logconfig.FileSinkConfig{
Channels: logconfig.ChannelList{Channels: []logpb.Channel{ch}},
FileDefaults: logconfig.FileDefaults{
Dir: dir,
Expand All @@ -443,6 +444,20 @@ func addPredefinedLogFiles(c *logconfig.Config) {
},
},
}

if ch == channel.TELEMETRY {
// Keep less data for telemetry.
//
// This is the default configuration; as usual, this can be
// customized by adding an explicit file-group specification in
// the logging configuration.
sz := logconfig.ByteSize(100 * 1024) // 100KiB
groupSize := logconfig.ByteSize(1 * 1024 * 1024) // 1MiB
sinkConfig.MaxFileSize = &sz
sinkConfig.MaxGroupSize = &groupSize
}

m[prefix] = sinkConfig
}
}

Expand All @@ -455,6 +470,7 @@ var predefinedLogFiles = map[logpb.Channel]string{
channel.SQL_EXEC: "sql-exec",
channel.SQL_PERF: "sql-slow",
channel.SQL_INTERNAL_PERF: "sql-slow-internal-only",
channel.TELEMETRY: "telemetry",
}

// predefinedAuditFiles indicate which channel-specific files are
Expand Down
120 changes: 108 additions & 12 deletions pkg/cli/testdata/logflags
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],<defaultLogDir>,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrDisabled>},
<stdCaptureFd2(<defaultLogDir>)>}

Expand All @@ -46,7 +54,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],<defaultLogDir>,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrDisabled>},
<stdCaptureFd2(<defaultLogDir>)>}

Expand Down Expand Up @@ -127,7 +143,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],/pathA/logs,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],/pathA/logs,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/pathA/logs,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/pathA/logs,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/pathA/logs,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/pathA/logs,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: /pathA/logs,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrDisabled>},
<stdCaptureFd2(/pathA/logs)>}

Expand All @@ -150,7 +174,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],/mypath,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],/mypath,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/mypath,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/mypath,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/mypath,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/mypath,true,crdb-v2)>,
telemetry: {channels: [ TELEMETRY],
dir: /mypath,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrDisabled>},
<stdCaptureFd2(/mypath)>}

Expand All @@ -174,7 +206,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],/pathA/logs,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],/pathA/logs,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/pathA/logs,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/pathA/logs,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/pathA/logs,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/pathA/logs,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: /pathA/logs,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrDisabled>},
<stdCaptureFd2(/pathA/logs)>}

Expand Down Expand Up @@ -203,7 +243,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],/mypath,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],/mypath,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/mypath,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/mypath,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/mypath,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/mypath,true,crdb-v2)>,
telemetry: {channels: [ TELEMETRY],
dir: /mypath,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrDisabled>},
<stdCaptureFd2(/mypath)>}

Expand All @@ -225,7 +273,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],<defaultLogDir>,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrCfg(ERROR,true)>},
<stdCaptureFd2(<defaultLogDir>)>}

Expand All @@ -248,7 +304,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],<defaultLogDir>,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrDisabled>}}

# Logging to stderr without stderr capture causes an error in the default config.
Expand Down Expand Up @@ -302,7 +366,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],/mypath,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],/mypath,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/mypath,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/mypath,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/mypath,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/mypath,true,crdb-v2)>,
telemetry: {channels: [ TELEMETRY],
dir: /mypath,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrDisabled>},
<stdCaptureFd2(/mypath)>}

Expand All @@ -326,7 +398,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],/pathA,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],/pathA,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],/pathA,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],/pathA,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/pathA,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],/pathA,true,crdb-v2)>,
telemetry: {channels: [ TELEMETRY],
dir: /pathA,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrDisabled>},
<stdCaptureFd2(/pathA)>}

Expand Down Expand Up @@ -366,7 +446,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],<defaultLogDir>,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrCfg(INFO,true)>},
<stdCaptureFd2(<defaultLogDir>)>}

Expand All @@ -388,7 +476,15 @@ sql-audit: <fileCfg([SENSITIVE_ACCESS],<defaultLogDir>,false,crdb-v2)>,
sql-auth: <fileCfg([SESSIONS],<defaultLogDir>,false,crdb-v2)>,
sql-exec: <fileCfg([SQL_EXEC],<defaultLogDir>,true,crdb-v2)>,
sql-slow: <fileCfg([SQL_PERF],<defaultLogDir>,true,crdb-v2)>,
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>},
sql-slow-internal-only: <fileCfg([SQL_INTERNAL_PERF],<defaultLogDir>,true,crdb-v2)>,
telemetry: {channels: [TELEMETRY],
dir: <defaultLogDir>,
max-file-size: 100KiB,
max-group-size: 1.0MiB,
buffered-writes: true,
filter: INFO,
format: crdb-v2,
redactable: true}},
<stderrCfg(INFO,true)>},
<stdCaptureFd2(<defaultLogDir>)>}

Expand Down
9 changes: 7 additions & 2 deletions pkg/cmd/roachtest/test_runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -928,8 +928,13 @@ func (r *testRunner) maybePostGithubIssue(
Artifacts: artifacts,
ExtraLabels: labels,
ReproductionCommand: fmt.Sprintf(
`# From https://go.crdb.dev/p/roachstress, perhaps edited lightly.
caffeinate ./roachstress.sh %s
`## Simple repro (linux-only):
$ make cockroachshort bin/worklaod bin/roachprod bin/roachtest
$ PATH=$PWD/bin:$PATH roachtest run %[1]s --local
## Proper repro probably needs more roachtest flags, or running
## the programs remotely on GCE. For more details, refer to
## pkg/cmd/roachtest/README.md.
`, t.Name()),
}
if err := issues.Post(
Expand Down
5 changes: 5 additions & 0 deletions pkg/util/log/channel/channel_generated.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 5c56090

Please sign in to comment.