Skip to content

fix: make log messages more AI-friendly #111

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.

Sign up for GitHub

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

Open
haliaeetusvocifer wants to merge 2 commits into
base: main
Choose a base branch
from
+9 −5
Open

fix: make log messages more AI-friendly #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f1c008e
fix: make log messages more AI-friendly
haliaeetusvocifer Apr 18, 2026
e27ae3d
Use uppercase for log properties
karolz-ms Apr 20, 2026
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
3 changes: 2 additions & 1 deletion controllers/executable_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,8 @@ func (r *ExecutableReconciler) startExecutable(

runner, runnerNotFoundErr := r.getExecutableRunner(exe, ri.startupStage)
if runnerNotFoundErr != nil {
log.Error(runnerNotFoundErr, "The Executable runner is not available", "StartupStage", ri.startupStage)
log.V(1).Info("Executable runner is not available for current startup stage, will try next stage if available",
"StartupStage", ri.startupStage, "Error", runnerNotFoundErr)
Comment on lines +455 to +456
Copy link
Copy Markdown
Collaborator

@karolz-ms karolz-ms Apr 20, 2026

Choose a reason for hiding this comment

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

Suggested change
log.V(1).Info("Executable runner is not available for current startup stage, will try next stage if available",
"StartupStage", ri.startupStage, "Error", runnerNotFoundErr)
log.V(1).Info("Executable runner is not available for current startup stage",
"StartupStage", ri.startupStage, "Error", runnerNotFoundErr)

Copy link
Copy Markdown
Collaborator

@karolz-ms karolz-ms Apr 20, 2026

Choose a reason for hiding this comment

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

If the execution gets to this place in the code, we know that there is more than one startup state to be tried, so "will try next stage if available" part is both redundant and bit misleading.

continue
}

Expand Down
5 changes: 4 additions & 1 deletion internal/containers/runtimes/runtime.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,10 @@ func FindAvailableContainerRuntime(ctx context.Context, log logr.Logger, executo
return nil, errNoRuntimeFound
}

log.V(1).Info("Runtime status", "Runtime", availableRuntime.orchestrator.Name(), "Status", availableRuntime.status)
log.V(1).Info("Container runtime selected (non-installed runtimes are expected and can be ignored)",
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

The new log message says "non-installed runtimes are expected and can be ignored", but this function logs after selecting a runtime and runs both when the runtime is auto-selected and when it is explicitly requested via flags. If the user explicitly selects a runtime that isn't installed/running, that is not expected/ignorable and this wording can hide real configuration problems. Consider making the message conditional on runtimeFlagValue == flags.UnknownRuntime, or tailor the text based on whether the runtime was user-selected vs auto-selected (and include guidance when the selected runtime is not installed).

Suggested change
log.V(1).Info("Container runtime selected (non-installed runtimes are expected and can be ignored)",
logMessage := "Container runtime selected"
if runtimeFlagValue == flags.UnknownRuntime {
logMessage = "Container runtime selected (non-installed runtimes are expected and can be ignored)"
}
log.V(1).Info(logMessage,

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

@karolz-ms karolz-ms Apr 20, 2026

Choose a reason for hiding this comment

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

Let's not change the properties here: Runtime and Status conveys what we need. Just change the message to "Container runtime selected" (that's it, no need to elaborate beyond that).

"SelectedRuntime", availableRuntime.orchestrator.Name(),
"Installed", availableRuntime.status.Installed,
"Running", availableRuntime.status.Running)
Comment thread
karolz-ms marked this conversation as resolved.

return availableRuntime.orchestrator, nil
}
4 changes: 2 additions & 2 deletions internal/exerunners/ide_connection_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ type ideConnectionInfo struct {
}

func NewIdeConnectionInfo(lifetimeCtx context.Context, log logr.Logger) (*ideConnectionInfo, error) {
const runnerNotAvailable = "Executables cannot be started via IDE: "
const missingRequiredEnvVar = "missing required environment variable '%s'"
const runnerNotAvailable = "IDE execution is not configured (this is expected in non-IDE environments): "
Copy link
Copy Markdown
Collaborator

@karolz-ms karolz-ms Apr 20, 2026

Choose a reason for hiding this comment

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

No, let's not change this error message. Requesting IDE execution is NOT expected in non-IDE environments and represents an invalid workload definition. Just keep the error message as-is.

const missingRequiredEnvVar = "environment variable '%s' is not set"
Copy link
Copy Markdown
Collaborator

@karolz-ms karolz-ms Apr 20, 2026

Choose a reason for hiding this comment

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

No, let's keep the message as-is. The part that the environment variable is REQUIRED is important.


createAndLogError := func(format string, a ...any) error {
err := fmt.Errorf(format, a...)
Expand Down
2 changes: 1 addition & 1 deletion internal/exerunners/ide_notification_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ func NewIdeNotificationHandler(
}

func (nh *ideNotificationHandler) WaitConnected(ctx context.Context) error {
const errDisposed = "the IDE session endpoint is not available"
const errDisposed = "IDE session endpoint has been disposed (this is expected during shutdown)"
Comment thread
karolz-ms marked this conversation as resolved.

nhState := nh.getState()
switch nhState {
Expand Down