feat: add log file path option to configuration

[prasad89]

Resolves #428

[Copilot]

Pull Request Overview

This PR adds a configurable log file path option to kubectl-ai, allowing users to specify where logs should be written or disable logging entirely. The change resolves an issue where users needed more control over log file location and the ability to disable file logging.

Key changes:

• Added LogFilePath configuration option with CLI flag support
• Refactored logging initialization to use the configured log path
• Added support for disabling logging by setting path to empty string or /dev/null

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
cmd/main.go	Added LogFilePath option, refactored logging setup to use configurable path, and added configureLogging helper function
README.md	Updated configuration documentation to include the new logFilePath setting

[Copilot]

Calling ParseFlags manually can interfere with Cobra's normal flag parsing flow and may cause issues with help text, completions, or subcommands. This could lead to flags being parsed twice or parsed arguments not being available to the command execution.

Suggested change

	// Parse flags first to get the final LogFilePath value
	if err := rootCmd.ParseFlags(os.Args[1:]); err != nil {
	return err
	}
	// Reconfigure logging with the final LogFilePath value (after CLI flags override config)
	configureLogging(klogFlags, opt.LogFilePath)
	// Reconfigure logging with the final LogFilePath value in PersistentPreRunE
	rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
	configureLogging(klogFlags, opt.LogFilePath)
	return nil
	}

[prasad89]

the suggestion or the code? ;)

[noahlwest]

Apologies for clarity, I meant that the suggestion seems reasonable. I spent a little time learning about cobra so I could try to write a more informed comment:

side note: In addition to the copilot suggestion, I think we could also remove the first configureLogging call (on 254). I think we can remove this because the second configureLogging call (either in your change or copilot's suggestion) will set it up with the final value.

My understanding here is that the first configureLogging call will have opt.LogFilePath value as either the default or the value provided by config file. Then, BuildRootCommand will call bindCLIFlags and register the cli flags we have with cobra, and those get parsed in rootCmd.ExecuteContext. rootCmd.PersistentPreRunE runs in ExecuteContext before RunE from BuildRootCommand (which is RunRootCommand for us). This PreRun would call configureLogging with the flags parsed from ExecuteContext, so it should have the final opt.LogFilePath there anyways, and it should also still configure the logging before we do any klog calling (I believe the first call is in RunRootCommand).

Both your change and the copilot suggestion will parse the flags and then set logging in configureLogging, but the copilot suggestion seems to do the same thing without adding whatever potential risks come from manually calling ParseFlags.

[Copilot]

Setting stderrthreshold to "FATAL" when logtostderr is "true" means that only fatal errors will be logged to stderr, effectively silencing most log output. This could hide important warnings and errors that users need to see.

Suggested change

	klogFlags.Set("stderrthreshold", "FATAL") // Only log fatal errors
	klogFlags.Set("stderrthreshold", "ERROR") // Log errors and above

[noahlwest]

Big thanks for also keeping the README updated!