From 3a8e77ba37c89e776a9ddc9ebc65c594a6ff9864 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Wed, 5 Aug 2026 18:52:31 -0400 Subject: [PATCH 1/5] Switch from zap to slog, remove logging statements outside of MCP server --- CLAUDE.md | 52 ++++--- README.md | 3 - cmd/tiger/main.go | 9 +- docs/development.md | 14 +- go.mod | 2 +- internal/analytics/analytics.go | 47 +------ internal/cmd/completion_helper.go | 2 +- internal/cmd/config_reset.go | 2 - internal/cmd/config_set.go | 3 - internal/cmd/config_show.go | 3 - internal/cmd/config_show_test.go | 3 - internal/cmd/config_unset.go | 3 - internal/cmd/logger_helper.go | 16 +++ internal/cmd/mcp_get.go | 2 +- internal/cmd/mcp_install.go | 32 ----- internal/cmd/mcp_list.go | 2 +- internal/cmd/mcp_start.go | 14 +- internal/cmd/mcp_start_http.go | 28 ++-- internal/cmd/mcp_start_stdio.go | 2 +- internal/cmd/root.go | 45 +++---- internal/cmd/root_test.go | 4 - internal/common/client_test.go | 9 -- internal/config/config.go | 7 +- internal/logging/logging.go | 81 ----------- internal/mcp/capabilities.go | 10 +- internal/mcp/db_execute_query.go | 17 ++- internal/mcp/db_schema.go | 21 ++- internal/mcp/proxy.go | 157 ++++++++++++---------- internal/mcp/server.go | 29 ++-- internal/mcp/server_test.go | 1 + internal/mcp/service_create.go | 27 ++-- internal/mcp/service_fork.go | 25 ++-- internal/mcp/service_get.go | 9 +- internal/mcp/service_list.go | 5 +- internal/mcp/service_logs.go | 17 ++- internal/mcp/service_metrics_available.go | 9 +- internal/mcp/service_metrics_series.go | 15 +-- internal/mcp/service_resize.go | 11 +- internal/mcp/service_start.go | 9 +- internal/mcp/service_stop.go | 9 +- internal/mcp/service_update_password.go | 13 +- internal/mcp/utils.go | 7 +- internal/version/check.go | 3 - 43 files changed, 309 insertions(+), 470 deletions(-) create mode 100644 internal/cmd/logger_helper.go delete mode 100644 internal/logging/logging.go diff --git a/CLAUDE.md b/CLAUDE.md index 8f416f2e..91bc6fce 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -299,7 +299,6 @@ Tiger CLI is a Go-based command-line interface for managing Tiger, the modern da - **Configuration**: `internal/config/config.go` - `Config` struct plus load/write helpers. Each `Load` uses its own viper instance (no global state); see "Configuration Management" above -- **Logging**: `internal/logging/logging.go` - Structured logging with zap - **API Client**: `internal/api/` - Generated OpenAPI client with mocks - **MCP Server**: `internal/mcp/` - Model Context Protocol server implementation. Each MCP tool lives in its own file, named to match the tool (see "One File Per @@ -369,13 +368,14 @@ The Tiger MCP server provides AI assistants with programmatic access to Tiger re **Server State:** -`NewServer(ctx, app)` takes the already-loaded `*common.App` and keeps it on the -`Server`. Read-only mode, the experimental gate, and the docs-proxy settings are -read once here at startup (a client must restart the server to pick those up), -while the analytics middleware calls `s.app.Load(ctx)` on every request so tool -handlers see current config and credentials. Handlers therefore never load -anything themselves — they read `s.app.GetAll()`, `s.app.GetClient()`, or -`s.app.GetConfig()`. +`NewServer(ctx, app, logger)` takes the already-loaded `*common.App` and keeps it +on the `Server`, along with the logger (nil is replaced with a discarding one; +see "Logging Architecture"). Read-only mode, the experimental gate, and the +docs-proxy settings are read once here at startup (a client must restart the +server to pick those up), while the analytics middleware calls `s.app.Load(ctx)` +on every request so tool handlers see current config and credentials. Handlers +therefore never load anything themselves — they read `s.app.GetAll()`, +`s.app.GetClient()`, or `s.app.GetConfig()`, and log via `s.logger`. **One File Per MCP Tool:** @@ -469,15 +469,30 @@ addTool(s, readOnly, newServiceCreateTool(), s.handleServiceCreate) ### Logging Architecture -Two-mode logging system using zap: -- **Production mode**: Minimal output, warn level and above, clean formatting -- **Debug mode**: Full development logging with colors and debug level +Only the MCP server logs. `newLogger(w io.Writer)` +(`internal/cmd/logger_helper.go`) points the standard `log` package at `w` and +returns `slog.Default()`; `tiger mcp start` (stdio and http) calls it with +`cmd.ErrOrStderr()` and passes the result to `mcp.NewServer`, which uses it for +its own output and hands it to the MCP SDK (`mcp.ServerOptions.Logger` and the +docs proxy's `mcp.ClientOptions.Logger`) — so the SDK's own session-lifecycle +lines land on the same stream. `mcp.NewServer(ctx, app, nil)` — used by +`mcp list`, `mcp get`, and completion, which only enumerate capabilities — +discards the output via `slog.New(slog.DiscardHandler)`. + +There is no level configuration and no `--debug` flag. `slog.Default()` drops +anything below `Info` unless `slog.SetLogLoggerLevel` is called, so log at `Info` +or above; a `Debug` call would silently go nowhere. Attach errors with +`slog.Any("error", err)`, not `slog.String("error", err.Error())`. + +Everything outside `internal/mcp` writes to stdout/stderr directly rather than +logging. Don't add log statements to CLI commands — print to +`cmd.OutOrStdout()`/`cmd.ErrOrStderr()`, or return an error. ### Dependencies - **Cobra**: CLI framework and command structure - **Viper**: Configuration management with multiple sources -- **Zap**: Structured logging +- **slog**: Structured logging for the MCP server - **oapi-codegen**: OpenAPI client generation (build-time dependency) - **gomock**: Mock generation for testing (build-time dependency) - **go-sdk (MCP)**: Model Context Protocol SDK for AI assistant integration @@ -494,7 +509,6 @@ tiger-cli/ │ ├── api/ # Generated OpenAPI client (oapi-codegen) │ │ └── mocks/ # Generated mocks for testing │ ├── config/ # Configuration management -│ ├── logging/ # Structured logging utilities │ ├── mcp/ # MCP server implementation (one file per tool) │ ├── common/ # Shared business logic (password storage, wait ops, error handling, log fetching) │ ├── cmd/ # CLI commands (Cobra, one file per command) @@ -580,11 +594,10 @@ order: 1. `app.SetFlags(cmd.Flags())` then `app.Load(ctx)` — the single config + API client load for the invocation -2. `logging.Init(cfg.Debug)`, with `logging.Sync()` deferred -3. `color.NoColor` from `cfg.Color` -4. `versionCheck(...)` — starts a background release check, deferring the print +2. `color.NoColor` from `cfg.Color` +3. `versionCheck(...)` — starts a background release check, deferring the print so it lands after the command's own output -5. analytics — deferred, so it records the command's outcome and re-reads the App +4. analytics — deferred, so it records the command's outcome and re-reads the App (see "Configuration Management") Commands cobra adds after `wrapCommands` runs — `help`, `completion`, and the @@ -614,8 +627,8 @@ Place a helper by who calls it, working down this list until one matches: 1. **One command** → that command's file. 2. **Several commands in one group** → the group file (`service.go`, `db.go`). 3. **Across groups** → a package-level `_helper.go` file: - `completion_helper.go`, `flag_helper.go`, `terminal_helper.go`, - `password_helper.go`. + `completion_helper.go`, `flag_helper.go`, `logger_helper.go`, + `terminal_helper.go`, `password_helper.go`. 4. **A genuine standalone utility** — small and isolated, with no notion of a command (`util.GenerateSecurePassword`) → `internal/util`. Anything shaped around the CLI stays in `cmd` even if its signature looks generic. @@ -659,7 +672,6 @@ func buildRootCmd(ctx context.Context) (*cobra.Command, error) { // Set up persistent flags cmd.PersistentFlags().String("config-dir", config.GetDefaultConfigDir(), "config directory") - cmd.PersistentFlags().Bool("debug", false, "enable debug logging") skipUpdateCheck := cmd.PersistentFlags().Bool("skip-update-check", false, "skip checking for updates on startup") // ... add remaining persistent flags diff --git a/README.md b/README.md index 28447959..69033b44 100644 --- a/README.md +++ b/README.md @@ -245,7 +245,6 @@ All configuration options can be set via `tiger config set `: - `analytics` - Enable/disable analytics (default: `true`) - `color` - Enable/disable colored output (default: `true`) -- `debug` - Enable/disable debug logging (default: `false`) - `docs_mcp` - Enable/disable docs MCP proxy (default: `true`) - `mcp_max_rows` - Maximum number of rows the `db_execute_query` MCP tool returns per result set before truncating, to limit how much data lands in an AI agent's context. Only applies to the MCP tool, not CLI commands. Default: `100` - `output` - Output format: `json`, `yaml`, or `table` (default: `table`) @@ -261,7 +260,6 @@ Environment variables override configuration file values. All variables use the - `TIGER_ANALYTICS` - Enable/disable analytics - `TIGER_COLOR` - Enable/disable colored output - `TIGER_CONFIG_DIR` - Path to configuration directory (default: `~/.config/tiger`) -- `TIGER_DEBUG` - Enable/disable debug logging - `TIGER_DOCS_MCP` - Enable/disable docs MCP proxy - `TIGER_OUTPUT` - Output format: `json`, `yaml`, or `table` - `TIGER_PASSWORD_STORAGE` - Password storage method: `keyring`, `pgpass`, or `none` @@ -278,7 +276,6 @@ These flags are available on all commands and take precedence over both environm - `--analytics` - Enable/disable analytics - `--color` - Enable/disable colored output - `--config-dir ` - Path to configuration directory (default: `~/.config/tiger`) -- `--debug` - Enable/disable debug logging - `--password-storage ` - Password storage method: `keyring`, `pgpass`, or `none` - `--service-id ` - Specify service ID - `--skip-update-check` - Skip checking for updates on startup (default: `false`) diff --git a/cmd/tiger/main.go b/cmd/tiger/main.go index 43325d31..4eacd0e4 100644 --- a/cmd/tiger/main.go +++ b/cmd/tiger/main.go @@ -8,10 +8,7 @@ import ( "os/signal" "syscall" - "go.uber.org/zap" - "github.com/timescale/tiger-cli/internal/cmd" - "github.com/timescale/tiger-cli/internal/logging" ) func main() { @@ -40,8 +37,7 @@ func run() (err error) { // noifyContext sets up graceful shutdown handling and returns a context and // cleanup function. This is nearly identical to [signal.NotifyContext], except -// that it logs a message when a signal is received and also restores the default -// signal handling behavior. +// that it also restores the default signal handling behavior. func notifyContext(parent context.Context) (context.Context, context.CancelFunc) { ctx, cancel := context.WithCancel(parent) @@ -49,8 +45,7 @@ func notifyContext(parent context.Context) (context.Context, context.CancelFunc) signal.Notify(sigChan, os.Interrupt, syscall.SIGINT, syscall.SIGTERM) go func() { select { - case sig := <-sigChan: - logging.Info("Received interrupt signal, press control-C again to exit", zap.Stringer("signal", sig)) + case <-sigChan: signal.Stop(sigChan) // Restore default signal handling behavior cancel() case <-ctx.Done(): diff --git a/docs/development.md b/docs/development.md index 21bf439a..9fd15e82 100644 --- a/docs/development.md +++ b/docs/development.md @@ -105,7 +105,6 @@ tiger-cli/ │ ├── api/ # Generated OpenAPI client (oapi-codegen) │ │ └── mocks/ # Generated mocks for testing │ ├── config/ # Configuration management -│ ├── logging/ # Structured logging utilities │ ├── mcp/ # MCP server implementation (one file per tool) │ ├── common/ # Shared business logic used by CLI and MCP │ ├── cmd/ # CLI commands (Cobra, one file per command) @@ -130,15 +129,13 @@ Tiger CLI is a Go-based command-line interface for managing Tiger resources. The (`tiger service create` → `service_create.go`). `root.go` holds the root command, global flags, and `wrapCommands`, which gives every command the same per-invocation lifecycle: load config + API client once into a `common.App`, - initialize logging, apply color settings, check for a newer release, and track - analytics. + apply color settings, check for a newer release, and track analytics. - **App**: `internal/common/app.go` - per-invocation config and API client, built once by `wrapCommands` (or per request by the MCP analytics middleware) and read by commands, MCP tool handlers, and completion functions - **Configuration**: `internal/config/config.go` - `Config` struct plus load/write helpers. `config.Load(flags)` resolves values through a per-call viper instance (flag > env > file > default); there is no global config state -- **Logging**: `internal/logging/logging.go` - Structured logging with zap - **API Client**: `internal/api/` - Generated OpenAPI client - **MCP Server**: `internal/mcp/` - Model Context Protocol server implementation. Each MCP tool lives in its own file, named to match the tool @@ -154,9 +151,12 @@ The CLI uses a layered configuration approach (listed from lowest to highest pre ### Logging Architecture -Two-mode logging system using zap: -- **Production mode**: Minimal output, warn level and above, clean formatting -- **Debug mode**: Full development logging with colors and debug level. Enable with `--debug` or `TIGER_DEBUG=true`. +Only the MCP server logs. `tiger mcp start` builds a `*slog.Logger` with +`newLogger(cmd.ErrOrStderr())` (`internal/cmd/logger_helper.go`), which points +the standard `log` package at that writer and returns `slog.Default()`. That +logger is passed to `mcp.NewServer`, which uses it for its own output and hands +it to the MCP SDK (`mcp.ServerOptions.Logger`); a nil logger is discarded. +Everything else writes to stdout/stderr directly rather than logging. ## Code Generation diff --git a/go.mod b/go.mod index cbb5462b..9f919d66 100644 --- a/go.mod +++ b/go.mod @@ -26,7 +26,6 @@ require ( github.com/tailscale/hujson v0.0.0-20250605163823-992244df8c5a github.com/zalando/go-keyring v0.2.6 go.uber.org/mock v0.6.0 - go.uber.org/zap v1.27.1 golang.org/x/oauth2 v0.35.0 golang.org/x/term v0.40.0 golang.org/x/text v0.34.0 @@ -172,6 +171,7 @@ require ( go.opentelemetry.io/otel/trace v1.39.0 // indirect go.opentelemetry.io/proto/otlp v1.9.0 // indirect go.uber.org/multierr v1.11.0 // indirect + go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.3 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.ngrok.com/muxado/v2 v2.0.1 // indirect diff --git a/internal/analytics/analytics.go b/internal/analytics/analytics.go index 6540aaac..d03d456b 100644 --- a/internal/analytics/analytics.go +++ b/internal/analytics/analytics.go @@ -13,8 +13,6 @@ import ( "github.com/spf13/pflag" "github.com/timescale/tiger-cli/internal/api" "github.com/timescale/tiger-cli/internal/config" - "github.com/timescale/tiger-cli/internal/logging" - "go.uber.org/zap" ) // A list of properties that should never be recorded in analytics events. @@ -132,20 +130,14 @@ func (a *Analytics) Identify(options ...Option) { properties["project_id"] = a.projectID } - logger := logging.GetLogger().With( - zap.Any("properties", properties), - ) - // Check if analytics is disabled if !a.Enabled() { - logger.Debug("Analytics identify skipped (analytics disabled)") return } // Check for cases where the client was not initialized // (e.g. because API credentials are not available) if a.client == nil { - logger.Debug("Analytics identify skipped (client not initialized)") return } @@ -155,22 +147,10 @@ func (a *Analytics) Identify(options ...Option) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - // Send the event - resp, err := a.client.IdentifyUserWithResponse(ctx, api.IdentifyUserJSONRequestBody{ + // Send the event, ignoring failures - analytics should never block user actions + a.client.IdentifyUserWithResponse(ctx, api.IdentifyUserJSONRequestBody{ Properties: &properties, }) - if err != nil { - // Log error but don't fail the operation - analytics should never block user actions - logger.Debug("Failed to send analytics identify", zap.Error(err)) - return - } - - if resp.JSON200 == nil || resp.JSON200.Status == nil { - logger.Debug("Failed to retrieve response from analytics endpoint") - return - } - - logger.Debug("Analytics identify sent", zap.String("status", *resp.JSON200.Status)) } // Track sends an analytics event with the provided event name and properties. @@ -196,21 +176,14 @@ func (a *Analytics) Track(event string, options ...Option) { properties["project_id"] = a.projectID } - logger := logging.GetLogger().With( - zap.String("event", event), - zap.Any("properties", properties), - ) - // Check if analytics is disabled if !a.Enabled() { - logger.Debug("Analytics event skipped (analytics disabled)") return } // Check for cases where the client was not initialized // (e.g. because API credentials are not available) if a.client == nil { - logger.Debug("Analytics event skipped (client not initialized)") return } @@ -220,23 +193,11 @@ func (a *Analytics) Track(event string, options ...Option) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - // Send the event - resp, err := a.client.TrackEventWithResponse(ctx, api.TrackEventJSONRequestBody{ + // Send the event, ignoring failures - analytics should never block user actions + a.client.TrackEventWithResponse(ctx, api.TrackEventJSONRequestBody{ Event: event, Properties: &properties, }) - if err != nil { - // Log error but don't fail the operation - analytics should never block user actions - logger.Debug("Failed to send analytics event", zap.Error(err)) - return - } - - if resp.JSON200 == nil || resp.JSON200.Status == nil { - logger.Debug("Failed to retrieve response from analytics endpoint") - return - } - - logger.Debug("Analytics event sent", zap.String("status", *resp.JSON200.Status)) } // Enabled reports whether analytics events will actually be sent given the diff --git a/internal/cmd/completion_helper.go b/internal/cmd/completion_helper.go index 444bfaa8..f0ee4e73 100644 --- a/internal/cmd/completion_helper.go +++ b/internal/cmd/completion_helper.go @@ -99,7 +99,7 @@ func mcpGetCompletion(app *common.App) cobra.CompletionFunc { } // Create MCP server to get capabilities - server, err := mcp.NewServer(cmd.Context(), app) + server, err := mcp.NewServer(cmd.Context(), app, nil) if err != nil { return nil, cobra.ShellCompDirectiveNoFileComp } diff --git a/internal/cmd/config_reset.go b/internal/cmd/config_reset.go index 7185058c..e0500ed4 100644 --- a/internal/cmd/config_reset.go +++ b/internal/cmd/config_reset.go @@ -6,7 +6,6 @@ import ( "github.com/spf13/cobra" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" ) func buildConfigResetCmd(app *common.App) *cobra.Command { @@ -25,7 +24,6 @@ func buildConfigResetCmd(app *common.App) *cobra.Command { return fmt.Errorf("failed to reset config: %w", err) } - logging.Info("Configuration reset to defaults") fmt.Fprintln(cmd.OutOrStdout(), "Configuration reset to defaults") return nil }, diff --git a/internal/cmd/config_set.go b/internal/cmd/config_set.go index 51939e0a..662edea3 100644 --- a/internal/cmd/config_set.go +++ b/internal/cmd/config_set.go @@ -4,10 +4,8 @@ import ( "fmt" "github.com/spf13/cobra" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" ) func buildConfigSetCmd(app *common.App) *cobra.Command { @@ -27,7 +25,6 @@ func buildConfigSetCmd(app *common.App) *cobra.Command { return fmt.Errorf("failed to set config: %w", err) } - logging.Info("Configuration updated", zap.String("key", key), zap.String("value", value)) fmt.Fprintf(cmd.OutOrStdout(), "Set %s = %s\n", key, value) return nil }, diff --git a/internal/cmd/config_show.go b/internal/cmd/config_show.go index 9bb0d46a..1feb32c5 100644 --- a/internal/cmd/config_show.go +++ b/internal/cmd/config_show.go @@ -73,9 +73,6 @@ func outputTable(w io.Writer, cfg *config.ConfigOutput) error { if cfg.ConsoleURL != nil { table.Append("console_url", *cfg.ConsoleURL) } - if cfg.Debug != nil { - table.Append("debug", fmt.Sprintf("%t", *cfg.Debug)) - } if cfg.DocsMCP != nil { table.Append("docs_mcp", fmt.Sprintf("%t", *cfg.DocsMCP)) } diff --git a/internal/cmd/config_show_test.go b/internal/cmd/config_show_test.go index dc3811b9..dd47a36a 100644 --- a/internal/cmd/config_show_test.go +++ b/internal/cmd/config_show_test.go @@ -45,7 +45,6 @@ password_storage: pgpass "output": "table", "analytics": "false", "password_storage": "pgpass", - "debug": "false", "config_dir": tmpDir, "mcp_max_rows": strconv.Itoa(config.DefaultMCPMaxRows), } @@ -98,7 +97,6 @@ password_storage: keyring "analytics": false, "password_storage": "keyring", "read_only": false, - "debug": false, "config_dir": tmpDir, "releases_url": "https://cli.tigerdata.com", "version_check": true, @@ -156,7 +154,6 @@ password_storage: keyring "analytics": false, "password_storage": "keyring", "read_only": false, - "debug": false, "config_dir": tmpDir, "releases_url": "https://cli.tigerdata.com", "version_check": true, diff --git a/internal/cmd/config_unset.go b/internal/cmd/config_unset.go index 5bc88e27..8d5ef34b 100644 --- a/internal/cmd/config_unset.go +++ b/internal/cmd/config_unset.go @@ -4,10 +4,8 @@ import ( "fmt" "github.com/spf13/cobra" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" ) func buildConfigUnsetCmd(app *common.App) *cobra.Command { @@ -27,7 +25,6 @@ func buildConfigUnsetCmd(app *common.App) *cobra.Command { return fmt.Errorf("failed to unset config: %w", err) } - logging.Info("Configuration updated", zap.String("key", key)) fmt.Fprintf(cmd.OutOrStdout(), "Unset %s\n", key) return nil }, diff --git a/internal/cmd/logger_helper.go b/internal/cmd/logger_helper.go new file mode 100644 index 00000000..e6234458 --- /dev/null +++ b/internal/cmd/logger_helper.go @@ -0,0 +1,16 @@ +package cmd + +import ( + "io" + "log" + "log/slog" +) + +// newLogger configures the default log package to write to w and returns the +// default slog logger. It's intended for the MCP server, the only long-running, +// backend-like process the CLI hosts; ordinary commands write to +// stdout/stderr directly rather than logging. +func newLogger(w io.Writer) *slog.Logger { + log.SetOutput(w) + return slog.Default() +} diff --git a/internal/cmd/mcp_get.go b/internal/cmd/mcp_get.go index 09beeb77..88d1bc17 100644 --- a/internal/cmd/mcp_get.go +++ b/internal/cmd/mcp_get.go @@ -47,7 +47,7 @@ Examples: cfg := app.GetConfig() // Create MCP server - server, err := mcp.NewServer(cmd.Context(), app) + server, err := mcp.NewServer(cmd.Context(), app, nil) if err != nil { return fmt.Errorf("failed to create MCP server: %w", err) } diff --git a/internal/cmd/mcp_install.go b/internal/cmd/mcp_install.go index 3dd32149..f6d02d9c 100644 --- a/internal/cmd/mcp_install.go +++ b/internal/cmd/mcp_install.go @@ -17,10 +17,8 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/spf13/cobra" "github.com/tailscale/hujson" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/mcp" "github.com/timescale/tiger-cli/internal/util" ) @@ -321,16 +319,6 @@ func InstallMCPForClient(opts InstallOptions) error { } // else: CLI-only client - configPath remains empty, will use buildInstallCommand - logging.Info("Installing MCP server configuration", - zap.String("client", opts.ClientName), - zap.String("server_name", opts.ServerName), - zap.String("command", opts.Command), - zap.Strings("args", opts.Args), - zap.String("config_path", configPath), - zap.String("mcp_servers_path", mcpServersPathPrefix), - zap.Bool("create_backup", opts.CreateBackup), - ) - // Create backup if requested and we have a config file if opts.CreateBackup && configPath != "" { _, err = createConfigBackup(configPath) @@ -456,7 +444,6 @@ func findClientConfigFile(configPaths []string) (string, error) { // Check if file exists if _, err := os.Stat(expandedPath); err == nil { - logging.Info("Found existing config file", zap.String("path", expandedPath)) return expandedPath, nil } } @@ -467,8 +454,6 @@ func findClientConfigFile(configPaths []string) (string, error) { } defaultPath := util.ExpandPath(configPaths[0]) // Use first path as default - logging.Info("No existing config found, will create at default location", - zap.String("path", defaultPath)) return defaultPath, nil } @@ -641,10 +626,6 @@ func addMCPServerViaCLI(clientCfg *clientConfig, serverName, command string, arg return fmt.Errorf("failed to build install command: %w", err) } - logging.Info("Adding MCP server using CLI", - zap.String("client", clientCfg.Name), - zap.Strings("command", installCommand)) - // Run the configured CLI command cmd := exec.Command(installCommand[0], installCommand[1:]...) @@ -658,10 +639,6 @@ func addMCPServerViaCLI(clientCfg *clientConfig, serverName, command string, arg return fmt.Errorf("failed to run %s installation command: %w\nCommand: %s", clientCfg.Name, err, cmdStr) } - logging.Info("Successfully added MCP server via CLI", - zap.String("client", clientCfg.Name), - zap.String("output", string(output))) - return nil } @@ -670,7 +647,6 @@ func createConfigBackup(configPath string) (string, error) { // Check if config file exists if _, err := os.Stat(configPath); errors.Is(err, fs.ErrNotExist) { // No existing config file, no backup needed - logging.Info("No existing configuration file found, skipping backup") return "", nil } @@ -694,7 +670,6 @@ func createConfigBackup(configPath string) (string, error) { return "", fmt.Errorf("failed to write backup file: %w", err) } - logging.Info("Created configuration backup", zap.String("backup_path", backupPath)) return backupPath, nil } @@ -724,7 +699,6 @@ func addMCPServerViaJSON(configPath, mcpServersPathPrefix, serverName, command s if !errors.Is(err, fs.ErrNotExist) { return fmt.Errorf("failed to read config file: %w", err) } - logging.Info("Config file not found, creating new one") content = []byte("{}") } @@ -774,11 +748,5 @@ func addMCPServerViaJSON(configPath, mcpServersPathPrefix, serverName, command s return fmt.Errorf("failed to write config file: %w", err) } - logging.Info("Added MCP server to configuration", - zap.String("server_name", serverName), - zap.String("command", serverConfig.Command), - zap.Strings("args", serverConfig.Args), - ) - return nil } diff --git a/internal/cmd/mcp_list.go b/internal/cmd/mcp_list.go index 0f095f5b..ba18624a 100644 --- a/internal/cmd/mcp_list.go +++ b/internal/cmd/mcp_list.go @@ -39,7 +39,7 @@ Examples: cfg := app.GetConfig() // Create MCP server - server, err := mcp.NewServer(cmd.Context(), app) + server, err := mcp.NewServer(cmd.Context(), app, nil) if err != nil { return fmt.Errorf("failed to create MCP server: %w", err) } diff --git a/internal/cmd/mcp_start.go b/internal/cmd/mcp_start.go index 8e531aff..b93c70cc 100644 --- a/internal/cmd/mcp_start.go +++ b/internal/cmd/mcp_start.go @@ -4,12 +4,11 @@ import ( "context" "errors" "fmt" + "log/slog" "github.com/spf13/cobra" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/mcp" ) @@ -37,7 +36,7 @@ Examples: RunE: func(cmd *cobra.Command, args []string) error { // Default behavior when no subcommand is specified - use stdio cmd.SilenceUsage = true - return startStdioServer(cmd.Context(), app) + return startStdioServer(cmd, app) }, } @@ -49,11 +48,14 @@ Examples: } // startStdioServer starts the MCP server with stdio transport -func startStdioServer(ctx context.Context, app *common.App) error { - logging.Info("Starting Tiger MCP server", zap.String("transport", "stdio")) +func startStdioServer(cmd *cobra.Command, app *common.App) error { + ctx := cmd.Context() + logger := newLogger(cmd.ErrOrStderr()) + + logger.Info("Starting Tiger MCP server", slog.String("transport", "stdio")) // Create MCP server - server, err := mcp.NewServer(ctx, app) + server, err := mcp.NewServer(ctx, app, logger) if err != nil { return fmt.Errorf("failed to create MCP server: %w", err) } diff --git a/internal/cmd/mcp_start_http.go b/internal/cmd/mcp_start_http.go index 5e82097b..faee92ab 100644 --- a/internal/cmd/mcp_start_http.go +++ b/internal/cmd/mcp_start_http.go @@ -3,14 +3,13 @@ package cmd import ( "context" "fmt" + "log/slog" "net" "net/http" "github.com/spf13/cobra" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/mcp" ) @@ -42,7 +41,7 @@ Examples: ValidArgsFunction: cobra.NoFileCompletions, RunE: func(cmd *cobra.Command, args []string) error { cmd.SilenceUsage = true - return startHTTPServer(cmd.Context(), app, httpHost, httpPort) + return startHTTPServer(cmd, app, httpHost, httpPort) }, } @@ -54,11 +53,14 @@ Examples: } // startHTTPServer starts the MCP server with HTTP transport -func startHTTPServer(ctx context.Context, app *common.App, host string, port int) error { - logging.Info("Starting Tiger MCP server", zap.String("transport", "http")) +func startHTTPServer(cmd *cobra.Command, app *common.App, host string, port int) error { + ctx := cmd.Context() + logger := newLogger(cmd.ErrOrStderr()) + + logger.Info("Starting Tiger MCP server", slog.String("transport", "http")) // Create MCP server - server, err := mcp.NewServer(ctx, app) + server, err := mcp.NewServer(ctx, app, logger) if err != nil { return fmt.Errorf("failed to create MCP server: %w", err) } @@ -72,9 +74,9 @@ func startHTTPServer(ctx context.Context, app *common.App, host string, port int defer listener.Close() if actualPort != port { - logging.Info("Specified port was busy, using alternative port", - zap.Int("requested_port", port), - zap.Int("actual_port", actualPort), + logger.Info("Specified port was busy, using alternative port", + slog.Int("requested_port", port), + slog.Int("actual_port", actualPort), ) } @@ -85,13 +87,13 @@ func startHTTPServer(ctx context.Context, app *common.App, host string, port int Handler: server.HTTPHandler(), } - fmt.Printf("🚀 Tiger MCP server listening on http://%s\n", address) - fmt.Printf("💡 Use Ctrl+C to stop the server\n") + logger.Info("Tiger MCP server started", slog.String("address", address)) + logger.Info("Use Ctrl+C to stop the server") // Start server in goroutine using the existing listener go func() { if err := httpServer.Serve(listener); err != nil && err != http.ErrServerClosed { - logging.Error("HTTP server error", zap.Error(err)) + logger.Error("HTTP server error", slog.Any("error", err)) } }() @@ -104,7 +106,7 @@ func startHTTPServer(ctx context.Context, app *common.App, host string, port int <-ctx.Done() // Shutdown server gracefully - logging.Info("Gracefully shutting down HTTP server..., press control-C twice to immediately shutdown") + logger.Info("Gracefully shutting down HTTP server, press control-C twice to immediately shutdown") if err := httpServer.Shutdown(context.Background()); err != nil { return fmt.Errorf("failed to shut down HTTP server: %w", err) } diff --git a/internal/cmd/mcp_start_stdio.go b/internal/cmd/mcp_start_stdio.go index f21f02e1..a4d449bb 100644 --- a/internal/cmd/mcp_start_stdio.go +++ b/internal/cmd/mcp_start_stdio.go @@ -20,7 +20,7 @@ Examples: ValidArgsFunction: cobra.NoFileCompletions, RunE: func(cmd *cobra.Command, args []string) error { cmd.SilenceUsage = true - return startStdioServer(cmd.Context(), app) + return startStdioServer(cmd, app) }, } } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index d977ee01..6b3214c6 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -9,12 +9,10 @@ import ( "github.com/fatih/color" "github.com/spf13/cobra" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/analytics" "github.com/timescale/tiger-cli/internal/common" "github.com/timescale/tiger-cli/internal/config" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" "github.com/timescale/tiger-cli/internal/version" ) @@ -56,7 +54,6 @@ tiger auth login cmd.PersistentFlags().Bool("analytics", true, "enable/disable usage analytics") cmd.PersistentFlags().Bool("color", true, "enable colored output") cmd.PersistentFlags().String("config-dir", config.GetDefaultConfigDir(), "config directory") - cmd.PersistentFlags().Bool("debug", false, "enable debug logging") cmd.PersistentFlags().String("password-storage", config.DefaultPasswordStorage, "password storage method (keyring, pgpass, none)") cmd.PersistentFlags().String("service-id", "", "service ID") skipUpdateCheck := cmd.PersistentFlags().Bool("skip-update-check", false, "skip checking for updates on startup") @@ -77,8 +74,8 @@ tiger auth login // wrapCommands recursively wraps the RunE of every command in the tree rooted at // cmd with the shared per-invocation lifecycle: loading the config and API -// client, initializing logging, configuring color output, checking for a newer -// release, and tracking analytics. +// client, configuring color output, checking for a newer release, and tracking +// analytics. // // Commands added to the tree after this runs (cobra's built-in help, completion, // and __complete commands) are not wrapped and so skip the load entirely, which @@ -100,17 +97,6 @@ func wrapCommands(cmd *cobra.Command, app *common.App, skipUpdateCheck *bool) { return err } - if err := logging.Init(cfg.Debug); err != nil { - return fmt.Errorf("failed to initialize logging: %w", err) - } - defer logging.Sync() - - logging.Debug("CLI initialized", - zap.String("config_dir", cfg.ConfigDir), - zap.String("output", cfg.Output), - zap.Bool("debug", cfg.Debug), - ) - if !cfg.Color { color.NoColor = true } @@ -127,7 +113,8 @@ func wrapCommands(cmd *cobra.Command, app *common.App, skipUpdateCheck *bool) { defer func() { cfg, client, projectID := app.TryGetAll() a := analytics.New(cfg, client, projectID) - a.Track(fmt.Sprintf("Run %s", c.CommandPath()), + a.Track( + fmt.Sprintf("Run %s", c.CommandPath()), analytics.Property("args", args), // NOTE: Safe right now, but might need allow-list in the future if some args end up containing sensitive info analytics.Property("elapsed_seconds", time.Since(start).Seconds()), analytics.FlagSet(c.Flags()), @@ -160,21 +147,18 @@ func versionCheck(cmd *cobra.Command, cfg *config.Config, skipUpdateCheck bool) return func() {} } - resultCh := make(chan *version.CheckResult, 1) + type checkResult struct { + result *version.CheckResult + err error + } + resultCh := make(chan checkResult, 1) go func() { result, err := version.CheckForUpdate(cfg) - if err != nil { - // A failed check (e.g. offline) shouldn't spam a warning on every - // command; surface it only in debug logs. - logging.Debug("background version check failed", zap.Error(err)) - resultCh <- nil - return - } - resultCh <- result + resultCh <- checkResult{result: result, err: err} }() return func() { - result := <-resultCh + res := <-resultCh // Re-check cfg.VersionCheck: the command may have turned checks off in // place (e.g. `tiger config set version_check false`, which reloads the @@ -183,8 +167,13 @@ func versionCheck(cmd *cobra.Command, cfg *config.Config, skipUpdateCheck bool) return } + if res.err != nil { + cmd.PrintErrf("Warning: failed to check for updates: %v\n", res.err) + return + } + output := cmd.ErrOrStderr() - version.PrintUpdateWarning(result, cfg, &output) + version.PrintUpdateWarning(res.result, cfg, &output) } } diff --git a/internal/cmd/root_test.go b/internal/cmd/root_test.go index 020b1e47..97d6b17d 100644 --- a/internal/cmd/root_test.go +++ b/internal/cmd/root_test.go @@ -110,16 +110,12 @@ analytics: true "--config-dir", tmpDir, "--service-id", "flag-service", "--analytics=false", - "--debug", "version", // Need a subcommand to execute ) if cfg.ServiceID != "flag-service" { t.Errorf("Expected service_id 'flag-service', got '%s'", cfg.ServiceID) } - if !cfg.Debug { - t.Error("Expected debug true from flag, got false") - } if cfg.ConfigDir != tmpDir { t.Errorf("Expected config dir '%s' from flag, got '%s'", tmpDir, cfg.ConfigDir) } diff --git a/internal/common/client_test.go b/internal/common/client_test.go index 41b4dec7..e57e88c4 100644 --- a/internal/common/client_test.go +++ b/internal/common/client_test.go @@ -12,16 +12,11 @@ import ( "github.com/timescale/tiger-cli/internal/api" "github.com/timescale/tiger-cli/internal/config" - "github.com/timescale/tiger-cli/internal/logging" ) // TestNewAPIClient_OAuthCredentials verifies that when stored credentials are // OAuth-shaped, NewAPIClient builds a Bearer-authenticated client and returns the stored project ID. func TestNewAPIClient_OAuthCredentials(t *testing.T) { - if err := logging.Init(false); err != nil { - t.Fatalf("Failed to initialize logging: %v", err) - } - // Ensure env-var credentials don't take precedence over the stored override. t.Setenv("TIGER_PUBLIC_KEY", "") t.Setenv("TIGER_SECRET_KEY", "") @@ -63,10 +58,6 @@ func TestNewAPIClient_OAuthCredentials(t *testing.T) { func TestValidateAPIKey(t *testing.T) { // Initialize logger for analytics code - if err := logging.Init(false); err != nil { - t.Fatalf("Failed to initialize logging: %v", err) - } - tests := []struct { name string setupServer func() *httptest.Server diff --git a/internal/config/config.go b/internal/config/config.go index 0e7f75a5..4523bede 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -22,7 +22,6 @@ const ( DefaultAnalytics = true DefaultColor = true DefaultConsoleURL = "https://console.cloud.tigerdata.com" - DefaultDebug = false DefaultDocsMCP = true DefaultDocsMCPURL = "https://mcp.tigerdata.com/docs?disabled_skills=ghost-database" DefaultGatewayURL = "https://console.cloud.tigerdata.com/api" @@ -44,7 +43,6 @@ var defaultValues = map[string]any{ "api_url": DefaultAPIURL, "color": DefaultColor, "console_url": DefaultConsoleURL, - "debug": DefaultDebug, "docs_mcp": DefaultDocsMCP, "docs_mcp_url": DefaultDocsMCPURL, "gateway_url": DefaultGatewayURL, @@ -63,7 +61,6 @@ var defaultValues = map[string]any{ var flagBindings = map[string]string{ "analytics": "analytics", "color": "color", - "debug": "debug", "output": "output", "password-storage": "password_storage", "service-id": "service_id", @@ -76,7 +73,6 @@ type Config struct { Analytics bool `mapstructure:"analytics"` Color bool `mapstructure:"color"` ConsoleURL string `mapstructure:"console_url"` - Debug bool `mapstructure:"debug"` DocsMCP bool `mapstructure:"docs_mcp"` DocsMCPURL string `mapstructure:"docs_mcp_url"` GatewayURL string `mapstructure:"gateway_url"` @@ -100,7 +96,6 @@ type ConfigOutput struct { Color *bool `mapstructure:"color" json:"color,omitempty"` ConfigDir *string `mapstructure:"-" json:"config_dir,omitempty"` ConsoleURL *string `mapstructure:"console_url" json:"console_url,omitempty"` - Debug *bool `mapstructure:"debug" json:"debug,omitempty"` DocsMCP *bool `mapstructure:"docs_mcp" json:"docs_mcp,omitempty"` DocsMCPURL *string `mapstructure:"docs_mcp_url" json:"docs_mcp_url,omitempty"` GatewayURL *string `mapstructure:"gateway_url" json:"gateway_url,omitempty"` @@ -367,7 +362,7 @@ func validateValue(key, value string) (any, error) { switch key { case "api_url", "console_url", "docs_mcp_url", "gateway_url", "releases_url", "service_id": return value, nil - case "analytics", "color", "debug", "docs_mcp", "read_only", "version_check": + case "analytics", "color", "docs_mcp", "read_only", "version_check": return parseBool(key, value) case "mcp_max_rows": return parsePositiveInt(key, value) diff --git a/internal/logging/logging.go b/internal/logging/logging.go deleted file mode 100644 index 37b9a3af..00000000 --- a/internal/logging/logging.go +++ /dev/null @@ -1,81 +0,0 @@ -package logging - -import ( - "os" - - "go.uber.org/zap" - "go.uber.org/zap/zapcore" -) - -var logger *zap.Logger - -func Init(debug bool) error { - var config zap.Config - - if debug { - config = zap.NewDevelopmentConfig() - config.Level = zap.NewAtomicLevelAt(zap.DebugLevel) - config.EncoderConfig.EncodeLevel = zapcore.CapitalColorLevelEncoder - } else { - config = zap.NewProductionConfig() - config.Level = zap.NewAtomicLevelAt(zap.WarnLevel) - config.EncoderConfig.TimeKey = "" - config.EncoderConfig.LevelKey = "" - config.EncoderConfig.CallerKey = "" - config.EncoderConfig.MessageKey = "message" - config.EncoderConfig.StacktraceKey = "" - } - - config.OutputPaths = []string{"stderr"} - config.ErrorOutputPaths = []string{"stderr"} - - var err error - logger, err = config.Build() - if err != nil { - return err - } - - return nil -} - -func Debug(msg string, fields ...zap.Field) { - if logger != nil { - logger.Debug(msg, fields...) - } -} - -func Info(msg string, fields ...zap.Field) { - if logger != nil { - logger.Info(msg, fields...) - } -} - -func Warn(msg string, fields ...zap.Field) { - if logger != nil { - logger.Warn(msg, fields...) - } -} - -func Error(msg string, fields ...zap.Field) { - if logger != nil { - logger.Error(msg, fields...) - } -} - -func Fatal(msg string, fields ...zap.Field) { - if logger != nil { - logger.Fatal(msg, fields...) - } else { - os.Exit(1) - } -} - -func Sync() { - if logger != nil { - logger.Sync() - } -} - -func GetLogger() *zap.Logger { - return logger -} diff --git a/internal/mcp/capabilities.go b/internal/mcp/capabilities.go index 0d4b7741..a8b38efc 100644 --- a/internal/mcp/capabilities.go +++ b/internal/mcp/capabilities.go @@ -6,8 +6,6 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/timescale/tiger-cli/internal/config" - "github.com/timescale/tiger-cli/internal/logging" - "go.uber.org/zap" ) // Capabilities holds all MCP server capabilities @@ -26,7 +24,9 @@ func (s *Server) ListCapabilities(ctx context.Context) (*Capabilities, error) { client := mcp.NewClient(&mcp.Implementation{ Name: ServerName, Version: config.Version, - }, nil) + }, &mcp.ClientOptions{ + Logger: s.logger, + }) serverSession, err := s.mcpServer.Connect(ctx, serverTransport, nil) if err != nil { @@ -76,11 +76,11 @@ func (s *Server) ListCapabilities(ctx context.Context) (*Capabilities, error) { } if err := clientSession.Close(); err != nil { - logging.Error("Error closing client session", zap.Error(err)) + return nil, fmt.Errorf("error closing client session: %w", err) } if err := serverSession.Close(); err != nil { - logging.Error("Error closing server session", zap.Error(err)) + return nil, fmt.Errorf("error closing server session: %w", err) } return capabilities, nil diff --git a/internal/mcp/db_execute_query.go b/internal/mcp/db_execute_query.go index 46a3f085..2dd6ea99 100644 --- a/internal/mcp/db_execute_query.go +++ b/internal/mcp/db_execute_query.go @@ -4,16 +4,15 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/jackc/pgx/v5" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" "github.com/timescale/tiger-cli/internal/config" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -159,13 +158,13 @@ func (s *Server) handleDBExecuteQuery(ctx context.Context, req *mcp.CallToolRequ // Convert timeout in seconds to time.Duration timeout := time.Duration(input.TimeoutSeconds) * time.Second - logging.Debug("MCP: Executing database query", - zap.String("project_id", projectID), - zap.String("service_id", input.ServiceID), - zap.Duration("timeout", timeout), - zap.String("role", input.Role), - zap.Bool("pooled", input.Pooled), - zap.Bool("read_only", cfg.ReadOnly), + s.logger.Info("MCP: Executing database query", + slog.String("project_id", projectID), + slog.String("service_id", input.ServiceID), + slog.Duration("timeout", timeout), + slog.String("role", input.Role), + slog.Bool("pooled", input.Pooled), + slog.Bool("read_only", cfg.ReadOnly), ) // service_id may name a service or one of its read replicas. diff --git a/internal/mcp/db_schema.go b/internal/mcp/db_schema.go index 7aee5929..6e63d646 100644 --- a/internal/mcp/db_schema.go +++ b/internal/mcp/db_schema.go @@ -3,13 +3,12 @@ package mcp import ( "context" "encoding/json" + "log/slog" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -96,15 +95,15 @@ func (s *Server) handleDBSchema(ctx context.Context, req *mcp.CallToolRequest, i return nil, DBSchemaOutput{}, err } - logging.Debug("MCP: Getting database schema", - zap.String("project_id", projectID), - zap.String("service_id", input.ServiceID), - zap.String("schema", input.SchemaName), - zap.Bool("internal", input.Internal), - zap.Bool("definitions", input.Definitions), - zap.Bool("comments", input.Comments), - zap.String("role", input.Role), - zap.Bool("pooled", input.Pooled), + s.logger.Info("MCP: Getting database schema", + slog.String("project_id", projectID), + slog.String("service_id", input.ServiceID), + slog.String("schema", input.SchemaName), + slog.Bool("internal", input.Internal), + slog.Bool("definitions", input.Definitions), + slog.Bool("comments", input.Comments), + slog.String("role", input.Role), + slog.Bool("pooled", input.Pooled), ) // service_id may name a service or one of its read replicas. diff --git a/internal/mcp/proxy.go b/internal/mcp/proxy.go index dd2dc32a..486b6c33 100644 --- a/internal/mcp/proxy.go +++ b/internal/mcp/proxy.go @@ -3,14 +3,13 @@ package mcp import ( "context" "fmt" + "log/slog" "strings" "time" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/config" - "github.com/timescale/tiger-cli/internal/logging" ) // isMethodNotFoundError checks if the error is a JSON-RPC "Method not found" error. @@ -55,56 +54,64 @@ func (s *Server) registerDocsProxy(ctx context.Context) { cfg := s.app.GetConfig() if !cfg.DocsMCP || cfg.DocsMCPURL == "" { - logging.Debug("Docs MCP proxy is disabled") + s.logger.Info("Docs MCP proxy is disabled") return } - logging.Info("Setting up docs MCP proxy connection", - zap.String("url", cfg.DocsMCPURL), + s.logger.Info("Setting up docs MCP proxy connection", + slog.String("url", cfg.DocsMCPURL), ) // Create timeout for establishing proxy ctx, cancel := context.WithTimeout(ctx, time.Minute) defer cancel() - proxyClient, err := NewProxyClient(ctx, cfg.DocsMCPURL) + proxyClient, err := NewProxyClient(ctx, cfg.DocsMCPURL, s.logger) if err != nil { - logging.Error("Failed to connect to docs MCP server", - zap.String("url", cfg.DocsMCPURL), - zap.Error(err), + s.logger.Error("Failed to connect to docs MCP server", + slog.String("url", cfg.DocsMCPURL), + slog.Any("error", err), ) return } s.docsProxyClient = proxyClient if err := proxyClient.RegisterTools(ctx, s.mcpServer); err != nil { - logging.Error("Failed to register tools from docs MCP server", zap.Error(err)) + s.logger.Error("Failed to register tools from docs MCP server", + slog.Any("error", err), + ) } if err := proxyClient.RegisterResources(ctx, s.mcpServer); err != nil { // Check if this is a "Method not found" error as those are expected in servers that don't have any resources if isMethodNotFoundError(err) { - logging.Debug("Resources not supported by remote MCP server") + s.logger.Info("Resources not supported by remote MCP server") } else { - logging.Error("Failed to register resources from docs MCP server", zap.Error(err)) + s.logger.Error("Failed to register resources from docs MCP server", + slog.Any("error", err), + ) } } if err := proxyClient.RegisterResourceTemplates(ctx, s.mcpServer); err != nil { // Check if this is a "Method not found" error as those are expected in servers that don't have any resource templates if isMethodNotFoundError(err) { - logging.Debug("Resource templates not supported by remote MCP server") + s.logger.Info("Resource templates not supported by remote MCP server") } else { - logging.Error("Failed to register resource templates from docs MCP server", zap.Error(err)) + s.logger.Error("Failed to register resource templates from docs MCP server", + slog.Any("error", err), + ) } } if err := proxyClient.RegisterPrompts(ctx, s.mcpServer); err != nil { - logging.Error("Failed to register prompts from docs MCP server", zap.Error(err)) + s.logger.Error("Failed to register prompts from docs MCP server", + slog.Any("error", err), + ) } - logging.Info("Successfully connected to docs MCP server", - zap.String("url", cfg.DocsMCPURL), + s.logger.Info("Successfully connected to docs MCP server", + slog.String("url", cfg.DocsMCPURL), ) } @@ -113,12 +120,15 @@ type ProxyClient struct { url string client *mcp.Client session *mcp.ClientSession + logger *slog.Logger } // NewProxyClient creates a new proxy client for the given remote server configuration -func NewProxyClient(ctx context.Context, url string) (*ProxyClient, error) { - logging.Debug("Connecting to docs MCP server", - zap.String("url", url), +func NewProxyClient(ctx context.Context, url string, logger *slog.Logger) (*ProxyClient, error) { + logger = ensureLogger(logger) + + logger.Info("Connecting to docs MCP server", + slog.String("url", url), ) transport := &mcp.StreamableClientTransport{ @@ -129,19 +139,22 @@ func NewProxyClient(ctx context.Context, url string) (*ProxyClient, error) { Name: "tiger-mcp-proxy-client", Title: "Tiger MCP Proxy Client", Version: config.Version, - }, nil) + }, &mcp.ClientOptions{ + Logger: logger, + }) session, err := client.Connect(ctx, transport, nil) if err != nil { return nil, fmt.Errorf("failed to connect to remote MCP server: %w", err) } - logging.Info("Successfully connected to docs MCP server") + logger.Info("Successfully connected to docs MCP server") return &ProxyClient{ url: url, client: client, session: session, + logger: logger, }, nil } @@ -151,7 +164,7 @@ func (p *ProxyClient) RegisterTools(ctx context.Context, server *mcp.Server) err return fmt.Errorf("not connected to remote server") } - logging.Debug("Discovering tools from remote MCP server") + p.logger.Info("Discovering tools from remote MCP server") // List tools from remote server toolsResp, err := p.session.ListTools(ctx, &mcp.ListToolsParams{}) @@ -160,14 +173,14 @@ func (p *ProxyClient) RegisterTools(ctx context.Context, server *mcp.Server) err } if toolsResp == nil || len(toolsResp.Tools) == 0 { - logging.Debug("No tools found on remote server") + p.logger.Info("No tools found on remote server") return nil } // Register each remote tool as a proxy tool for _, tool := range toolsResp.Tools { if tool.Name == "" { - logging.Warn("Skipping tool with empty name") + p.logger.Warn("Skipping tool with empty name") continue } @@ -177,13 +190,13 @@ func (p *ProxyClient) RegisterTools(ctx context.Context, server *mcp.Server) err // Register the proxy tool with our MCP server server.AddTool(tool, handler) - logging.Debug("Registered proxy tool", - zap.String("name", tool.Name), + p.logger.Info("Registered proxy tool", + slog.String("name", tool.Name), ) } - logging.Info("Successfully registered proxy tools", - zap.Int("count", len(toolsResp.Tools)), + p.logger.Info("Successfully registered proxy tools", + slog.Int("count", len(toolsResp.Tools)), ) return nil } @@ -191,8 +204,8 @@ func (p *ProxyClient) RegisterTools(ctx context.Context, server *mcp.Server) err // createProxyToolHandler creates a handler function that forwards tool calls to the remote server func (p *ProxyClient) createProxyToolHandler() mcp.ToolHandler { return func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) { - logging.Debug("Proxying tool call to remote server", - zap.String("tool_name", req.Params.Name), + p.logger.Info("Proxying tool call to remote server", + slog.String("tool_name", req.Params.Name), ) if p.session == nil { @@ -209,15 +222,15 @@ func (p *ProxyClient) createProxyToolHandler() mcp.ToolHandler { // Call remote tool result, err := p.session.CallTool(ctx, params) if err != nil { - logging.Error("Remote tool call failed", - zap.String("tool_name", req.Params.Name), - zap.Error(err), + p.logger.Error("Remote tool call failed", + slog.String("tool_name", req.Params.Name), + slog.Any("error", err), ) return nil, fmt.Errorf("remote tool call failed: %w", err) } - logging.Debug("Remote tool call successful", - zap.String("tool_name", req.Params.Name), + p.logger.Info("Remote tool call successful", + slog.String("tool_name", req.Params.Name), ) return result, nil @@ -231,7 +244,7 @@ func (p *ProxyClient) RegisterResources(ctx context.Context, server *mcp.Server) return fmt.Errorf("not connected to remote server") } - logging.Debug("Discovering resources from remote MCP server") + p.logger.Info("Discovering resources from remote MCP server") // List resources from remote server resourcesResp, err := p.session.ListResources(ctx, &mcp.ListResourcesParams{}) @@ -240,14 +253,14 @@ func (p *ProxyClient) RegisterResources(ctx context.Context, server *mcp.Server) } if resourcesResp == nil || len(resourcesResp.Resources) == 0 { - logging.Debug("No resources found on remote server") + p.logger.Info("No resources found on remote server") return nil } // Register each remote resource as a proxy resource for _, resource := range resourcesResp.Resources { if resource.URI == "" { - logging.Warn("Skipping resource with empty URI") + p.logger.Warn("Skipping resource with empty URI") continue } @@ -257,13 +270,13 @@ func (p *ProxyClient) RegisterResources(ctx context.Context, server *mcp.Server) // Register the proxy resource with our MCP server server.AddResource(resource, handler) - logging.Debug("Registered proxy resource", - zap.String("uri", resource.URI), + p.logger.Info("Registered proxy resource", + slog.String("uri", resource.URI), ) } - logging.Info("Successfully registered proxy resources", - zap.Int("count", len(resourcesResp.Resources)), + p.logger.Info("Successfully registered proxy resources", + slog.Int("count", len(resourcesResp.Resources)), ) return nil } @@ -274,7 +287,7 @@ func (p *ProxyClient) RegisterResourceTemplates(ctx context.Context, server *mcp return fmt.Errorf("not connected to remote server") } - logging.Debug("Discovering resource templates from remote MCP server") + p.logger.Info("Discovering resource templates from remote MCP server") // List resource templates from remote server templatesResp, err := p.session.ListResourceTemplates(ctx, &mcp.ListResourceTemplatesParams{}) @@ -283,14 +296,14 @@ func (p *ProxyClient) RegisterResourceTemplates(ctx context.Context, server *mcp } if templatesResp == nil || len(templatesResp.ResourceTemplates) == 0 { - logging.Debug("No resource templates found on remote server") + p.logger.Info("No resource templates found on remote server") return nil } // Register each remote resource template as a proxy resource template for _, resourceTemplate := range templatesResp.ResourceTemplates { if resourceTemplate.URITemplate == "" { - logging.Warn("Skipping resource template with empty URI template") + p.logger.Warn("Skipping resource template with empty URI template") continue } @@ -300,13 +313,13 @@ func (p *ProxyClient) RegisterResourceTemplates(ctx context.Context, server *mcp // Register the proxy resource template with our MCP server server.AddResourceTemplate(resourceTemplate, handler) - logging.Debug("Registered proxy resource template", - zap.String("uri_template", resourceTemplate.URITemplate), + p.logger.Info("Registered proxy resource template", + slog.String("uri_template", resourceTemplate.URITemplate), ) } - logging.Info("Successfully registered proxy resource templates", - zap.Int("count", len(templatesResp.ResourceTemplates)), + p.logger.Info("Successfully registered proxy resource templates", + slog.Int("count", len(templatesResp.ResourceTemplates)), ) return nil } @@ -314,8 +327,8 @@ func (p *ProxyClient) RegisterResourceTemplates(ctx context.Context, server *mcp // createProxyResourceHandler creates a handler function that forwards resource reads to the remote server func (p *ProxyClient) createProxyResourceHandler() mcp.ResourceHandler { return func(ctx context.Context, req *mcp.ReadResourceRequest) (*mcp.ReadResourceResult, error) { - logging.Debug("Proxying resource read to remote server", - zap.String("resource_uri", req.Params.URI), + p.logger.Info("Proxying resource read to remote server", + slog.String("resource_uri", req.Params.URI), ) if p.session == nil { @@ -325,15 +338,15 @@ func (p *ProxyClient) createProxyResourceHandler() mcp.ResourceHandler { // Call remote resource result, err := p.session.ReadResource(ctx, req.Params) if err != nil { - logging.Error("Remote resource read failed", - zap.String("resource_uri", req.Params.URI), - zap.Error(err), + p.logger.Error("Remote resource read failed", + slog.String("resource_uri", req.Params.URI), + slog.Any("error", err), ) return nil, fmt.Errorf("remote resource read failed: %w", err) } - logging.Debug("Remote resource read successful", - zap.String("resource_uri", req.Params.URI), + p.logger.Info("Remote resource read successful", + slog.String("resource_uri", req.Params.URI), ) return result, nil @@ -346,7 +359,7 @@ func (p *ProxyClient) RegisterPrompts(ctx context.Context, server *mcp.Server) e return fmt.Errorf("not connected to remote server") } - logging.Debug("Discovering prompts from remote MCP server") + p.logger.Info("Discovering prompts from remote MCP server") // List prompts from remote server promptsResp, err := p.session.ListPrompts(ctx, &mcp.ListPromptsParams{}) @@ -355,14 +368,14 @@ func (p *ProxyClient) RegisterPrompts(ctx context.Context, server *mcp.Server) e } if promptsResp == nil || len(promptsResp.Prompts) == 0 { - logging.Debug("No prompts found on remote server") + p.logger.Info("No prompts found on remote server") return nil } // Register each remote prompt as a proxy prompt for _, prompt := range promptsResp.Prompts { if prompt.Name == "" { - logging.Warn("Skipping prompt with empty name") + p.logger.Warn("Skipping prompt with empty name") continue } @@ -372,13 +385,13 @@ func (p *ProxyClient) RegisterPrompts(ctx context.Context, server *mcp.Server) e // Register the proxy prompt with our MCP server server.AddPrompt(prompt, handler) - logging.Debug("Registered proxy prompt", - zap.String("original_name", prompt.Name), + p.logger.Info("Registered proxy prompt", + slog.String("original_name", prompt.Name), ) } - logging.Info("Successfully registered proxy prompts", - zap.Int("count", len(promptsResp.Prompts)), + p.logger.Info("Successfully registered proxy prompts", + slog.Int("count", len(promptsResp.Prompts)), ) return nil } @@ -386,8 +399,8 @@ func (p *ProxyClient) RegisterPrompts(ctx context.Context, server *mcp.Server) e // createProxyPromptHandler creates a handler function that forwards prompt requests to the remote server func (p *ProxyClient) createProxyPromptHandler() mcp.PromptHandler { return func(ctx context.Context, req *mcp.GetPromptRequest) (*mcp.GetPromptResult, error) { - logging.Debug("Proxying prompt request to remote server", - zap.String("prompt_name", req.Params.Name), + p.logger.Info("Proxying prompt request to remote server", + slog.String("prompt_name", req.Params.Name), ) if p.session == nil { @@ -397,15 +410,15 @@ func (p *ProxyClient) createProxyPromptHandler() mcp.PromptHandler { // Call remote prompt result, err := p.session.GetPrompt(ctx, req.Params) if err != nil { - logging.Error("Remote prompt request failed", - zap.String("prompt_name", req.Params.Name), - zap.Error(err), + p.logger.Error("Remote prompt request failed", + slog.String("prompt_name", req.Params.Name), + slog.Any("error", err), ) return nil, fmt.Errorf("remote prompt request failed: %w", err) } - logging.Debug("Remote prompt request successful", - zap.String("prompt_name", req.Params.Name), + p.logger.Info("Remote prompt request successful", + slog.String("prompt_name", req.Params.Name), ) return result, nil @@ -415,7 +428,7 @@ func (p *ProxyClient) createProxyPromptHandler() mcp.PromptHandler { // Close closes the connection to the remote MCP server func (p *ProxyClient) Close() error { if p != nil && p.session != nil { - logging.Debug("Closing connection to remote MCP server") + p.logger.Info("Closing connection to remote MCP server") return p.session.Close() } return nil diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 0b46bc4c..5b1f1194 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -5,17 +5,16 @@ import ( "encoding/json" "errors" "fmt" + "log/slog" "net/http" "slices" "time" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/analytics" "github.com/timescale/tiger-cli/internal/common" "github.com/timescale/tiger-cli/internal/config" - "github.com/timescale/tiger-cli/internal/logging" ) const ( @@ -44,6 +43,7 @@ const ( type Server struct { mcpServer *mcp.Server docsProxyClient *ProxyClient + logger *slog.Logger // app holds the config and API client. The analytics middleware reloads it // once per request, so config changes and logins made while the session is @@ -55,7 +55,7 @@ type Server struct { // addTool registers an MCP tool, skipping readOnlyGatedTools in read-only mode. func addTool[In, Out any](s *Server, readOnly bool, t *mcp.Tool, h mcp.ToolHandlerFor[In, Out]) { if readOnly && slices.Contains(readOnlyGatedTools, t.Name) { - logging.Debug("Skipping write tool in read-only mode", zap.String("tool", t.Name)) + s.logger.Info("Skipping write tool in read-only mode", slog.String("tool", t.Name)) return } mcp.AddTool(s.mcpServer, t, h) @@ -80,17 +80,23 @@ func buildServerInstructions(cfg *config.Config) string { // NewServer creates a new Tiger MCP server instance. The app must already be // loaded: its config renders the read-only warning in the server instructions // and gates which tools are registered, both evaluated once here at startup. -func NewServer(ctx context.Context, app *common.App) (*Server, error) { +// A nil logger discards the server's log output. +func NewServer(ctx context.Context, app *common.App, logger *slog.Logger) (*Server, error) { cfg := app.GetConfig() + logger = ensureLogger(logger) mcpServer := mcp.NewServer(&mcp.Implementation{ Name: ServerName, Title: serverTitle, Version: config.Version, - }, &mcp.ServerOptions{Instructions: buildServerInstructions(cfg)}) + }, &mcp.ServerOptions{ + Instructions: buildServerInstructions(cfg), + Logger: logger, + }) server := &Server{ mcpServer: mcpServer, + logger: logger, app: app, } @@ -106,6 +112,13 @@ func NewServer(ctx context.Context, app *common.App) (*Server, error) { return server, nil } +func ensureLogger(logger *slog.Logger) *slog.Logger { + if logger != nil { + return logger + } + return slog.New(slog.DiscardHandler) +} + // StartStdio starts the MCP server with the stdio transport func (s *Server) StartStdio(ctx context.Context) error { return s.mcpServer.Run(ctx, &mcp.StdioTransport{}) @@ -133,7 +146,7 @@ func (s *Server) registerTools(ctx context.Context, readOnly, experimental bool) // Register remote docs MCP server proxy s.registerDocsProxy(ctx) - logging.Info("MCP tools registered successfully") + s.logger.Info("MCP tools registered successfully") } // registerServiceTools registers service management tools with comprehensive schemas and descriptions @@ -187,7 +200,7 @@ func (s *Server) analyticsMiddleware(next mcp.MethodHandler) mcp.MethodHandler { var args map[string]any if len(r.Params.Arguments) > 0 { if err := json.Unmarshal(r.Params.Arguments, &args); err != nil { - logging.Error("Error unmarshaling tool call arguments", zap.Error(err)) + s.logger.Error("Error unmarshaling tool call arguments", slog.Any("error", err)) } } @@ -229,7 +242,7 @@ func (s *Server) analyticsMiddleware(next mcp.MethodHandler) mcp.MethodHandler { // Close gracefully shuts down the MCP server and all proxy connections func (s *Server) Close() error { - logging.Debug("Closing MCP server and proxy connections") + s.logger.Info("Closing MCP server and proxy connections") // Close docs proxy connection if err := s.docsProxyClient.Close(); err != nil { diff --git a/internal/mcp/server_test.go b/internal/mcp/server_test.go index a9ccdd8c..f81858ae 100644 --- a/internal/mcp/server_test.go +++ b/internal/mcp/server_test.go @@ -34,6 +34,7 @@ func registeredToolNames(t *testing.T, readOnly bool) []string { Title: serverTitle, Version: config.Version, }, nil), + logger: ensureLogger(nil), } s.registerServiceTools(readOnly, false) s.registerDatabaseTools(readOnly) diff --git a/internal/mcp/service_create.go b/internal/mcp/service_create.go index f38069ce..75f2bb3a 100644 --- a/internal/mcp/service_create.go +++ b/internal/mcp/service_create.go @@ -4,16 +4,15 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "net/http" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/api" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -123,14 +122,14 @@ func (s *Server) handleServiceCreate(ctx context.Context, req *mcp.CallToolReque cpuMillis, memoryGBs = &cpuMillisStr, &memoryGBsStr } - logging.Debug("MCP: Creating service", - zap.String("project_id", projectID), - zap.String("name", input.Name), - zap.Strings("addons", input.Addons), - zap.Stringp("region", input.Region), - zap.Stringp("cpu", cpuMillis), - zap.Stringp("memory", memoryGBs), - zap.Int("replicas", input.Replicas), + s.logger.Info("MCP: Creating service", + slog.String("project_id", projectID), + slog.String("name", input.Name), + slog.Any("addons", input.Addons), + slog.Any("region", input.Region), + slog.Any("cpu", cpuMillis), + slog.Any("memory", memoryGBs), + slog.Int("replicas", input.Replicas), ) // Prepare service creation request @@ -168,9 +167,9 @@ func (s *Server) handleServiceCreate(ctx context.Context, req *mcp.CallToolReque if input.SetDefault { if err := cfg.Set("service_id", serviceID); err != nil { // Log warning but don't fail the service creation - logging.Debug("MCP: Failed to set service as default", zap.Error(err)) + s.logger.Warn("MCP: Failed to set service as default", slog.Any("error", err)) } else { - logging.Debug("MCP: Set service as default", zap.String("service_id", serviceID)) + s.logger.Info("MCP: Set service as default", slog.String("service_id", serviceID)) } } @@ -181,9 +180,9 @@ func (s *Server) handleServiceCreate(ctx context.Context, req *mcp.CallToolReque result, err := common.SavePasswordWithResult(cfg, api.Service(service), *service.InitialPassword, "tsdbadmin") passwordStorage = &result if err != nil { - logging.Debug("MCP: Password storage failed", zap.Error(err)) + s.logger.Warn("MCP: Password storage failed", slog.Any("error", err)) } else { - logging.Debug("MCP: Password saved successfully", zap.String("method", result.Method)) + s.logger.Info("MCP: Password saved successfully", slog.String("method", result.Method)) } } diff --git a/internal/mcp/service_fork.go b/internal/mcp/service_fork.go index e9307689..e7417c30 100644 --- a/internal/mcp/service_fork.go +++ b/internal/mcp/service_fork.go @@ -4,16 +4,15 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "net/http" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/api" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -133,13 +132,13 @@ func (s *Server) handleServiceFork(ctx context.Context, req *mcp.CallToolRequest cpuMillis, memoryGBs = &cpuMillisStr, &memoryGBsStr } - logging.Debug("MCP: Forking service", - zap.String("project_id", projectID), - zap.String("service_id", input.ServiceID), - zap.String("name", input.Name), - zap.String("fork_strategy", string(input.ForkStrategy)), - zap.Stringp("cpu", cpuMillis), - zap.Stringp("memory", memoryGBs), + s.logger.Info("MCP: Forking service", + slog.String("project_id", projectID), + slog.String("service_id", input.ServiceID), + slog.String("name", input.Name), + slog.String("fork_strategy", string(input.ForkStrategy)), + slog.Any("cpu", cpuMillis), + slog.Any("memory", memoryGBs), ) // Prepare service fork request @@ -183,9 +182,9 @@ func (s *Server) handleServiceFork(ctx context.Context, req *mcp.CallToolRequest result, err := common.SavePasswordWithResult(cfg, api.Service(service), *service.InitialPassword, "tsdbadmin") passwordStorage = &result if err != nil { - logging.Debug("MCP: Password storage failed", zap.Error(err)) + s.logger.Warn("MCP: Password storage failed", slog.Any("error", err)) } else { - logging.Debug("MCP: Password saved successfully", zap.String("method", result.Method)) + s.logger.Info("MCP: Password saved successfully", slog.String("method", result.Method)) } } @@ -193,9 +192,9 @@ func (s *Server) handleServiceFork(ctx context.Context, req *mcp.CallToolRequest if input.SetDefault { if err := cfg.Set("service_id", serviceID); err != nil { // Log warning but don't fail the service fork - logging.Debug("MCP: Failed to set service as default", zap.Error(err)) + s.logger.Warn("MCP: Failed to set service as default", slog.Any("error", err)) } else { - logging.Debug("MCP: Set service as default", zap.String("service_id", serviceID)) + s.logger.Info("MCP: Set service as default", slog.String("service_id", serviceID)) } } diff --git a/internal/mcp/service_get.go b/internal/mcp/service_get.go index 91e18c25..f805bf62 100644 --- a/internal/mcp/service_get.go +++ b/internal/mcp/service_get.go @@ -3,15 +3,14 @@ package mcp import ( "context" "fmt" + "log/slog" "net/http" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -61,9 +60,9 @@ func (s *Server) handleServiceGet(ctx context.Context, req *mcp.CallToolRequest, return nil, ServiceGetOutput{}, err } - logging.Debug("MCP: Getting service details", - zap.String("project_id", projectID), - zap.String("service_id", input.ServiceID)) + s.logger.Info("MCP: Getting service details", + slog.String("project_id", projectID), + slog.String("service_id", input.ServiceID)) // Make API call to get service details ctx, cancel := context.WithTimeout(ctx, 30*time.Second) diff --git a/internal/mcp/service_list.go b/internal/mcp/service_list.go index 2ecd524e..386135b8 100644 --- a/internal/mcp/service_list.go +++ b/internal/mcp/service_list.go @@ -3,16 +3,15 @@ package mcp import ( "context" "fmt" + "log/slog" "net/http" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/api" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -72,7 +71,7 @@ func (s *Server) handleServiceList(ctx context.Context, req *mcp.CallToolRequest return nil, ServiceListOutput{}, err } - logging.Debug("MCP: Listing services", zap.String("project_id", projectID)) + s.logger.Info("MCP: Listing services", slog.String("project_id", projectID)) // Make API call to list services ctx, cancel := context.WithTimeout(ctx, 30*time.Second) diff --git a/internal/mcp/service_logs.go b/internal/mcp/service_logs.go index 27e10ba7..d42e15eb 100644 --- a/internal/mcp/service_logs.go +++ b/internal/mcp/service_logs.go @@ -3,14 +3,13 @@ package mcp import ( "context" "encoding/json" + "log/slog" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -81,13 +80,13 @@ func (s *Server) handleServiceLogs(ctx context.Context, req *mcp.CallToolRequest return nil, ServiceLogsOutput{}, err } - logging.Debug("MCP: Fetching service logs", - zap.String("project_id", projectID), - zap.String("service_id", input.ServiceID), - zap.Intp("node", input.Node), - zap.Int("tail", input.Tail), - zap.Timep("since", input.Since), - zap.Timep("until", input.Until), + s.logger.Info("MCP: Fetching service logs", + slog.String("project_id", projectID), + slog.String("service_id", input.ServiceID), + slog.Any("node", input.Node), + slog.Int("tail", input.Tail), + slog.Any("since", input.Since), + slog.Any("until", input.Until), ) // Fetch logs with pagination support diff --git a/internal/mcp/service_metrics_available.go b/internal/mcp/service_metrics_available.go index 7aabb46f..419fae4b 100644 --- a/internal/mcp/service_metrics_available.go +++ b/internal/mcp/service_metrics_available.go @@ -3,15 +3,14 @@ package mcp import ( "context" "fmt" + "log/slog" "net/http" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -58,9 +57,9 @@ func (s *Server) handleServiceMetricsAvailable(ctx context.Context, req *mcp.Cal return nil, ServiceMetricsAvailableOutput{}, err } - logging.Debug("MCP: Listing available metric series", - zap.String("project_id", projectID), - zap.String("service_id", input.ServiceID), + s.logger.Info("MCP: Listing available metric series", + slog.String("project_id", projectID), + slog.String("service_id", input.ServiceID), ) ctx, cancel := context.WithTimeout(ctx, 30*time.Second) diff --git a/internal/mcp/service_metrics_series.go b/internal/mcp/service_metrics_series.go index bd8e42e4..39aea206 100644 --- a/internal/mcp/service_metrics_series.go +++ b/internal/mcp/service_metrics_series.go @@ -3,17 +3,16 @@ package mcp import ( "context" "fmt" + "log/slog" "net/http" "strings" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/api" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -110,12 +109,12 @@ func (s *Server) handleServiceMetricsSeries(ctx context.Context, req *mcp.CallTo return nil, nil, err } - logging.Debug("MCP: Fetching metric series", - zap.String("project_id", projectID), - zap.String("service_id", input.ServiceID), - zap.String("metric", input.MetricName), - zap.String("from", input.From), - zap.String("to", input.To), + s.logger.Info("MCP: Fetching metric series", + slog.String("project_id", projectID), + slog.String("service_id", input.ServiceID), + slog.String("metric", input.MetricName), + slog.String("from", input.From), + slog.String("to", input.To), ) fromTime, err := time.Parse(time.RFC3339, input.From) diff --git a/internal/mcp/service_resize.go b/internal/mcp/service_resize.go index d3948c1a..d34af47e 100644 --- a/internal/mcp/service_resize.go +++ b/internal/mcp/service_resize.go @@ -4,16 +4,15 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "net/http" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/api" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -82,10 +81,10 @@ func (s *Server) handleServiceResize(ctx context.Context, req *mcp.CallToolReque return nil, ServiceResizeOutput{}, err } - logging.Debug("MCP: Resizing service", - zap.String("project_id", projectID), - zap.String("service_id", input.ServiceID), - zap.String("cpu_memory", input.CPUMemory), + s.logger.Info("MCP: Resizing service", + slog.String("project_id", projectID), + slog.String("service_id", input.ServiceID), + slog.String("cpu_memory", input.CPUMemory), ) // Parse CPU/Memory combination diff --git a/internal/mcp/service_start.go b/internal/mcp/service_start.go index 1825e7b6..878c67c6 100644 --- a/internal/mcp/service_start.go +++ b/internal/mcp/service_start.go @@ -4,15 +4,14 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "net/http" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -74,9 +73,9 @@ func (s *Server) handleServiceStart(ctx context.Context, req *mcp.CallToolReques return nil, ServiceStartOutput{}, err } - logging.Debug("MCP: Starting service", - zap.String("project_id", projectID), - zap.String("service_id", input.ServiceID)) + s.logger.Info("MCP: Starting service", + slog.String("project_id", projectID), + slog.String("service_id", input.ServiceID)) // Make API call to start service startCtx, cancel := context.WithTimeout(ctx, 30*time.Second) diff --git a/internal/mcp/service_stop.go b/internal/mcp/service_stop.go index 2fbee318..35adc02e 100644 --- a/internal/mcp/service_stop.go +++ b/internal/mcp/service_stop.go @@ -4,15 +4,14 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "net/http" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -74,9 +73,9 @@ func (s *Server) handleServiceStop(ctx context.Context, req *mcp.CallToolRequest return nil, ServiceStopOutput{}, err } - logging.Debug("MCP: Stopping service", - zap.String("project_id", projectID), - zap.String("service_id", input.ServiceID)) + s.logger.Info("MCP: Stopping service", + slog.String("project_id", projectID), + slog.String("service_id", input.ServiceID)) // Make API call to stop service stopCtx, cancel := context.WithTimeout(ctx, 30*time.Second) diff --git a/internal/mcp/service_update_password.go b/internal/mcp/service_update_password.go index 477d4289..770de5f2 100644 --- a/internal/mcp/service_update_password.go +++ b/internal/mcp/service_update_password.go @@ -3,16 +3,15 @@ package mcp import ( "context" "fmt" + "log/slog" "net/http" "time" "github.com/google/jsonschema-go/jsonschema" "github.com/modelcontextprotocol/go-sdk/mcp" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/api" "github.com/timescale/tiger-cli/internal/common" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -72,9 +71,9 @@ func (s *Server) handleServiceUpdatePassword(ctx context.Context, req *mcp.CallT return nil, ServiceUpdatePasswordOutput{}, err } - logging.Debug("MCP: Updating service password", - zap.String("project_id", projectID), - zap.String("service_id", input.ServiceID)) + s.logger.Info("MCP: Updating service password", + slog.String("project_id", projectID), + slog.String("service_id", input.ServiceID)) // Prepare password update request updateReq := api.UpdatePasswordInput{ @@ -114,9 +113,9 @@ func (s *Server) handleServiceUpdatePassword(ctx context.Context, req *mcp.CallT result, saveErr := common.SavePasswordWithResult(cfg, service, input.Password, "tsdbadmin") passwordStorage := &result if saveErr != nil { - logging.Debug("MCP: Password storage failed", zap.Error(saveErr)) + s.logger.Warn("MCP: Password storage failed", slog.Any("error", saveErr)) } else { - logging.Debug("MCP: Password saved successfully", zap.String("method", result.Method)) + s.logger.Info("MCP: Password saved successfully", slog.String("method", result.Method)) } output := ServiceUpdatePasswordOutput{ diff --git a/internal/mcp/utils.go b/internal/mcp/utils.go index 0115fc26..03ef08c8 100644 --- a/internal/mcp/utils.go +++ b/internal/mcp/utils.go @@ -3,15 +3,14 @@ package mcp import ( "encoding/json" "fmt" + "log/slog" "time" "github.com/google/jsonschema-go/jsonschema" - "go.uber.org/zap" "github.com/timescale/tiger-cli/internal/api" "github.com/timescale/tiger-cli/internal/common" "github.com/timescale/tiger-cli/internal/config" - "github.com/timescale/tiger-cli/internal/logging" "github.com/timescale/tiger-cli/internal/util" ) @@ -155,10 +154,10 @@ func (s *Server) convertToServiceDetail(cfg *config.Config, service api.Service, WithPassword: withPassword, InitialPassword: util.Deref(service.InitialPassword), }); err != nil { - logging.Error("MCP: Failed to build connection string", zap.Error(err)) + s.logger.Error("MCP: Failed to build connection string", slog.Any("error", err)) } else { if withPassword && details.Password == "" { - logging.Error("MCP: Requested password but password not available") + s.logger.Error("MCP: Requested password but password not available") } detail.ConnectionString = details.String() detail.Password = details.Password diff --git a/internal/version/check.go b/internal/version/check.go index 375b3ab4..34ba12a9 100644 --- a/internal/version/check.go +++ b/internal/version/check.go @@ -224,9 +224,6 @@ func PrintUpdateWarning(result *CheckResult, cfg *config.Config, output *io.Writ return } if !result.UpdateAvailable { - if cfg.Debug { - fmt.Fprintf(*output, "No update available\n") - } return } From e7d9efa276ebbfee98b67882680c3f9bd39ca3ae Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Wed, 5 Aug 2026 19:11:01 -0400 Subject: [PATCH 2/5] Simplify MCP logging: drop startup/proxy tracing, propagate server errors Removes the per-step registration and proxy tracing so `tiger mcp start` is silent until something goes wrong, applies the "Method not found" guard to all four docs-proxy registrations, and makes `mcp start http` surface a Serve error instead of logging it and blocking on a dead listener. --- CLAUDE.md | 5 ++ internal/cmd/mcp_start.go | 3 - internal/cmd/mcp_start_http.go | 36 ++++++--- internal/cmd/root.go | 4 +- internal/mcp/proxy.go | 131 +++------------------------------ internal/mcp/server.go | 5 -- 6 files changed, 44 insertions(+), 140 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 91bc6fce..36877f8a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -484,6 +484,11 @@ anything below `Info` unless `slog.SetLogLoggerLevel` is called, so log at `Info or above; a `Debug` call would silently go nowhere. Attach errors with `slog.Any("error", err)`, not `slog.String("error", err.Error())`. +Because every statement is visible by default, keep them sparse: log failures +that would otherwise be swallowed (the docs-proxy registration errors are the +model), not per-step tracing of work that succeeded. Startup registration emits +nothing — `tiger mcp start` should be silent until something goes wrong. + Everything outside `internal/mcp` writes to stdout/stderr directly rather than logging. Don't add log statements to CLI commands — print to `cmd.OutOrStdout()`/`cmd.ErrOrStderr()`, or return an error. diff --git a/internal/cmd/mcp_start.go b/internal/cmd/mcp_start.go index b93c70cc..22f69b1f 100644 --- a/internal/cmd/mcp_start.go +++ b/internal/cmd/mcp_start.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "log/slog" "github.com/spf13/cobra" @@ -52,8 +51,6 @@ func startStdioServer(cmd *cobra.Command, app *common.App) error { ctx := cmd.Context() logger := newLogger(cmd.ErrOrStderr()) - logger.Info("Starting Tiger MCP server", slog.String("transport", "stdio")) - // Create MCP server server, err := mcp.NewServer(ctx, app, logger) if err != nil { diff --git a/internal/cmd/mcp_start_http.go b/internal/cmd/mcp_start_http.go index faee92ab..0adc8913 100644 --- a/internal/cmd/mcp_start_http.go +++ b/internal/cmd/mcp_start_http.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "errors" "fmt" "log/slog" "net" @@ -39,6 +40,7 @@ Examples: tiger mcp start http --host 192.168.1.100 --port 9000`, Args: cobra.NoArgs, ValidArgsFunction: cobra.NoFileCompletions, + SilenceErrors: true, // HTTP server uses slog for all output, including errors RunE: func(cmd *cobra.Command, args []string) error { cmd.SilenceUsage = true return startHTTPServer(cmd, app, httpHost, httpPort) @@ -57,11 +59,10 @@ func startHTTPServer(cmd *cobra.Command, app *common.App, host string, port int) ctx := cmd.Context() logger := newLogger(cmd.ErrOrStderr()) - logger.Info("Starting Tiger MCP server", slog.String("transport", "http")) - // Create MCP server server, err := mcp.NewServer(ctx, app, logger) if err != nil { + logger.Error("failed to create MCP server", slog.Any("error", err)) return fmt.Errorf("failed to create MCP server: %w", err) } defer server.Close() @@ -69,6 +70,11 @@ func startHTTPServer(cmd *cobra.Command, app *common.App, host string, port int) // Find available port and get the listener listener, actualPort, err := getListener(host, port) if err != nil { + logger.Error("failed to get listener", + slog.String("host", host), + slog.Int("port", port), + slog.Any("error", err), + ) return fmt.Errorf("failed to get listener: %w", err) } defer listener.Close() @@ -91,28 +97,36 @@ func startHTTPServer(cmd *cobra.Command, app *common.App, host string, port int) logger.Info("Use Ctrl+C to stop the server") // Start server in goroutine using the existing listener + errCh := make(chan error, 1) go func() { - if err := httpServer.Serve(listener); err != nil && err != http.ErrServerClosed { - logger.Error("HTTP server error", slog.Any("error", err)) + if err := httpServer.Serve(listener); err != nil && !errors.Is(err, http.ErrServerClosed) { + errCh <- err } }() - // Wait for context cancellation. Once canceled, stop handling signals and - // revert to default signal handling behavior. This allows a second - // SIGINT/SIGTERM to forcibly kill the server (useful if there's currently - // an active MCP session but you want to kill it anyways). Note that stop() - // is idempotent and safe to call multiple times, so it's okay that it's - // called here and via the deferred call above. - <-ctx.Done() + // Wait for a server error or context cancellation. Once canceled, stop + // handling signals and revert to default signal handling behavior. This + // allows a second SIGINT/SIGTERM to forcibly kill the server (useful if + // there's currently an active MCP session but you want to kill it anyways). + // Note that stop() is idempotent and safe to call multiple times, so it's + // okay that it's called here and via the deferred call above. + select { + case err := <-errCh: + logger.Error("HTTP server error", slog.Any("error", err)) + return fmt.Errorf("HTTP server error: %w", err) + case <-ctx.Done(): + } // Shutdown server gracefully logger.Info("Gracefully shutting down HTTP server, press control-C twice to immediately shutdown") if err := httpServer.Shutdown(context.Background()); err != nil { + logger.Error("failed to shut down HTTP server", slog.Any("error", err)) return fmt.Errorf("failed to shut down HTTP server: %w", err) } // Close the MCP server when finished if err := server.Close(); err != nil { + logger.Error("failed to close MCP server", slog.Any("error", err)) return fmt.Errorf("failed to close MCP server: %w", err) } return nil diff --git a/internal/cmd/root.go b/internal/cmd/root.go index 6b3214c6..acdfbab3 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -158,12 +158,12 @@ func versionCheck(cmd *cobra.Command, cfg *config.Config, skipUpdateCheck bool) }() return func() { - res := <-resultCh + res, ok := <-resultCh // Re-check cfg.VersionCheck: the command may have turned checks off in // place (e.g. `tiger config set version_check false`, which reloads the // config struct rather than replacing it). - if !cfg.VersionCheck { + if !ok || !cfg.VersionCheck { return } diff --git a/internal/mcp/proxy.go b/internal/mcp/proxy.go index 486b6c33..f63f140f 100644 --- a/internal/mcp/proxy.go +++ b/internal/mcp/proxy.go @@ -54,14 +54,9 @@ func (s *Server) registerDocsProxy(ctx context.Context) { cfg := s.app.GetConfig() if !cfg.DocsMCP || cfg.DocsMCPURL == "" { - s.logger.Info("Docs MCP proxy is disabled") return } - s.logger.Info("Setting up docs MCP proxy connection", - slog.String("url", cfg.DocsMCPURL), - ) - // Create timeout for establishing proxy ctx, cancel := context.WithTimeout(ctx, time.Minute) defer cancel() @@ -76,43 +71,32 @@ func (s *Server) registerDocsProxy(ctx context.Context) { } s.docsProxyClient = proxyClient - if err := proxyClient.RegisterTools(ctx, s.mcpServer); err != nil { + // A "Method not found" error is expected from remote servers that don't + // expose the corresponding capability at all. + if err := proxyClient.RegisterTools(ctx, s.mcpServer); err != nil && !isMethodNotFoundError(err) { s.logger.Error("Failed to register tools from docs MCP server", slog.Any("error", err), ) } - if err := proxyClient.RegisterResources(ctx, s.mcpServer); err != nil { - // Check if this is a "Method not found" error as those are expected in servers that don't have any resources - if isMethodNotFoundError(err) { - s.logger.Info("Resources not supported by remote MCP server") - } else { - s.logger.Error("Failed to register resources from docs MCP server", - slog.Any("error", err), - ) - } + if err := proxyClient.RegisterResources(ctx, s.mcpServer); err != nil && !isMethodNotFoundError(err) { + s.logger.Error("Failed to register resources from docs MCP server", + slog.Any("error", err), + ) } - if err := proxyClient.RegisterResourceTemplates(ctx, s.mcpServer); err != nil { - // Check if this is a "Method not found" error as those are expected in servers that don't have any resource templates - if isMethodNotFoundError(err) { - s.logger.Info("Resource templates not supported by remote MCP server") - } else { - s.logger.Error("Failed to register resource templates from docs MCP server", - slog.Any("error", err), - ) - } + if err := proxyClient.RegisterResourceTemplates(ctx, s.mcpServer); err != nil && !isMethodNotFoundError(err) { + s.logger.Error("Failed to register resource templates from docs MCP server", + slog.Any("error", err), + ) } - if err := proxyClient.RegisterPrompts(ctx, s.mcpServer); err != nil { + if err := proxyClient.RegisterPrompts(ctx, s.mcpServer); err != nil && !isMethodNotFoundError(err) { s.logger.Error("Failed to register prompts from docs MCP server", slog.Any("error", err), ) } - s.logger.Info("Successfully connected to docs MCP server", - slog.String("url", cfg.DocsMCPURL), - ) } // ProxyClient manages connection to a remote MCP server and forwards requests @@ -120,17 +104,10 @@ type ProxyClient struct { url string client *mcp.Client session *mcp.ClientSession - logger *slog.Logger } // NewProxyClient creates a new proxy client for the given remote server configuration func NewProxyClient(ctx context.Context, url string, logger *slog.Logger) (*ProxyClient, error) { - logger = ensureLogger(logger) - - logger.Info("Connecting to docs MCP server", - slog.String("url", url), - ) - transport := &mcp.StreamableClientTransport{ Endpoint: url, } @@ -148,13 +125,10 @@ func NewProxyClient(ctx context.Context, url string, logger *slog.Logger) (*Prox return nil, fmt.Errorf("failed to connect to remote MCP server: %w", err) } - logger.Info("Successfully connected to docs MCP server") - return &ProxyClient{ url: url, client: client, session: session, - logger: logger, }, nil } @@ -164,8 +138,6 @@ func (p *ProxyClient) RegisterTools(ctx context.Context, server *mcp.Server) err return fmt.Errorf("not connected to remote server") } - p.logger.Info("Discovering tools from remote MCP server") - // List tools from remote server toolsResp, err := p.session.ListTools(ctx, &mcp.ListToolsParams{}) if err != nil { @@ -173,14 +145,12 @@ func (p *ProxyClient) RegisterTools(ctx context.Context, server *mcp.Server) err } if toolsResp == nil || len(toolsResp.Tools) == 0 { - p.logger.Info("No tools found on remote server") return nil } // Register each remote tool as a proxy tool for _, tool := range toolsResp.Tools { if tool.Name == "" { - p.logger.Warn("Skipping tool with empty name") continue } @@ -189,25 +159,14 @@ func (p *ProxyClient) RegisterTools(ctx context.Context, server *mcp.Server) err // Register the proxy tool with our MCP server server.AddTool(tool, handler) - - p.logger.Info("Registered proxy tool", - slog.String("name", tool.Name), - ) } - p.logger.Info("Successfully registered proxy tools", - slog.Int("count", len(toolsResp.Tools)), - ) return nil } // createProxyToolHandler creates a handler function that forwards tool calls to the remote server func (p *ProxyClient) createProxyToolHandler() mcp.ToolHandler { return func(ctx context.Context, req *mcp.CallToolRequest) (*mcp.CallToolResult, error) { - p.logger.Info("Proxying tool call to remote server", - slog.String("tool_name", req.Params.Name), - ) - if p.session == nil { return nil, fmt.Errorf("not connected to remote MCP server") } @@ -222,17 +181,9 @@ func (p *ProxyClient) createProxyToolHandler() mcp.ToolHandler { // Call remote tool result, err := p.session.CallTool(ctx, params) if err != nil { - p.logger.Error("Remote tool call failed", - slog.String("tool_name", req.Params.Name), - slog.Any("error", err), - ) return nil, fmt.Errorf("remote tool call failed: %w", err) } - p.logger.Info("Remote tool call successful", - slog.String("tool_name", req.Params.Name), - ) - return result, nil } } @@ -244,8 +195,6 @@ func (p *ProxyClient) RegisterResources(ctx context.Context, server *mcp.Server) return fmt.Errorf("not connected to remote server") } - p.logger.Info("Discovering resources from remote MCP server") - // List resources from remote server resourcesResp, err := p.session.ListResources(ctx, &mcp.ListResourcesParams{}) if err != nil { @@ -253,14 +202,12 @@ func (p *ProxyClient) RegisterResources(ctx context.Context, server *mcp.Server) } if resourcesResp == nil || len(resourcesResp.Resources) == 0 { - p.logger.Info("No resources found on remote server") return nil } // Register each remote resource as a proxy resource for _, resource := range resourcesResp.Resources { if resource.URI == "" { - p.logger.Warn("Skipping resource with empty URI") continue } @@ -269,15 +216,8 @@ func (p *ProxyClient) RegisterResources(ctx context.Context, server *mcp.Server) // Register the proxy resource with our MCP server server.AddResource(resource, handler) - - p.logger.Info("Registered proxy resource", - slog.String("uri", resource.URI), - ) } - p.logger.Info("Successfully registered proxy resources", - slog.Int("count", len(resourcesResp.Resources)), - ) return nil } @@ -287,8 +227,6 @@ func (p *ProxyClient) RegisterResourceTemplates(ctx context.Context, server *mcp return fmt.Errorf("not connected to remote server") } - p.logger.Info("Discovering resource templates from remote MCP server") - // List resource templates from remote server templatesResp, err := p.session.ListResourceTemplates(ctx, &mcp.ListResourceTemplatesParams{}) if err != nil { @@ -296,14 +234,12 @@ func (p *ProxyClient) RegisterResourceTemplates(ctx context.Context, server *mcp } if templatesResp == nil || len(templatesResp.ResourceTemplates) == 0 { - p.logger.Info("No resource templates found on remote server") return nil } // Register each remote resource template as a proxy resource template for _, resourceTemplate := range templatesResp.ResourceTemplates { if resourceTemplate.URITemplate == "" { - p.logger.Warn("Skipping resource template with empty URI template") continue } @@ -312,25 +248,14 @@ func (p *ProxyClient) RegisterResourceTemplates(ctx context.Context, server *mcp // Register the proxy resource template with our MCP server server.AddResourceTemplate(resourceTemplate, handler) - - p.logger.Info("Registered proxy resource template", - slog.String("uri_template", resourceTemplate.URITemplate), - ) } - p.logger.Info("Successfully registered proxy resource templates", - slog.Int("count", len(templatesResp.ResourceTemplates)), - ) return nil } // createProxyResourceHandler creates a handler function that forwards resource reads to the remote server func (p *ProxyClient) createProxyResourceHandler() mcp.ResourceHandler { return func(ctx context.Context, req *mcp.ReadResourceRequest) (*mcp.ReadResourceResult, error) { - p.logger.Info("Proxying resource read to remote server", - slog.String("resource_uri", req.Params.URI), - ) - if p.session == nil { return nil, fmt.Errorf("not connected to remote MCP server") } @@ -338,17 +263,9 @@ func (p *ProxyClient) createProxyResourceHandler() mcp.ResourceHandler { // Call remote resource result, err := p.session.ReadResource(ctx, req.Params) if err != nil { - p.logger.Error("Remote resource read failed", - slog.String("resource_uri", req.Params.URI), - slog.Any("error", err), - ) return nil, fmt.Errorf("remote resource read failed: %w", err) } - p.logger.Info("Remote resource read successful", - slog.String("resource_uri", req.Params.URI), - ) - return result, nil } } @@ -359,8 +276,6 @@ func (p *ProxyClient) RegisterPrompts(ctx context.Context, server *mcp.Server) e return fmt.Errorf("not connected to remote server") } - p.logger.Info("Discovering prompts from remote MCP server") - // List prompts from remote server promptsResp, err := p.session.ListPrompts(ctx, &mcp.ListPromptsParams{}) if err != nil { @@ -368,14 +283,12 @@ func (p *ProxyClient) RegisterPrompts(ctx context.Context, server *mcp.Server) e } if promptsResp == nil || len(promptsResp.Prompts) == 0 { - p.logger.Info("No prompts found on remote server") return nil } // Register each remote prompt as a proxy prompt for _, prompt := range promptsResp.Prompts { if prompt.Name == "" { - p.logger.Warn("Skipping prompt with empty name") continue } @@ -384,25 +297,14 @@ func (p *ProxyClient) RegisterPrompts(ctx context.Context, server *mcp.Server) e // Register the proxy prompt with our MCP server server.AddPrompt(prompt, handler) - - p.logger.Info("Registered proxy prompt", - slog.String("original_name", prompt.Name), - ) } - p.logger.Info("Successfully registered proxy prompts", - slog.Int("count", len(promptsResp.Prompts)), - ) return nil } // createProxyPromptHandler creates a handler function that forwards prompt requests to the remote server func (p *ProxyClient) createProxyPromptHandler() mcp.PromptHandler { return func(ctx context.Context, req *mcp.GetPromptRequest) (*mcp.GetPromptResult, error) { - p.logger.Info("Proxying prompt request to remote server", - slog.String("prompt_name", req.Params.Name), - ) - if p.session == nil { return nil, fmt.Errorf("not connected to remote MCP server") } @@ -410,17 +312,9 @@ func (p *ProxyClient) createProxyPromptHandler() mcp.PromptHandler { // Call remote prompt result, err := p.session.GetPrompt(ctx, req.Params) if err != nil { - p.logger.Error("Remote prompt request failed", - slog.String("prompt_name", req.Params.Name), - slog.Any("error", err), - ) return nil, fmt.Errorf("remote prompt request failed: %w", err) } - p.logger.Info("Remote prompt request successful", - slog.String("prompt_name", req.Params.Name), - ) - return result, nil } } @@ -428,7 +322,6 @@ func (p *ProxyClient) createProxyPromptHandler() mcp.PromptHandler { // Close closes the connection to the remote MCP server func (p *ProxyClient) Close() error { if p != nil && p.session != nil { - p.logger.Info("Closing connection to remote MCP server") return p.session.Close() } return nil diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 5b1f1194..ba6660ae 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -55,7 +55,6 @@ type Server struct { // addTool registers an MCP tool, skipping readOnlyGatedTools in read-only mode. func addTool[In, Out any](s *Server, readOnly bool, t *mcp.Tool, h mcp.ToolHandlerFor[In, Out]) { if readOnly && slices.Contains(readOnlyGatedTools, t.Name) { - s.logger.Info("Skipping write tool in read-only mode", slog.String("tool", t.Name)) return } mcp.AddTool(s.mcpServer, t, h) @@ -145,8 +144,6 @@ func (s *Server) registerTools(ctx context.Context, readOnly, experimental bool) // Register remote docs MCP server proxy s.registerDocsProxy(ctx) - - s.logger.Info("MCP tools registered successfully") } // registerServiceTools registers service management tools with comprehensive schemas and descriptions @@ -242,8 +239,6 @@ func (s *Server) analyticsMiddleware(next mcp.MethodHandler) mcp.MethodHandler { // Close gracefully shuts down the MCP server and all proxy connections func (s *Server) Close() error { - s.logger.Info("Closing MCP server and proxy connections") - // Close docs proxy connection if err := s.docsProxyClient.Close(); err != nil { return fmt.Errorf("failed to close docs proxy client: %w", err) From e1a1ef0965546a487bc89ad43e93964e871e8d0d Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 6 Aug 2026 14:44:12 -0400 Subject: [PATCH 3/5] Fix logging/error message capitalization --- internal/cmd/mcp_get.go | 8 ++++---- internal/cmd/mcp_install.go | 8 ++++---- internal/cmd/mcp_start_http.go | 8 ++++---- internal/mcp/proxy.go | 1 - 4 files changed, 12 insertions(+), 13 deletions(-) diff --git a/internal/cmd/mcp_get.go b/internal/cmd/mcp_get.go index 88d1bc17..3ea35ca9 100644 --- a/internal/cmd/mcp_get.go +++ b/internal/cmd/mcp_get.go @@ -150,12 +150,12 @@ func outputToolText(output io.Writer, tool *mcpsdk.Tool) error { if tool.InputSchema != nil { raw, err := json.Marshal(tool.InputSchema) if err != nil { - return fmt.Errorf("Error marshaling input schema to JSON: %w", err) + return fmt.Errorf("error marshaling input schema to JSON: %w", err) } var inputSchema *jsonschema.Schema if err := json.Unmarshal(raw, &inputSchema); err != nil { - return fmt.Errorf("Error unmarshaling input schema from JSON: %w", err) + return fmt.Errorf("error unmarshaling input schema from JSON: %w", err) } formatted := formatJSONSchema(inputSchema, 1) @@ -170,12 +170,12 @@ func outputToolText(output io.Writer, tool *mcpsdk.Tool) error { if tool.OutputSchema != nil { raw, err := json.Marshal(tool.OutputSchema) if err != nil { - return fmt.Errorf("Error marshaling output schema to JSON: %w", err) + return fmt.Errorf("error marshaling output schema to JSON: %w", err) } var outputSchema *jsonschema.Schema if err := json.Unmarshal(raw, &outputSchema); err != nil { - return fmt.Errorf("Error unmarshaling output schema from JSON: %w", err) + return fmt.Errorf("error unmarshaling output schema from JSON: %w", err) } formatted := formatJSONSchema(outputSchema, 1) diff --git a/internal/cmd/mcp_install.go b/internal/cmd/mcp_install.go index f6d02d9c..fcf3656b 100644 --- a/internal/cmd/mcp_install.go +++ b/internal/cmd/mcp_install.go @@ -283,16 +283,16 @@ func SupportedClients() []ClientInfo { func InstallMCPForClient(opts InstallOptions) error { // Validate required options if opts.ClientName == "" { - return fmt.Errorf("ClientName is required") + return fmt.Errorf("missing required option: ClientName") } if opts.ServerName == "" { - return fmt.Errorf("ServerName is required") + return fmt.Errorf("missing required option: ServerName") } if opts.Command == "" { - return fmt.Errorf("Command is required") + return fmt.Errorf("missing required option: Command") } if opts.Args == nil { - return fmt.Errorf("Args is required") + return fmt.Errorf("missing required option: Args") } // Find the client configuration by name diff --git a/internal/cmd/mcp_start_http.go b/internal/cmd/mcp_start_http.go index 0adc8913..7ea54dc1 100644 --- a/internal/cmd/mcp_start_http.go +++ b/internal/cmd/mcp_start_http.go @@ -62,7 +62,7 @@ func startHTTPServer(cmd *cobra.Command, app *common.App, host string, port int) // Create MCP server server, err := mcp.NewServer(ctx, app, logger) if err != nil { - logger.Error("failed to create MCP server", slog.Any("error", err)) + logger.Error("Failed to create MCP server", slog.Any("error", err)) return fmt.Errorf("failed to create MCP server: %w", err) } defer server.Close() @@ -70,7 +70,7 @@ func startHTTPServer(cmd *cobra.Command, app *common.App, host string, port int) // Find available port and get the listener listener, actualPort, err := getListener(host, port) if err != nil { - logger.Error("failed to get listener", + logger.Error("Failed to get listener", slog.String("host", host), slog.Int("port", port), slog.Any("error", err), @@ -120,13 +120,13 @@ func startHTTPServer(cmd *cobra.Command, app *common.App, host string, port int) // Shutdown server gracefully logger.Info("Gracefully shutting down HTTP server, press control-C twice to immediately shutdown") if err := httpServer.Shutdown(context.Background()); err != nil { - logger.Error("failed to shut down HTTP server", slog.Any("error", err)) + logger.Error("Failed to shut down HTTP server", slog.Any("error", err)) return fmt.Errorf("failed to shut down HTTP server: %w", err) } // Close the MCP server when finished if err := server.Close(); err != nil { - logger.Error("failed to close MCP server", slog.Any("error", err)) + logger.Error("Failed to close MCP server", slog.Any("error", err)) return fmt.Errorf("failed to close MCP server: %w", err) } return nil diff --git a/internal/mcp/proxy.go b/internal/mcp/proxy.go index f63f140f..cb59f742 100644 --- a/internal/mcp/proxy.go +++ b/internal/mcp/proxy.go @@ -96,7 +96,6 @@ func (s *Server) registerDocsProxy(ctx context.Context) { slog.Any("error", err), ) } - } // ProxyClient manages connection to a remote MCP server and forwards requests From 86fbafd60c389a428aea21d343d45b1ea3f81705 Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 6 Aug 2026 14:54:37 -0400 Subject: [PATCH 4/5] Restore a couple log lines --- CLAUDE.md | 6 ++++-- internal/mcp/proxy.go | 1 + internal/mcp/server.go | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 36877f8a..9f3c5269 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -486,8 +486,10 @@ or above; a `Debug` call would silently go nowhere. Attach errors with Because every statement is visible by default, keep them sparse: log failures that would otherwise be swallowed (the docs-proxy registration errors are the -model), not per-step tracing of work that succeeded. Startup registration emits -nothing — `tiger mcp start` should be silent until something goes wrong. +model), not per-step tracing of work that succeeded. A default `tiger mcp start` +is silent; the startup lines that do exist report configuration that removes +capabilities — the docs proxy being disabled, and each write tool skipped in +read-only mode — so a client can see why a tool it expected is missing. Everything outside `internal/mcp` writes to stdout/stderr directly rather than logging. Don't add log statements to CLI commands — print to diff --git a/internal/mcp/proxy.go b/internal/mcp/proxy.go index cb59f742..2558ae35 100644 --- a/internal/mcp/proxy.go +++ b/internal/mcp/proxy.go @@ -54,6 +54,7 @@ func (s *Server) registerDocsProxy(ctx context.Context) { cfg := s.app.GetConfig() if !cfg.DocsMCP || cfg.DocsMCPURL == "" { + s.logger.Info("Docs MCP proxy is disabled") return } diff --git a/internal/mcp/server.go b/internal/mcp/server.go index ba6660ae..6fb45d84 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -55,6 +55,7 @@ type Server struct { // addTool registers an MCP tool, skipping readOnlyGatedTools in read-only mode. func addTool[In, Out any](s *Server, readOnly bool, t *mcp.Tool, h mcp.ToolHandlerFor[In, Out]) { if readOnly && slices.Contains(readOnlyGatedTools, t.Name) { + s.logger.Info("Skipping write tool in read-only mode", slog.String("tool", t.Name)) return } mcp.AddTool(s.mcpServer, t, h) From 52279e13f1ae0d1472bbfce90a1dd01179fe345f Mon Sep 17 00:00:00 2001 From: Nathan Cochran Date: Thu, 6 Aug 2026 15:50:48 -0400 Subject: [PATCH 5/5] Update CLAUDE.md --- CLAUDE.md | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9f3c5269..874d325c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -132,6 +132,17 @@ go generate ./internal/api - Run `go vet ./...` to catch potential issues before committing - Run `go test ./...` to ensure all tests pass +### Error Messages + +Error strings start with a **lowercase** letter, so they read correctly when a +caller wraps them: `fmt.Errorf("failed to get service: %w", err)`. Log messages +are the opposite — they start with a capital letter (see "Logging Architecture"). + +The exception is a leading proper noun or initialism, which keeps its +capitalization (`fmt.Errorf("API key validation failed: %w", err)`). If that +reads awkwardly, reword so the identifier isn't first — `missing required option: +ClientName` rather than `ClientName is required`. + ### Configuration Management **IMPORTANT:** Follow these rules when working with configuration: @@ -482,7 +493,9 @@ discards the output via `slog.New(slog.DiscardHandler)`. There is no level configuration and no `--debug` flag. `slog.Default()` drops anything below `Info` unless `slog.SetLogLoggerLevel` is called, so log at `Info` or above; a `Debug` call would silently go nowhere. Attach errors with -`slog.Any("error", err)`, not `slog.String("error", err.Error())`. +`slog.Any("error", err)`, not `slog.String("error", err.Error())`. Log messages +start with a capital letter — error strings do the opposite (see "Error +Messages"). Because every statement is visible by default, keep them sparse: log failures that would otherwise be swallowed (the docs-proxy registration errors are the @@ -569,6 +582,11 @@ RunE: func(cmd *cobra.Command, args []string) error { This provides fine-grained control over when usage is displayed, improving user experience by showing help when it's relevant and hiding it when it's not. +`SilenceErrors` is a separate setting and is rarely needed: set it only on a +command that already reports its own errors, so cobra doesn't print them a second +time. `tiger mcp start http` sets it because the MCP server logs failures through +slog before returning them. + ## Command Architecture: Pure Functional Builder Pattern Tiger CLI uses a pure functional builder pattern with **zero global command state**. This architecture ensures perfect test isolation, eliminates shared state issues, and provides a clean, maintainable command structure.