Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions src/cmd/cli/command/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"log/slog"
"os"
"regexp"
"strings"
Expand Down Expand Up @@ -49,7 +50,7 @@ func Execute(ctx context.Context) error {

if err := RootCmd.ExecuteContext(ctx); err != nil {
if !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) {
term.Error("Error:", client.PrettyError(err))
slog.ErrorContext(ctx, fmt.Sprint("Error:", client.PrettyError(err)))
track.Evt("CLI Error", P("err", err))
}

Expand Down Expand Up @@ -101,15 +102,15 @@ func Execute(ctx context.Context) error {
}

if global.HasTty && term.HadWarnings() {
term.Println("For help with warnings, check our FAQ at https://s.defang.io/warnings")
fmt.Println("For help with warnings, check our FAQ at https://s.defang.io/warnings")
}

if global.HasTty && !global.HideUpdate && pkg.RandomIndex(10) == 0 {
if latest, err := github.GetLatestReleaseTag(ctx); err == nil && isNewer(GetCurrentVersion(), latest) {
term.Debug("Latest Version:", latest, "Current Version:", GetCurrentVersion())
term.Println("A newer version of the CLI is available at https://github.com/DefangLabs/defang/releases/latest")
slog.Debug("Newer version", "github", latest, "current", GetCurrentVersion())
fmt.Println("A newer version of the CLI is available at https://github.com/DefangLabs/defang/releases/latest")
if pkg.RandomIndex(10) == 0 && !pkg.GetenvBool("DEFANG_HIDE_HINTS") {
term.Println("To silence these notices, do: export DEFANG_HIDE_UPDATE=1")
fmt.Println("To silence these notices, do: export DEFANG_HIDE_UPDATE=1")
}
}
}
Expand Down Expand Up @@ -409,16 +410,16 @@ var RootCmd = &cobra.Command{
if connect.CodeOf(err) != connect.CodeUnauthenticated {
return err
}
term.Debug("Using existing token failed; continuing to allow login/ToS flow:", err)
slog.Debug(fmt.Sprint("Using existing token failed; continuing to allow login/ToS flow:", err))
}

track.Tracker = global.Client // update tracker with the real client

if v, err := global.Client.GetVersions(ctx); err == nil {
version := cmd.Root().Version // HACK to avoid circular dependency with RootCmd
term.Debug("Fabric:", v.Fabric, "CLI:", version, "CLI-Min:", v.CliMin)
slog.Debug(fmt.Sprint("Fabric:", v.Fabric, "CLI:", version, "CLI-Min:", v.CliMin))
if global.HasTty && isNewer(version, v.CliMin) && !isUpgradeCommand(cmd) {
term.Warn("Your CLI version is outdated. Please upgrade to the latest version by running:\n\n defang upgrade\n")
slog.WarnContext(ctx, "Your CLI version is outdated. Please upgrade to the latest version by running:\n\n defang upgrade\n")
global.HideUpdate = true // hide the upgrade hint at the end
}
}
Expand Down
57 changes: 29 additions & 28 deletions src/cmd/cli/command/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"io"
"log/slog"
"slices"
"strings"
"time"
Expand Down Expand Up @@ -35,9 +36,9 @@ const SERVICE_PORTAL_URL = "https://" + DEFANG_PORTAL_HOST + "/service"
func printPlaygroundPortalServiceURLs(serviceInfos []*defangv1.ServiceInfo) {
// We can only show services deployed to the prod1 defang SaaS environment.
if global.Stack.Provider == client.ProviderDefang && global.FabricAddr == client.DefaultFabricAddr {
term.Info("Monitor your services' status in the defang portal")
slog.Info("Monitor your services' status in the defang portal")
for _, serviceInfo := range serviceInfos {
term.Println(" -", SERVICE_PORTAL_URL+"/"+serviceInfo.Service.Name)
fmt.Println(" -", SERVICE_PORTAL_URL+"/"+serviceInfo.Service.Name)
}
}
}
Expand Down Expand Up @@ -94,9 +95,9 @@ func makeComposeUpCmd() *cobra.Command {
Type: defangv1.DeploymentType_DEPLOYMENT_TYPE_ACTIVE,
Stack: session.Stack.Name,
}); err != nil {
term.Debugf("ListDeployments failed: %v", err)
slog.Debug("ListDeployments failed", "err", err)
} else if accountInfo, err := session.Provider.AccountInfo(ctx); err != nil {
term.Debugf("AccountInfo failed: %v", err)
slog.Debug("AccountInfo failed", "err", err)
} else if len(resp.Deployments) > 0 {
workingDir, _ := session.Loader.ProjectWorkingDir(ctx)
confirmed, err := confirmDeployment(workingDir, resp.Deployments, accountInfo, session.Provider.GetStackName())
Expand All @@ -115,7 +116,7 @@ func makeComposeUpCmd() *cobra.Command {
Mode: session.Stack.Mode,
})
if err != nil {
term.Debug("Failed to create stack:", err)
slog.Debug(fmt.Sprint("Failed to create stack:", err))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's check if the file exists and its total line count
wc -l src/cmd/cli/command/compose.go

Repository: DefangLabs/defang

Length of output: 95


🏁 Script executed:

# Now let's look at the specific lines mentioned
sed -n '115,125p' src/cmd/cli/command/compose.go | cat -n

Repository: DefangLabs/defang

Length of output: 455


🏁 Script executed:

# Check lines around 164
sed -n '160,170p' src/cmd/cli/command/compose.go | cat -n

Repository: DefangLabs/defang

Length of output: 696


🏁 Script executed:

# Check lines around 172
sed -n '168,178p' src/cmd/cli/command/compose.go | cat -n

Repository: DefangLabs/defang

Length of output: 564


🏁 Script executed:

# Check all the other lines: 314, 326, 335, 347, 447, 524, 534
sed -n '310,320p' src/cmd/cli/command/compose.go | cat -n
sed -n '322,332p' src/cmd/cli/command/compose.go | cat -n
sed -n '331,341p' src/cmd/cli/command/compose.go | cat -n
sed -n '343,353p' src/cmd/cli/command/compose.go | cat -n
sed -n '443,453p' src/cmd/cli/command/compose.go | cat -n
sed -n '520,530p' src/cmd/cli/command/compose.go | cat -n
sed -n '530,540p' src/cmd/cli/command/compose.go | cat -n

Repository: DefangLabs/defang

Length of output: 3489


🏁 Script executed:

# Let's test the fmt.Sprint behavior to confirm the space issue
python3 << 'EOF'
import subprocess

# Test Go behavior
test_go_code = '''package main
import (
	"fmt"
	"errors"
)

func main() {
	err := errors.New("operation failed")
	
	// Test case 1: fmt.Sprint with colon
	result1 := fmt.Sprint("Failed to create stack:", err)
	fmt.Println("Test 1 (fmt.Sprint):", result1)
	
	// Test case 2: fmt.Sprintf with proper spacing
	result2 := fmt.Sprintf("Failed to create stack: %v", err)
	fmt.Println("Test 2 (fmt.Sprintf):", result2)
	
	// Test case 3: Direct concatenation
	result3 := "Failed to create stack: " + err.Error()
	fmt.Println("Test 3 (concatenation):", result3)
}
'''

# Write the test file
with open('/tmp/test_fmt.go', 'w') as f:
    f.write(test_go_code)

# Run the test
result = subprocess.run(['go', 'run', '/tmp/test_fmt.go'], capture_output=True, text=True)
print(result.stdout)
if result.stderr:
    print("STDERR:", result.stderr)
EOF

Repository: DefangLabs/defang

Length of output: 250


Several slog messages lose spaces because they are built with fmt.Sprint.

fmt.Sprint("prefix:", err) produces prefix:<err> without a space. Use structured logging with attributes instead:

-slog.Debug(fmt.Sprint("Failed to create stack:", err))
+slog.Debug("Failed to create stack", "err", err)
-slog.InfoContext(ctx, fmt.Sprint("Tailing logs for", tailSource, "; press Ctrl+C to detach:"))
+slog.InfoContext(ctx, "Tailing logs; press Ctrl+C to detach", "source", tailSource)
-slog.Info(fmt.Sprint("Deleted services, deployment ID", deployment))
+slog.Info("Deleted services", "deployment", deployment)

Apply the same refactoring to all fmt.Sprint-backed slog calls in this file (lines 119, 164, 172, 314, 326, 335, 347, 447, 524, 534).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
slog.Debug(fmt.Sprint("Failed to create stack:", err))
slog.Debug("Failed to create stack", "err", err)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cmd/cli/command/compose.go` at line 119, The slog calls currently
concatenate messages with fmt.Sprint (e.g., slog.Debug(fmt.Sprint("Failed to
create stack:", err))) which removes the space and bypasses structured logging;
replace each fmt.Sprint-backed slog invocation in this file (examples: the
slog.Debug in compose.go that uses "Failed to create stack:", and the similar
calls at the other listed locations) with structured slog calls that pass
attributes instead (e.g., use slog.Debug("Failed to create stack", "err", err)
or slog.Debug("message", slog.String("key", val)) so the message and fields are
separate); update all occurrences at the specified spots (lines referenced in
the review: 119, 164, 172, 314, 326, 335, 347, 447, 524, 534) to follow this
pattern and remove fmt.Sprint usage.

}
}

Expand All @@ -127,7 +128,7 @@ func makeComposeUpCmd() *cobra.Command {
}
}
if len(managedServices) > 0 {
term.Warnf("Defang cannot monitor status of the following managed service(s): %v.\n To check if the managed service is up, check the status of the service which depends on it.", managedServices)
slog.WarnContext(ctx, fmt.Sprintf("Defang cannot monitor status of the following managed service(s): %v.\n To check if the managed service is up, check the status of the service which depends on it.", managedServices))
}

deploy, project, err := cli.ComposeUp(ctx, global.Client, session.Provider, session.Stack, cli.ComposeUpParams{
Expand All @@ -151,7 +152,7 @@ func makeComposeUpCmd() *cobra.Command {
printPlaygroundPortalServiceURLs(deploy.Services)

if detach {
term.Info("Detached.")
slog.InfoContext(ctx, "Detached.")
return nil
}

Expand All @@ -160,15 +161,15 @@ func makeComposeUpCmd() *cobra.Command {
if deploy.Etag != "" {
tailSource = "deployment ID " + deploy.Etag
}
term.Info("Tailing logs for", tailSource, "; press Ctrl+C to detach:")
slog.InfoContext(ctx, fmt.Sprint("Tailing logs for", tailSource, "; press Ctrl+C to detach:"))

tailOptions := newTailOptionsForDeploy(session.Stack.Name, deploy.Etag, since, global.Verbose)
serviceStates, err := cli.TailAndMonitor(ctx, project, session.Provider, time.Duration(waitTimeout)*time.Second, tailOptions)
if err != nil {
deploymentErr := err
debugger, err := debug.NewDebugger(ctx, global.FabricAddr, session.Stack)
if err != nil {
term.Warn("Failed to initialize debugger:", err)
slog.WarnContext(ctx, fmt.Sprint("Failed to initialize debugger:", err))
return deploymentErr
}
handleTailAndMonitorErr(ctx, deploymentErr, debugger, debug.DebugConfig{
Expand Down Expand Up @@ -197,7 +198,7 @@ func makeComposeUpCmd() *cobra.Command {
return err
}

term.Info("Done.")
slog.InfoContext(ctx, "Done.")
flushWarnings()
return nil
},
Expand Down Expand Up @@ -246,7 +247,7 @@ func confirmDeployment(targetDirectory string, existingDeployments []*defangv1.D
Mode: global.Stack.Mode,
})
if err != nil {
term.Debugf("Failed to create stack %v", err)
slog.Debug("Failed to create stack", "err", err)
} else {
stacks.PrintCreateMessage(stackName)
}
Expand All @@ -255,7 +256,7 @@ func confirmDeployment(targetDirectory string, existingDeployments []*defangv1.D
}

func printExistingDeployments(existingDeployments []*defangv1.Deployment) {
term.Info("This project was previously deployed to the following locations:")
fmt.Println("This project was previously deployed to the following locations:")
deploymentStrings := make([]string, 0, len(existingDeployments))
for _, dep := range existingDeployments {
var providerId client.ProviderID
Expand Down Expand Up @@ -283,7 +284,7 @@ func confirmDeploymentToNewLocation() (bool, error) {

func promptToCreateStack(ctx context.Context, targetDirectory string, params stacks.Parameters) error {
if global.NonInteractive {
term.Info("Consider creating a stack to manage your deployments.")
fmt.Println("Consider creating a stack to manage your deployments.")
printDefangHint("To create a stack, do:", "stack new --name="+params.Name)
return nil
}
Expand All @@ -310,7 +311,7 @@ func handleComposeUpErr(ctx context.Context, debugger *debug.Debugger, project *
}

if connect.CodeOf(originalErr) == connect.CodeResourceExhausted && strings.Contains(originalErr.Error(), "maximum number of projects") {
term.Error("Error:", client.PrettyError(originalErr))
slog.ErrorContext(ctx, fmt.Sprint("Error:", client.PrettyError(originalErr)))
err := handleTooManyProjectsError(ctx, provider, originalErr)
if err != nil {
return originalErr
Expand All @@ -322,7 +323,7 @@ func handleComposeUpErr(ctx context.Context, debugger *debug.Debugger, project *
return originalErr
}

term.Error("Error:", client.PrettyError(originalErr))
slog.ErrorContext(ctx, fmt.Sprint("Error:", client.PrettyError(originalErr)))
return debugger.DebugDeploymentError(ctx, debug.DebugConfig{
Project: project,
}, originalErr)
Expand All @@ -331,7 +332,7 @@ func handleComposeUpErr(ctx context.Context, debugger *debug.Debugger, project *
func handleTooManyProjectsError(ctx context.Context, provider client.Provider, originalErr error) error {
projectName, err := provider.RemoteProjectName(ctx)
if err != nil {
term.Warn("failed to get remote project name:", err)
slog.WarnContext(ctx, fmt.Sprint("failed to get remote project name:", err))
return originalErr
}

Expand All @@ -343,7 +344,7 @@ func handleTooManyProjectsError(ctx context.Context, provider client.Provider, o

_, err = cli.InteractiveComposeDown(ctx, projectName, global.Client, provider)
if err != nil {
term.Warn("ComposeDown failed:", err)
slog.WarnContext(ctx, fmt.Sprint("ComposeDown failed:", err))
printDefangHint("To deactivate a project, do:", "compose down --project-name "+projectName)
return originalErr
} else {
Expand All @@ -358,7 +359,7 @@ func handleTailAndMonitorErr(ctx context.Context, err error, debugger *debug.Deb
var errDeploymentFailed client.ErrDeploymentFailed
if errors.As(err, &errDeploymentFailed) {
// Tail got canceled because of deployment failure: prompt to show the debugger
term.Warn(errDeploymentFailed)
slog.WarnContext(ctx, fmt.Sprintf("%v", errDeploymentFailed))
if errDeploymentFailed.Service != "" {
debugConfig.FailedServices = []string{errDeploymentFailed.Service}
}
Expand Down Expand Up @@ -437,21 +438,21 @@ func makeComposeDownCmd() *cobra.Command {
if err != nil {
if connect.CodeOf(err) == connect.CodeNotFound {
// Show a warning (not an error) if the service was not found
term.Warn(client.PrettyError(err))
slog.Warn(fmt.Sprintf("%v", client.PrettyError(err)))
return nil
}
return err
}

term.Info("Deleted services, deployment ID", deployment)
slog.Info(fmt.Sprint("Deleted services, deployment ID", deployment))

listConfigs, err := session.Provider.ListConfig(cmd.Context(), &defangv1.ListConfigsRequest{Project: projectName})
if err == nil {
if len(listConfigs.Names) > 0 {
term.Warn("Stored project configs are not deleted.")
slog.Warn("Stored project configs are not deleted.")
}
} else {
term.Debugf("ListConfigs failed: %v", err)
slog.Debug("ListConfigs failed", "err", err)
}

if detach {
Expand All @@ -468,12 +469,12 @@ func makeComposeDownCmd() *cobra.Command {
// different than `up`, which will wait for the deployment to finish, but we don't have an
// ECS event subscription for `down` so we can't wait for the deployment to finish.
// Instead, we'll just show a warning and detach.
term.Warn("Unable to tail logs. Detaching.")
slog.Warn("Unable to tail logs. Detaching.")
return nil
}
return err
}
term.Info("Done.")
slog.Info("Done.")
if len(listConfigs.Names) > 0 {
printDefangHint("To delete stored project configs, run:", "config rm --project-name="+projectName+" "+strings.Join(listConfigs.Names, " "))
}
Expand Down Expand Up @@ -520,7 +521,7 @@ func makeComposeConfigCmd() *cobra.Command {
CheckAccountInfo: false,
})
if err != nil {
term.Warn("unable to load stack:", err, "- some information may not be up-to-date")
slog.WarnContext(ctx, fmt.Sprint("unable to load stack:", err, "- some information may not be up-to-date"))
sessionx = &session.Session{
Loader: configureLoaderForCommand(cmd),
Provider: client.NewPlaygroundProvider(global.Client, stacks.DefaultBeta),
Expand All @@ -530,7 +531,7 @@ func makeComposeConfigCmd() *cobra.Command {

_, err = sessionx.Provider.AccountInfo(ctx)
if err != nil {
term.Warn("unable to connect to cloud provider:", err, "- some information may not be up-to-date")
slog.WarnContext(ctx, fmt.Sprint("unable to connect to cloud provider:", err, "- some information may not be up-to-date"))
}

project, loadErr := sessionx.Loader.LoadProject(ctx)
Expand Down Expand Up @@ -580,7 +581,7 @@ func makeComposePsCmd() *cobra.Command {
return err
}

term.Warn(err)
slog.Warn(fmt.Sprintf("%v", err))
printDefangHint("To start a new project, do:", "new")
return nil
}
Expand Down Expand Up @@ -676,7 +677,7 @@ func handleLogsCmd(cmd *cobra.Command, args []string) error {
if pkg.IsValidTime(untilTs) {
rangeStr += " until " + untilTs.Format(time.RFC3339Nano)
}
term.Infof("Showing logs%s; press Ctrl+C to stop:", rangeStr)
fmt.Printf("Showing logs%s; press Ctrl+C to stop:\n", rangeStr)

services := args
if len(name) > 0 {
Expand Down
34 changes: 27 additions & 7 deletions src/cmd/cli/command/compose_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@ package command
import (
"bytes"
"context"
"log/slog"
"os"
"testing"

"connectrpc.com/connect"
"github.com/DefangLabs/defang/src/pkg/cli/client"
"github.com/DefangLabs/defang/src/pkg/logs"
"github.com/DefangLabs/defang/src/pkg/term"
defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1"
)
Expand All @@ -25,24 +27,42 @@ func TestInitializeTailCmd(t *testing.T) {

func TestPrintPlaygroundPortalServiceURLs(t *testing.T) {
defaultTerm := term.DefaultTerm
oldStdout := os.Stdout
t.Cleanup(func() {
term.DefaultTerm = defaultTerm
os.Stdout = oldStdout
})

var stdout, stderr bytes.Buffer
term.DefaultTerm = term.NewTerm(os.Stdin, &stdout, &stderr)
// Capture slog output via term logger
var termBuf, stderr bytes.Buffer
term.DefaultTerm = term.NewTerm(os.Stdin, &termBuf, &stderr)
slog.SetDefault(logs.NewTermLogger(term.DefaultTerm))

// Capture fmt.Println output via os.Pipe
r, w, err := os.Pipe()
if err != nil {
t.Fatal(err)
}
os.Stdout = w

global.Stack.Provider = client.ProviderDefang
global.FabricAddr = client.DefaultFabricAddr
printPlaygroundPortalServiceURLs([]*defangv1.ServiceInfo{
{
Service: &defangv1.Service{Name: "service1"},
}})
const want = ` * Monitor your services' status in the defang portal
- https://portal.defang.io/service/service1
`
if got := stdout.String(); got != want {
t.Errorf("got %q, want %q", got, want)

w.Close()
var stdoutBuf bytes.Buffer
stdoutBuf.ReadFrom(r)

const wantSlog = " * Monitor your services' status in the defang portal\n"
if got := termBuf.String(); got != wantSlog {
t.Errorf("slog output: got %q, want %q", got, wantSlog)
}
const wantStdout = " - https://portal.defang.io/service/service1\n"
if got := stdoutBuf.String(); got != wantStdout {
t.Errorf("stdout output: got %q, want %q", got, wantStdout)
}
}

Expand Down
11 changes: 6 additions & 5 deletions src/cmd/cli/command/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"fmt"
"io"
"log/slog"
"os"
"strings"

Expand Down Expand Up @@ -164,13 +165,13 @@ var configSetCmd = &cobra.Command{
if err != nil {
errs = append(errs, err)
} else if ifNotSet && !didSet {
term.Info("Config", name, "is already set; skipping due to --if-not-set flag")
slog.Info(fmt.Sprintf("Config %s is already set; skipping due to --if-not-set flag", name))
} else {
term.Info("Updated value for", name)
slog.Info("Updated value for " + name)
}
}

term.Infof("Successfully set %d config value(s)", len(envMap)-len(errs))
slog.Info(fmt.Sprintf("Successfully set %d config value(s)", len(envMap)-len(errs)))

printDefangHint("To update the deployed values, do:", "compose up")
return errors.Join(errs...)
Expand All @@ -197,12 +198,12 @@ var configDeleteCmd = &cobra.Command{
if err := cli.ConfigDelete(cmd.Context(), projectName, session.Provider, names...); err != nil {
// Show a warning (not an error) if the config was not found
if connect.CodeOf(err) == connect.CodeNotFound {
term.Warn(client.PrettyError(err))
slog.Warn(fmt.Sprintf("%v", client.PrettyError(err)))
return nil
}
return err
}
term.Info("Deleted", names)
slog.Info(fmt.Sprint("Deleted", names))

printDefangHint("To list the configs (but not their values), do:", "config ls")
return nil
Expand Down
Loading
Loading