[APP-6612] Support downloading logs

[JosephBorodach]

This PR implements enhancements to the machines logs CLI command, including support for writing logs to a file in text or json format, while maintaining the ability to print logs to stdout. Below are the details and test cases for the command:

See ticket here

Print text format to console (stdout)

Command

go run cli/viam/main.go machines logs -machine="1" --organization="viam-dev" --location="office temp" --format="text"

Output

Print json format to console (stdout)

Command

go run cli/viam/main.go machines logs -machine="1" --organization="viam-dev" --location="office temp" --format="json"

Output

Write logs to a file in text format

Command

go run cli/viam/main.go machines logs -machine="1" --organization="viam-dev" --location="office temp" --output="logs.txt" --format="text"

Output

Write logs to a file in json format

Command

go run cli/viam/main.go machines logs -machine="1" --organization="viam-dev" --location="office temp" --output="logs.json" --format="json"

Output

Invalid: Without any arguments

Command

go run cli/viam/main.go machines logs

Output

Missing format args, will default to text format and print logs to console

Command

go run cli/viam/main.go machines logs --machine="1" --organization="viam-dev" --location="office temp"

Output

Invalid format - will print error to console

Command

go run cli/viam/main.go machines logs --machine="1" --organization="viam-dev" --location="office temp" --format="foo"

Output

[dgottlieb]

Dumb question -- were you aware we already had a way to download logs?

$ ./viam-cli robot log --help
NAME:
   viam machines logs - display machine logs

USAGE:
   viam machines logs --machine=<machine> [other options]

OPTIONS:
   --count value                   number of logs to fetch (max 10000) (default: 100)
   --errors                        show only errors (default: false)
   --location value                (default: first location alphabetically)
   --machine value, --robot value  
   --organization value            (default: first organization alphabetically)

It's admittedly a bit unwieldy to discover that.

[JosephBorodach]

Dumb question -- were you aware we already had a way to download logs?

$ ./viam-cli robot log --help
NAME:
   viam machines logs - display machine logs

USAGE:
   viam machines logs --machine=<machine> [other options]

OPTIONS:
   --count value                   number of logs to fetch (max 10000) (default: 100)
   --errors                        show only errors (default: false)
   --location value                (default: first location alphabetically)
   --machine value, --robot value  
   --organization value            (default: first organization alphabetically)

It's admittedly a bit unwieldy to discover that.

@dgottlieb - I’m aware that we already have a way to download logs, but the current method prints them directly to the console. The new feature addresses interest in being able to save logs as a file (e.g., in text or JSON formats) for easier consumption, sharing, or further processing. I think this enhances the usability of the tool.

[dgottlieb]

Ah got it! The ticket wasn't clear on exactly what functionality was missing. I'm going to (strongly) recommend not adding another command altogether. And instead add this functionality to the existing logs command. But I suppose SDK is ultimately in charge of that.

From what you said, this adds to bits of functionality:

• Downloading instead of printing
• Being able to choose between text vs json outputs

If you really* want to add an option for downloading, that's fine. But it is (very) typical to turn text output into a file with shell redirection, e.g:

$ ./viam-cli print-logs > myfile.txt

But certainly for some commands (like curl) they default to stdout, but can additionally write to a file (using -o is traditional. Short for --output_file).

The other bit you mentioned -- giving the user a choice between text and json is very cool! I think you already went with --format=<text/json> which is exactly what I would recommend!

[JosephBorodach]

Ah got it! The ticket wasn't clear on exactly what functionality was missing. I'm going to (strongly) recommend not adding another command altogether. And instead add this functionality to the existing logs command. But I suppose SDK is ultimately in charge of that.

From what you said, this adds to bits of functionality:

• Downloading instead of printing
• Being able to choose between text vs json outputs

If you really* want to add an option for downloading, that's fine. But it is (very) typical to turn text output into a file with shell redirection, e.g:

$ ./viam-cli print-logs > myfile.txt

But certainly for some commands (like curl) they default to stdout, but can additionally write to a file (using -o is traditional. Short for --output_file).

The other bit you mentioned -- giving the user a choice between text and json is very cool! I think you already went with --format=<text/json> which is exactly what I would recommend!

@dgottlieb - Great point! I agree with your suggestion to extend the current functionality of the logs command rather than introducing a new command - it ensures consistency and avoids redundancy in the CLI. I'll proceed to modify the logs command to support saving logs to a file.

[dgottlieb]

When outputting data as json, I think it's customary to output:

{"ts": 123, "message": "log line 1"}
{"ts": 124, "message": "log line 2"}

IIUC, this will instead output an array with 2 items:

[
  {"ts": 123, "message": "log line 1"},
  {"ts": 124, "message": "log line 2"}
]

Related to the above, the reason is because it is often difficult for json parsers/utilities to work with large lists efficiently/ergonomically.

This feels like a recipe to OOM. I've downloaded multi-GB log files

[jr22]

Haven't looked at the code yet but adding my own context on the work-- ideally this would bring the export logs experience closer to the export data and logs UI experience which supports time ranges, filters, etc. I'm not super familiar with the existing command's code but would imagine theres also some room for improvement for downloading large amounts of logs. Basically reducing friction when person 1 is looking at these specific logs in the UI and wants to send these exact logs to person 2 that doesnt have access to the machine without copying and pasting from the UI and getting gross formatting. part 2 of this work will be to surface this command (prepopulated with values when possible) in the UI (like we do for data) so that its more discoverable

[jr22]

removing myself since dan is already reviewing. if there is anything core specific that you want my opinion on, please retag me in a comment. my one request is that we do a follow up pr to include things that i mentioned in my comment above such as filters and time span. thanks joseph!

[JosephBorodach]

removing myself since dan is already reviewing. if there is anything core specific that you want my opinion on, please retag me in a comment. my one request is that we do a follow up pr to include things that i mentioned in my comment above such as filters and time span. thanks joseph!

I am going to stack another pr onto this one.

[JosephBorodach]

[todo] remove

[JosephBorodach]

[todo] have fetchAndSaveLogs io.writer, determine the formatting accordingly, and then write the formatted log to io.writer using fmt.fPrintln. Combine fetchAndSaveLogs and printLogsToConsole (essentially removing printLogsToConsole).

[JosephBorodach]

[todo] Change to fmt.fPrintln and accept a io.writter. [In order to do this we will need to change the signature of the function]

[benjirewis]

Nice!

[benjirewis]

Are we still limited to 10k log fetch at once? /Is a future ticket adding date ranges for log-grabbing?