Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bacalhau Client commands should log at Error level by default #4342

Closed
aronchick opened this issue Aug 26, 2024 · 5 comments
Closed

Bacalhau Client commands should log at Error level by default #4342

aronchick opened this issue Aug 26, 2024 · 5 comments
Labels
comp/cli Anything that affects the command-line interface th/refactor Theme: Code refactoring for better structure and maintenance th/user-experience Theme: Issues aimed at improving the end-user experience

Comments

@aronchick
Copy link
Collaborator

immediately after installing on a new machine

$ bacalhau node list --output json --api-host 0.0.0.0
01:22:21.385 | INF pkg/repo/fs.go:99 > Initializing repo at '/home/azureuser/.bacalhau' for environment 'production'
[{"Info":{"NodeID":"n-d620a0f0-0ee8-4d13-9120-6cf2a6898c80","NodeType":"Requester","Labels":{"Architecture":"amd64","DISK_GB":"29","HOSTNAME":"uu5tba-vm","IP":"13.89.100.89","LOCATION":"centralus","MACHINE_TYPE":"Standard_DS1_v2","MEMORY_GB":"3.3","NODE_TYPE":"requester","ORCHESTRATORS":"0.0.0.0","Operating-System":"linux","VCPU_COUNT":"1"},"BacalhauVersion":{"Major":"1","Minor":"4","GitVersion":"v1.4.0","GitCommit":"081eabfba0d723fbd3889d8e4e59c1ffc126ad0f","BuildDate":"2024-06-28T10:14:58Z","GOOS":"linux","GOARCH":"amd64"}},"Membership":"APPROVED","Connection":"DISCONNECTED"}]

Notice the first line - even thought it was supposed to be in "json"

@aronchick aronchick added type/bug Type: Something is not working as expected request/new Request: Indicates a new request that has been submitted and awaits initial triage labels Aug 26, 2024
@frrist frrist added request/needs-discussion Request: Requires further discussion to proceed and removed type/bug Type: Something is not working as expected request/new Request: Indicates a new request that has been submitted and awaits initial triage labels Aug 26, 2024
@frrist
Copy link
Member

frrist commented Aug 26, 2024

The output flag on the command wasn't ignored, the returned content is json and is written to stdout.

The first line:

01:22:21.385 | INF pkg/repo/fs.go:99 > Initializing repo at '/home/azureuser/.bacalhau' for environment 'production'

Is a log message, written to stderr.

[{"Info":{"NodeID":"n-d620a0f0-0ee8-4d13-9120-6cf2a6898c80","NodeType":"Requester","Labels":{"Architecture":"amd64","DISK_GB":"29","HOSTNAME":"uu5tba-vm","IP":"13.89.100.89","LOCATION":"centralus","MACHINE_TYPE":"Standard_DS1_v2","MEMORY_GB":"3.3","NODE_TYPE":"requester","ORCHESTRATORS":"0.0.0.0","Operating-System":"linux","VCPU_COUNT":"1"},"BacalhauVersion":{"Major":"1","Minor":"4","GitVersion":"v1.4.0","GitCommit":"081eabfba0d723fbd3889d8e4e59c1ffc126ad0f","BuildDate":"2024-06-28T10:14:58Z","GOOS":"linux","GOARCH":"amd64"}},"Membership":"APPROVED","Connection":"DISCONNECTED"}]

The rest of the output comes from the command, and is written to stdout as json.

@aronchick
Copy link
Collaborator Author

aronchick commented Aug 26, 2024

We should NEVER EVER write logs to stderr unless they are errors. That is an "INFO".

Also, when not logging in interactively, this is indisguishable.

E.g.:

	out, err := sshConfig.ExecuteCommand(
		ctx,
		fmt.Sprintf("bacalhau node list --output json --api-host %s", orchestratorIP),
	)
	if err != nil {
		return fmt.Errorf("failed to list Bacalhau nodes: %w", err)
	}

This logs in via golang, and is indistinguishable as stdout vs. stderr

@frrist
Copy link
Member

frrist commented Aug 26, 2024

We should NEVER EVER write logs to stderr unless they are errors. That is an "INFO".

Understood, will re-purpose this issue to change the default logging level for client commands to log at the Error level only.

This logs in via golang, and is indistinguishable as stdout vs. stderr

Assumedly, this is go code that is shelling out to a bacalhau binary installed on the host? If that is the case it might be worth using https://pkg.go.dev/os/exec#Cmd.StderrPipe and https://pkg.go.dev/os/exec#Cmd.StdoutPipe to differentiate errors from the app vs. output of the bacalhau command being executed.

@frrist frrist added type/tech-debt Type: Issues meant to address technical debt th/refactor Theme: Code refactoring for better structure and maintenance th/user-experience Theme: Issues aimed at improving the end-user experience comp/cli Anything that affects the command-line interface and removed request/needs-discussion Request: Requires further discussion to proceed type/tech-debt Type: Issues meant to address technical debt labels Aug 26, 2024
@frrist frrist moved this from Inbox to Next in Engineering Planning Aug 26, 2024
@frrist frrist changed the title After running for the first time, "--json" is overridden. Bacalhau Client commands should log at Error level by default Aug 26, 2024
@aronchick
Copy link
Collaborator Author

no, it's an external app that is shelling into the remote machine - the only thing we have available there is the bacalhau binary (no code deployed).

@wdbaruni
Copy link
Member

wdbaruni commented Oct 8, 2024

This was fixed in #4588 and released with v1.5.0, and now will look like:

→ bacalhau node list --output json --limit 1
Initializing repo at /Users/walid/.bacalhau
[{"Info":{"NodeID":"QmPLPUUjaVE3wQNSSkxmYoaBPHVAWdjBjDYmMkWvtMZxAf","NodeType":"Compute","Labels":{"Architecture":"amd64","Operating-System":"linux","name":"node-2","owner":"bacalhau"},"ComputeNodeInfo":{"ExecutionEngines":["wasm","docker"],"Publishers":["noop","s3","local","ipfs"],"StorageSources":["inline","s3","ipfs","urldownload"],"MaxCapacity":{"CPU":1.4,"Memory":5827443507,"Disk":1473789946675},"QueueCapacity":{},"AvailableCapacity":{"CPU":1.4,"Memory":5827443507,"Disk":1473789946675},"MaxJobRequirements":{"CPU":1.4,"Memory":5827443507,"Disk":1473789946675},"RunningExecutions":0,"EnqueuedExecutions":0},"BacalhauVersion":{"Major":"1","Minor":"5","GitVersion":"v1.5.0","GitCommit":"f9d5c971d381a26e8f9393ee7f04de4016cba13c","BuildDate":"2024-10-07T14:01:16Z","GOOS":"linux","GOARCH":"amd64"}},"Membership":"APPROVED","Connection":"CONNECTED"}]
  1. Now all logs go to stdout instead of stderr
  2. Client command logs are more user friendly format that is different than serve log format which is more suitable for running a server
  3. We minimized the logs that we print in client commands such as node list and docker run. The repo initialization is only logged the very first time you use a data dir

@wdbaruni wdbaruni closed this as completed Oct 8, 2024
@github-project-automation github-project-automation bot moved this from Next to Done in Engineering Planning Oct 8, 2024
@github-staff github-staff deleted a comment from Lxx-c Oct 23, 2024
@github-staff github-staff deleted a comment from Lxx-c Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp/cli Anything that affects the command-line interface th/refactor Theme: Code refactoring for better structure and maintenance th/user-experience Theme: Issues aimed at improving the end-user experience
Projects
Status: Done
Development

No branches or pull requests

4 participants
@aronchick @frrist @wdbaruni and others