Can't use debug merge-logs on logs from 20.2 unless explicitly provide flag

[aliher1911]

Describe the problem

When parsing logs from 20.2 format detector will panic unhelpfully. It requires --format crdb-v1 to be passed to proceed which is unfriendly.

To Reproduce

Try

cockroach debug merge-logs with logs 

created by cockroach-20.2 and observe

ERROR: decoding format: failed to extract log file format from the log

Expected behavior
We should stick to v1 format if detector fails maybe or provide some helpful message at least?

This change was introduced in #65633

[aliher1911]

Would it work if cluster was upgraded and log format changed?

[ajwerner]

Making this work is going to require a very small code change. The decoder has the capability to auto detect the format per file. We're just not using that.

[aliher1911]

I think decoder tries to match regexp to guess format, but it fails because there's nothing like that in the logs as far as i can see.

[ajwerner]

These formats are so similar that it's hard to tell the difference. In the real world we should have the headers which should help us most of the time. Here's a WIP patch that does something but not enough: #68282

[ajwerner]

I'm probably not going to look at this any more.

[aliher1911]

Log parsing seems to be a rats nest if we want to cover old logs reliable where we don't have format type specifier.
I think we should just fall back to v1 parser in absence of info instead of failing. Maybe do a warning in worst case to let caller know, but it could generate alot of stderr noise.

I just created a PR to default it. Not sure if spending lots of time on it is justifiable.

[aliher1911]

As an example, this is the beginning of log from cockroach-v20.1.17

I210801 21:05:59.364923 1 util/log/sync_buffer.go:70  [config] file created at: 2021/08/01 21:05:59
I210801 21:05:59.364923 1 util/log/sync_buffer.go:70  [config] running on machine: Olegs-MacBook-Pro
I210801 21:05:59.364923 1 util/log/sync_buffer.go:70  [config] binary: CockroachDB CCL v20.1.17 (x86_64-apple-darwin14, built 2021/05/17 16:30:22,
I210801 21:05:59.364923 1 util/log/sync_buffer.go:70  [config] arguments: [./cockroach start]
I210801 21:05:59.364923 1 util/log/sync_buffer.go:70  line format: [IWEF]yymmdd hh:mm:ss.uuuuuu goid file:line msg utf8=✓
I210801 21:05:59.364921 1 cli/start.go:1051  logging to directory /Users/ali/Projects/LogExamples/cockroach-v20.1.17.darwin-10.9-amd64/cockroach-da

if I run

cockroach debug merge-logs .

I get

panic: decoding format: failed to extract log file format from the log

goroutine 14 [running]:
github.com/cockroachdb/cockroach/pkg/cli.(*fileLogStream).open(0xc000f78360, 0xc000f78360)
	/Users/ali/go/src/github.com/cockroachdb/cockroach/pkg/cli/debug_merge_logs.go:503 +0x385
github.com/cockroachdb/cockroach/pkg/cli.(*fileLogStream).peek(0xc000f78360, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/ali/go/src/github.com/cockroachdb/cockroach/pkg/cli/debug_merge_logs.go:512 +0x5c5
github.com/cockroachdb/cockroach/pkg/cli.newFileLogStream(0xc001892900, 0x7f, 0xc0004c6320, 0xc00028dbc0, 0xc, 0xc, 0x0, 0x0, 0x0, 0x0, ...)
	/Users/ali/go/src/github.com/cockroachdb/cockroach/pkg/cli/debug_merge_logs.go:477 +0x126
github.com/cockroachdb/cockroach/pkg/cli.newMergedStream.func1.1(0x0, 0x0)
	/Users/ali/go/src/github.com/cockroachdb/cockroach/pkg/cli/debug_merge_logs.go:217 +0x1c5
golang.org/x/sync/errgroup.(*Group).Go.func1(0xc000d90300, 0xc000379d80)
	/Users/ali/go/src/github.com/cockroachdb/cockroach/vendor/golang.org/x/sync/errgroup/errgroup.go:57 +0x59
created by golang.org/x/sync/errgroup.(*Group).Go
	/Users/ali/go/src/github.com/cockroachdb/cockroach/vendor/golang.org/x/sync/errgroup/errgroup.go:54 +0x66

I tried that with all major versions and 21.1.? has

log format (utf8=✓): crdb-v2

which would let it pass beyond panic.

[ajwerner]

While defaulting to v1 would help, it's not great. The patch in #68282 does a bit more. Importantly it'll allow v2 to be used when using date offsets.

[piyush-singh]

cc @cameronnunez when you get back we should take a look at this

[cameronnunez]

Log parsing seems to be a rats nest if we want to cover old logs reliable where we don't have format type specifier.

Yes, older logs do not have log format specification in the headers. This was introduced with #66096.

I think we should just fall back to v1 parser in absence of info instead of failing.

I think this makes sense as a short term solution if we need to look over old logs for users and upgrading is not desired. We will probably want to go back to failing after some time since having logs with format specification is desirable, and v2 is currently the main file format, not v1.

However, one thing to take note of is that older logs can be v2 formatted without a format specifier at the top of the log, so then defaulting to v1 is bad in this situation.