log: replace a panic with an error in debug merge-logs

[cameronnunez]

Related issue: #68278

Release justification: bug fix

Release note (bug fix): Users would receive a panic message when the log parser
fails to extract log file formats. This has been replaced with a helpful
error message.

[cockroach-teamcity]

This change is 

[ajwerner]

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @cameronnunez)

[knz]

nice

[knz]

a unit test wouldn't hurt though

[knz]

for this you can create a new Example_xxx test in debug_merge_logs_test.go. You can take inspiration from node_test.go.

[cameronnunez]

@ajwerner mind taking a quick look at this Example test I threw up? The test isn't passing for some reason. No output.

--- FAIL: Example_format_error (0.01s)
got:
debug merge-logs /var/folders/k4/z4qfgyt11lbf01lqjpp8fc6h0000gq/T/1022529215
want:
debug merge-logs .
ERROR: decoding format: failed to extract log file format from the log

[knz]

I have tried to run the command manually (from the command line) on the test data.
The command succeeds without error! So it is not surprising that the test does not pass...

This means that the logic is not yet ready. In fact, I don't see any kind of error being handled properly (e.g. disk read errors are probably not handled properly either). This needs to be looked at first.

In general, before you build a test, try running the command manually! There's nothing better than the human eye to give confidence that something works as expected.

[knz]

Actually now if I run the cockroach debug merge-log on any log file, even one that contains data, I do not see any output any more.

[knz]

I stand corrected! the problem is that merge-logs only recognizes file names that follow the particular file name structure. Let's rename the file in the test.

[knz]

here you are: cameronnunez#2

merge-log only accepts file names with a particular structure. That's why the output was empty.

(Arguably it's poor UX that the command terminates without telling you that no files were processed. This could be improved too. Just not in this PR)

[knz]

don't forget to squash before merging

[knz]

this is ready to merge I think?

[cameronnunez]

this is ready to merge I think?

@knz CI was failing – had to add a release justification. Should I see if CI passes or go ahead and merge?

[knz]

let's wait for github CI to be green, you're right. then merge after that

[knz]

What's remaining to do with this PR?

[cameronnunez]

What's remaining to do with this PR?

For some reason, the test is failing with this output:

got:
debug merge-logs testdata/merge_logs_v1/missing_format/*
I210801 21:05:59.364923 1 :70 ⋮ [‹×›] ‹×›
I210801 21:05:59.364923 1 :70 ⋮ [‹×›] ‹×›
I210801 21:05:59.364923 1 :70 ⋮ ‹×›
want:
debug merge-logs testdata/merge_logs_v1/missing_format/*
ERROR: decoding format: failed to extract log file format from the log

[knz]

I think this is good now.

nit: I think our release note parser prefers when the release justification comes above the release note.

[cameronnunez]

bors r=ajwerner,knz,otan

[craig]

Build succeeded:

• GitHub CI (Cockroach)