bacalhau-project / bacalhau

Compute over Data framework for public, transparent, and optionally verifiable computation
https://docs.bacalhau.org
Apache License 2.0
671 stars 87 forks source link

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

Open aronchick opened 2 weeks ago

aronchick commented 2 weeks ago

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"

frrist commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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.

aronchick commented 2 weeks ago

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).