From ee0ddcf71599c03cb5ae5b400be50518abbd73db Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 8 Oct 2026 12:49:02 +0530 Subject: [PATCH 1/6] feat(metrics,logs): make nbctl usable from Nubi's workspace Nubi's workspace runs nbctl against a proxy in llm-server with a short-lived token, to fetch raw metrics and logs into files (nudgebee/nudgebee-enterprise#40463). - config: IsConfigured no longer requires username (unused for auth since the API key is sent directly as Bearer) - metrics list-metrics: call metrics_list_names; metrics_list is the series query action and returned nothing without queries - metrics query / logs query: -o json prints the backend results unchanged (no JSON-in-strings), and [] when empty; failed queries, notes and log suggestions go to stderr - metrics query: --step , sent as step_interval seconds; the request is sent as one $request: FetchMetricsRequest! variable - client: never write Authorization, cookies or X-Api-Key to nbctl_graphql.log (--verbose) - client: per-request timeout configurable via --http-timeout / NUDGEBEE_HTTP_TIMEOUT (default 30s) - NUDGEBEE_ENABLED_COMMANDS=metrics,logs removes other top-level commands - the GraphQL documents of the seven metrics/logs commands are named constants, pinned by a contract test Co-Authored-By: Claude Opus 5.5 --- README.md | 19 ++- cmd/logs_list_label_values.go | 15 ++- cmd/logs_list_labels.go | 15 ++- cmd/logs_query.go | 72 +++++++---- cmd/metrics_list_label_values.go | 15 ++- cmd/metrics_list_labels.go | 15 ++- cmd/metrics_list_metrics.go | 19 +-- cmd/metrics_list_metrics_test.go | 2 +- cmd/metrics_query.go | 96 +++++++++----- cmd/root.go | 31 +++++ cmd/workspace_contract_test.go | 214 +++++++++++++++++++++++++++++++ pkg/client/client.go | 56 +++++++- pkg/client/logging_test.go | 84 ++++++++++++ pkg/config/config.go | 3 +- pkg/config/config_test.go | 4 +- pkg/format/format.go | 13 ++ 16 files changed, 561 insertions(+), 112 deletions(-) create mode 100644 cmd/workspace_contract_test.go create mode 100644 pkg/client/logging_test.go diff --git a/README.md b/README.md index 09d43d1..b854ac9 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ This command interactively guides you through setting up a new configuration pro * **Nudgebee API Endpoint**: The URL of the Nudgebee API (e.g., `https://api.nudgebee.com`). * **Nudgebee API Key**: Your personal API token (`sk-nb-…`), created under **Settings → API Tokens**. `nbctl` sends it directly as a Bearer token on every request. Tokens created before direct token auth was supported are rejected with a 401 and must be recreated. -* **Nudgebee Username**: Your Nudgebee account username (e.g., your email). +* **Nudgebee Username**: Your Nudgebee account username (e.g., your email). Optional: used by `nubi` and `mcp`, not needed to authenticate. * **Default Account ID**: The ID of the Nudgebee account you wish to interact with by default. After collecting the information, `nbctl` will attempt to validate your credentials by making a test API call. @@ -171,12 +171,17 @@ Add the following to your Claude Desktop configuration file (usually `~/Library/ Once configured, restart Claude Desktop. You can then ask questions like "List my Nudgebee accounts" or "Show me high severity security recommendations". +### Limiting the available commands + +Set `NUDGEBEE_ENABLED_COMMANDS` to a comma-separated list of top-level commands (e.g. `metrics,logs`) to remove every other command from `nbctl` (`help`, `version` and `completion` always stay). This is meant for embedding `nbctl` in a restricted environment; it is a convenience, not an access control. + ### Persistent Flags The following flags can be used with any `nbctl` command: * `--log-level `: Sets the logging level. Accepted values are `debug`, `info` (default), `warn`, and `error`. -* `--verbose`: Enables verbose logging, including detailed GraphQL requests and responses. Useful for debugging API interactions. +* `--verbose`: Enables verbose logging, including detailed GraphQL requests and responses, to `nbctl_graphql.log` in the current directory. Credential headers (`Authorization`, cookies) are redacted. Useful for debugging API interactions. +* `--http-timeout `: Timeout for each API request, as a duration (`50s`, `2m`) or seconds. Default `30s`; `0` disables it. Also set by `NUDGEBEE_HTTP_TIMEOUT`. * `--format `: Specifies the output format for command results. Currently, `json` is supported in addition to the default human-readable `text` format. Example: @@ -538,6 +543,8 @@ Queries logs from the Nudgebee API based on various filters. * `--offset `: Specifies an offset for pagination. Default is 0. * `--only-message`: If set, only the log messages are displayed, without timestamp, severity, or labels. +With `-o json`, the backend's log entries are printed unchanged (an array of `{timestamp, severity, message, labels}`). A backend suggestion for an empty result is printed on stderr. + Example: ```bash @@ -598,13 +605,17 @@ Queries metrics from the Nudgebee API based on a PromQL-like query string and va * `--account-id `: The account ID to query metrics from. If not provided, it attempts to read it from the configuration. * `--start-time `: Filters metrics starting from this time. Defaults to 1 hour ago. * `--end-time `: Filters metrics up to this time. Defaults to the current time. - * `--metric-provider `: Filters metrics by a specific metric provider. - * `--only-metric`: If set, only the metric names are displayed, without attributes. + * `--step `: Resolution of a range query (e.g. `30s`, `5m`). Default: chosen by the backend. + * `--instant`: Run an instant query instead of a range query. + * `--chart`: Plot the series in the terminal. + +With `-o json`, the backend's `results` are printed unchanged (an array of `{query_key, query, payload: [{metric, timestamps, values}]}`), so large results can be redirected to a file and read by scripts. Failed queries and backend notes are reported on stderr. Example: ```bash nbctl metrics query --account-id 123e4567-e89b-12d3-a456-426614174000 --query "node_memory_usage_bytes" --start-time "2023-10-26T00:00:00Z" +nbctl metrics query --query 'rate(container_cpu_usage_seconds_total[5m])' --start-time "2026-10-01T00:00:00Z" --end-time "2026-10-08T00:00:00Z" --step 5m -o json > cpu.json ``` #### `nbctl nubi` diff --git a/cmd/logs_list_label_values.go b/cmd/logs_list_label_values.go index 61eb279..3024b58 100644 --- a/cmd/logs_list_label_values.go +++ b/cmd/logs_list_label_values.go @@ -10,6 +10,13 @@ import ( "github.com/spf13/cobra" ) +// LogsListLabelValuesQuery lists the values of a log label in a time window ($query is "start=&end="). +const LogsListLabelValuesQuery = `query FetchLogLabelValues($accountId: String!, $labelName: String!, $query: String!) { + logs_list_label_values(request: {account_id: $accountId, label_name: $labelName, request: {query: $query}}) { + value + } +}` + var logsListLabelValuesCmd = &cobra.Command{ Use: "list-label-values", Short: "List log label values", @@ -47,13 +54,7 @@ var logsListLabelValuesCmd = &cobra.Command{ query := fmt.Sprintf("start=%d&end=%d", startTime.UnixNano(), endTime.UnixNano()) - req := client.NewRequest(` - query FetchLogLabelValues($accountId: String!, $labelName: String!, $query: String!) { - logs_list_label_values(request: {account_id: $accountId, label_name: $labelName, request: {query: $query}}) { - value - } - } - `) + req := client.NewRequest(LogsListLabelValuesQuery) req.Var("accountId", accountId) req.Var("labelName", labelName) diff --git a/cmd/logs_list_labels.go b/cmd/logs_list_labels.go index 7eec8d2..5bd2478 100644 --- a/cmd/logs_list_labels.go +++ b/cmd/logs_list_labels.go @@ -10,6 +10,13 @@ import ( "github.com/spf13/cobra" ) +// LogsListLabelsQuery lists log labels in a time window ($query is "start=&end="). +const LogsListLabelsQuery = `query FetchLogLabels($accountId: String!, $query: String!) { + logs_list_labels(request: {account_id: $accountId, request: {query: $query}}) { + label + } +}` + var logsListLabelsCmd = &cobra.Command{ Use: "list-labels", Short: "List log labels", @@ -46,13 +53,7 @@ var logsListLabelsCmd = &cobra.Command{ query := fmt.Sprintf("start=%d&end=%d", startTime.UnixNano(), endTime.UnixNano()) - req := client.NewRequest(` - query FetchLogLabels($accountId: String!, $query: String!) { - logs_list_labels(request: {account_id: $accountId, request: {query: $query}}) { - label - } - } - `) + req := client.NewRequest(LogsListLabelsQuery) req.Var("accountId", accountId) req.Var("query", query) diff --git a/cmd/logs_query.go b/cmd/logs_query.go index d7dee58..617d337 100644 --- a/cmd/logs_query.go +++ b/cmd/logs_query.go @@ -11,6 +11,19 @@ import ( "github.com/spf13/cobra" ) +// LogsQueryQuery fetches log lines through the logs_list action. +const LogsQueryQuery = `query FetchLogs($request: FetchLogRequest!) { + logs_list(request: $request) { + logs { + timestamp + severity + message + labels + } + suggestion + } +}` + var logsQueryCmd = &cobra.Command{ Use: "query", Short: "Query logs", @@ -44,27 +57,12 @@ var logsQueryCmd = &cobra.Command{ return fmt.Errorf("invalid end-time format: %w", err) } - // Convert to Unix milliseconds - startTimeMs := startTime.UnixNano() / int64(time.Millisecond) - endTimeMs := endTime.UnixNano() / int64(time.Millisecond) - - req := client.NewRequest(` - query FetchLogs($request: FetchLogRequest!) { - logs_list(request: $request) { - logs { - timestamp - severity - message - labels - } - } - } - `) + req := client.NewRequest(LogsQueryQuery) requestVars := map[string]any{ "account_id": accountId, - "end_time": endTimeMs, - "start_time": startTimeMs, + "end_time": endTime.UnixMilli(), + "start_time": startTime.UnixMilli(), "query": queryStr, "limit": limit, "offset": offset, @@ -73,12 +71,8 @@ var logsQueryCmd = &cobra.Command{ var respData struct { LogsList struct { - Logs []struct { - Timestamp string `json:"timestamp"` - Severity string `json:"severity"` - Message string `json:"message"` - Labels json.RawMessage `json:"labels"` - } `json:"logs"` + Logs json.RawMessage `json:"logs"` + Suggestion string `json:"suggestion"` } `json:"logs_list"` } @@ -86,13 +80,37 @@ var logsQueryCmd = &cobra.Command{ return err } - if len(respData.LogsList.Logs) == 0 { - fmt.Println("No logs found.") + if respData.LogsList.Suggestion != "" { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Suggestion: %s\n", respData.LogsList.Suggestion) + } + + raw := respData.LogsList.Logs + if len(raw) == 0 || string(raw) == "null" { + raw = json.RawMessage("[]") + } + + // JSON output is the backend's log entries, unchanged. + if format.GetFormat().Get() == "json" { + return format.GetFormat().PrintRawJSON(raw) + } + + var logs []struct { + Timestamp string `json:"timestamp"` + Severity string `json:"severity"` + Message string `json:"message"` + Labels json.RawMessage `json:"labels"` + } + if err := json.Unmarshal(raw, &logs); err != nil { + return fmt.Errorf("failed to decode logs: %w", err) + } + + if len(logs) == 0 { + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "No logs found.") return nil } table := format.TabularData{ - Data: respData.LogsList.Logs, + Data: logs, Fields: []format.TableField{ {Header: "Timestamp", Field: "Timestamp"}, {Header: "Severity", Field: "Severity"}, diff --git a/cmd/metrics_list_label_values.go b/cmd/metrics_list_label_values.go index 50d9174..f67c5d9 100644 --- a/cmd/metrics_list_label_values.go +++ b/cmd/metrics_list_label_values.go @@ -8,6 +8,13 @@ import ( "github.com/spf13/cobra" ) +// MetricsListLabelValuesQuery lists the values of a metric label. +const MetricsListLabelValuesQuery = `query MetricsLabelValueList($accountId: String!, $labelName: String!) { + metrics_list_label_values(request: {account_id: $accountId, label: $labelName}) { + value + } +}` + var metricsListLabelValuesCmd = &cobra.Command{ Use: "list-label-values", Short: "List metric label values", @@ -21,13 +28,7 @@ var metricsListLabelValuesCmd = &cobra.Command{ label, _ := cmd.Flags().GetString("label") - req := client.NewRequest(` - query MetricsLabelValueList($accountId: String!, $labelName: String!) { - metrics_list_label_values(request: {account_id: $accountId, label: $labelName}) { - value - } - } - `) + req := client.NewRequest(MetricsListLabelValuesQuery) req.Var("accountId", accountId) req.Var("labelName", label) diff --git a/cmd/metrics_list_labels.go b/cmd/metrics_list_labels.go index 947de8d..f4aa736 100644 --- a/cmd/metrics_list_labels.go +++ b/cmd/metrics_list_labels.go @@ -8,6 +8,13 @@ import ( "github.com/spf13/cobra" ) +// MetricsListLabelsQuery lists the labels of a metric. +const MetricsListLabelsQuery = `query MetricsLabelList($accountId: String!, $metricName: String!) { + metrics_list_labels(request: {account_id: $accountId, metric: $metricName}) { + label + } +}` + var metricsListLabelsCmd = &cobra.Command{ Use: "list-labels", Short: "List metric labels", @@ -21,13 +28,7 @@ var metricsListLabelsCmd = &cobra.Command{ metric, _ := cmd.Flags().GetString("metric") - req := client.NewRequest(` - query MetricsLabelList($accountId: String!, $metricName: String!) { - metrics_list_labels(request: {account_id: $accountId, metric: $metricName}) { - label - } - } - `) + req := client.NewRequest(MetricsListLabelsQuery) req.Var("accountId", accountId) req.Var("metricName", metric) diff --git a/cmd/metrics_list_metrics.go b/cmd/metrics_list_metrics.go index 375d912..105152e 100644 --- a/cmd/metrics_list_metrics.go +++ b/cmd/metrics_list_metrics.go @@ -8,6 +8,14 @@ import ( "github.com/spf13/cobra" ) +// MetricsListNamesQuery lists metric names. metrics_list is the series query +// action (see metrics query), so names come from metrics_list_names. +const MetricsListNamesQuery = `query MetricsListNames($accountId: String!) { + metrics_list_names(request: {account_id: $accountId}) { + metric + } +}` + var metricsListMetricsCmd = &cobra.Command{ Use: "list-metrics", Short: "List metrics", @@ -19,20 +27,13 @@ var metricsListMetricsCmd = &cobra.Command{ return err } - req := client.NewRequest(` - query MetricsList($accountId: String!) { - metrics_list(request: {account_id: $accountId}) { - metric - } - } - `) - + req := client.NewRequest(MetricsListNamesQuery) req.Var("accountId", accountId) var respData struct { MetricsList []struct { Metric string `json:"metric"` - } `json:"metrics_list"` + } `json:"metrics_list_names"` } if err := graphqlClient.Run(context.Background(), req, &respData); err != nil { diff --git a/cmd/metrics_list_metrics_test.go b/cmd/metrics_list_metrics_test.go index 6dcd8cb..14442b5 100644 --- a/cmd/metrics_list_metrics_test.go +++ b/cmd/metrics_list_metrics_test.go @@ -8,7 +8,7 @@ import ( func TestMetricsListMetrics_Unit(t *testing.T) { mockData := map[string]any{ - "metrics_list": []map[string]any{ + "metrics_list_names": []map[string]any{ { "metric": "metric1", "attributes": "{}", diff --git a/cmd/metrics_query.go b/cmd/metrics_query.go index 9b10e7a..e43ca54 100644 --- a/cmd/metrics_query.go +++ b/cmd/metrics_query.go @@ -26,15 +26,32 @@ func renderChart(payload []MetricsResult) { } } +// MetricsQueryQuery runs a metrics query (PromQL or the account's own language) +// through the metrics_list action. results is passed through untouched. +const MetricsQueryQuery = `query MetricsQuery($request: FetchMetricsRequest!) { + metrics_list(request: $request) { + results + } +}` + type MetricsQueryResponse struct { MetricsQuery struct { Results []MetricsResponse `json:"results"` } `json:"metrics_list"` } +// metricsQueryRawResponse keeps results as sent by the API, for -o json. +type metricsQueryRawResponse struct { + MetricsQuery struct { + Results json.RawMessage `json:"results"` + } `json:"metrics_list"` +} + type MetricsResponse struct { QueryKey string `json:"query_key"` Payload []MetricsResult `json:"payload"` + Error *string `json:"error,omitempty"` + Note string `json:"note,omitempty"` } type MetricsResult struct { @@ -90,50 +107,60 @@ var metricsQueryCmd = &cobra.Command{ return fmt.Errorf("invalid end-time format: %w", err) } - // Convert to Unix milliseconds - startTimeMs := float64(startTime.UnixNano() / int64(time.Millisecond)) - endTimeMs := float64(endTime.UnixNano() / int64(time.Millisecond)) - - req := client.NewRequest(` - query MetricsQuery( - $account_id: String! - $queries: jsonb! - $instant: Boolean! - $start_time: Float! - $end_time: Float! - ) { - metrics_list( - request: { - account_id: $account_id - queries: $queries - instant: $instant - end_time: $end_time - start_time: $start_time - } - ) { - results - } - } - `) + step, _ := cmd.Flags().GetDuration("step") + if step < 0 { + return fmt.Errorf("invalid step: must not be negative") + } + + request := map[string]any{ + "account_id": accountId, + "queries": queries, + "instant": instant, + "start_time": startTime.UnixMilli(), + "end_time": endTime.UnixMilli(), + } + if step > 0 { + // step_interval is whole seconds; round a sub-second step up to 1s. + request["step_interval"] = max(1, int(step.Round(time.Second)/time.Second)) + } - req.Var("account_id", accountId) - req.Var("queries", queries) - req.Var("instant", instant) - req.Var("start_time", startTimeMs) - req.Var("end_time", endTimeMs) + req := client.NewRequest(MetricsQueryQuery) + req.Var("request", request) - var respData MetricsQueryResponse + var respData metricsQueryRawResponse if err := graphqlClient.Run(context.Background(), req, &respData); err != nil { return err } - if len(respData.MetricsQuery.Results) == 0 { + raw := respData.MetricsQuery.Results + if len(raw) == 0 || string(raw) == "null" { + raw = json.RawMessage("[]") + } + + var results []MetricsResponse + if err := json.Unmarshal(raw, &results); err != nil { + return fmt.Errorf("failed to decode metrics results: %w", err) + } + for _, r := range results { + if r.Error != nil && *r.Error != "" { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: query %q failed: %s\n", r.QueryKey, *r.Error) + } else if r.Note != "" { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Note: %s\n", r.Note) + } + } + + // JSON output is the backend's results, unchanged, so scripts can use it as is. + if format.GetFormat().Get() == "json" { + return format.GetFormat().PrintRawJSON(raw) + } + + if len(results) == 0 { format.GetFormat().Print("No Data") return nil } var displayPayload []DisplayMetricsResult - for _, r := range respData.MetricsQuery.Results[0].Payload { + for _, r := range results[0].Payload { metricJSON, err := json.Marshal(r.Metric) if err != nil { return fmt.Errorf("failed to marshal metric to JSON: %w", err) @@ -165,7 +192,7 @@ var metricsQueryCmd = &cobra.Command{ }, } if chart { - renderChart(respData.MetricsQuery.Results[0].Payload) + renderChart(results[0].Payload) } else { format.GetFormat().Print(table) } @@ -181,4 +208,5 @@ func init() { metricsQueryCmd.Flags().String("account-id", "", "Account ID") metricsQueryCmd.Flags().Bool("instant", false, "Instant query") metricsQueryCmd.Flags().Bool("chart", false, "Display data as a chart") + metricsQueryCmd.Flags().Duration("step", 0, "Resolution step for range queries, e.g. 30s, 5m (default: chosen by the backend)") } diff --git a/cmd/root.go b/cmd/root.go index d7399d0..dbc5f16 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -4,6 +4,7 @@ import ( "fmt" "log" "os" + "strings" "github.com/nudgebee/nbctl/pkg/config" "github.com/nudgebee/nbctl/pkg/format" @@ -115,6 +116,8 @@ func init() { _ = rootCmd.PersistentFlags().MarkHidden("output") rootCmd.PersistentFlags().String("profile", "", "Use a specific profile from your config file") _ = viper.BindPFlag("profile", rootCmd.PersistentFlags().Lookup("profile")) + rootCmd.PersistentFlags().String("http-timeout", "", "Timeout for each API request, e.g. 50s (env NUDGEBEE_HTTP_TIMEOUT; default 30s, 0 disables)") + _ = viper.BindPFlag("http-timeout", rootCmd.PersistentFlags().Lookup("http-timeout")) // Initialize a logger that writes to the command's stderr. Using PersistentPreRunE // ensures cmd.ErrOrStderr() is available during execution and in tests. @@ -143,7 +146,35 @@ func init() { } } +// enabledCommandsEnv limits nbctl to a comma-separated list of top-level +// command groups (e.g. "metrics,logs"), for embedding nbctl where only some +// commands are useful. It hides commands; it is not an access control. +const enabledCommandsEnv = "NUDGEBEE_ENABLED_COMMANDS" + +// alwaysEnabledCommands stay available whatever enabledCommandsEnv says. +var alwaysEnabledCommands = map[string]bool{"help": true, "version": true, "completion": true} + +// restrictCommands removes the top-level commands of root that are not listed +// in enabled (comma-separated). An empty list leaves root unchanged. +func restrictCommands(root *cobra.Command, enabled string) { + allowed := map[string]bool{} + for _, name := range strings.Split(enabled, ",") { + if name = strings.TrimSpace(name); name != "" { + allowed[name] = true + } + } + if len(allowed) == 0 { + return + } + for _, c := range root.Commands() { + if !allowed[c.Name()] && !alwaysEnabledCommands[c.Name()] { + root.RemoveCommand(c) + } + } +} + func Execute() { + restrictCommands(rootCmd, os.Getenv(enabledCommandsEnv)) if err := rootCmd.Execute(); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) diff --git a/cmd/workspace_contract_test.go b/cmd/workspace_contract_test.go new file mode 100644 index 0000000..1ed12e9 --- /dev/null +++ b/cmd/workspace_contract_test.go @@ -0,0 +1,214 @@ +package cmd + +import ( + "encoding/json" + "net/http" + "testing" + + "github.com/nudgebee/nbctl/pkg/testutil" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type capturedRequest struct { + Path string + Auth string + Query string `json:"query"` + Variables map[string]any `json:"variables"` +} + +// resetFlags puts every flag of the command at args back to its default; the +// shared rootCmd keeps flag values from earlier runs otherwise. +func resetFlags(t *testing.T, args []string) { + t.Helper() + c, _, err := rootCmd.Find(args) + require.NoError(t, err) + c.Flags().VisitAll(func(f *pflag.Flag) { + _ = f.Value.Set(f.DefValue) + f.Changed = false + }) +} + +// runCapturing runs args against a mock API that answers with data and +// returns the command output and every request nbctl sent. +func runCapturing(t *testing.T, data any, args ...string) (string, []capturedRequest) { + t.Helper() + resetFlags(t, args) + var reqs []capturedRequest + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var c capturedRequest + _ = json.NewDecoder(r.Body).Decode(&c) + c.Path = r.URL.Path + c.Auth = r.Header.Get("Authorization") + reqs = append(reqs, c) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"data": data}) + }) + out, err := testutil.RunWithMockServer(handler, map[string]any{ + "api-key": "sk-nb-test", + "account-id": "acc-1", + }, rootCmd, args) + require.NoError(t, err, out) + return out, reqs +} + +// The workspace proxy in llm-server allowlists exactly these documents; keep +// them in sync with its contract test when they change. +func TestWorkspaceCommandsGraphQLContract(t *testing.T) { + tests := []struct { + name string + args []string + data any + query string + wantVars func(t *testing.T, vars map[string]any) + }{ + { + name: "metrics list-metrics", + args: []string{"metrics", "list-metrics"}, + data: map[string]any{"metrics_list_names": []any{map[string]any{"metric": "up"}}}, + query: MetricsListNamesQuery, + wantVars: func(t *testing.T, v map[string]any) { + assert.Equal(t, map[string]any{"accountId": "acc-1"}, v) + }, + }, + { + name: "metrics list-labels", + args: []string{"metrics", "list-labels", "--metric", "up"}, + data: map[string]any{"metrics_list_labels": []any{}}, + query: MetricsListLabelsQuery, + wantVars: func(t *testing.T, v map[string]any) { + assert.Equal(t, map[string]any{"accountId": "acc-1", "metricName": "up"}, v) + }, + }, + { + name: "metrics list-label-values", + args: []string{"metrics", "list-label-values", "--label", "job"}, + data: map[string]any{"metrics_list_label_values": []any{}}, + query: MetricsListLabelValuesQuery, + wantVars: func(t *testing.T, v map[string]any) { + assert.Equal(t, map[string]any{"accountId": "acc-1", "labelName": "job"}, v) + }, + }, + { + name: "metrics query", + args: []string{"metrics", "query", "--query", "up", "--step", "5m", + "--start-time", "2026-10-01T00:00:00Z", "--end-time", "2026-10-01T01:00:00Z"}, + data: map[string]any{"metrics_list": map[string]any{"results": []any{}}}, + query: MetricsQueryQuery, + wantVars: func(t *testing.T, v map[string]any) { + assert.Equal(t, map[string]any{"request": map[string]any{ + "account_id": "acc-1", + "queries": map[string]any{"query": "up"}, + "instant": false, + "start_time": float64(1790812800000), + "end_time": float64(1790816400000), + "step_interval": float64(300), + }}, v) + }, + }, + { + name: "logs list-labels", + args: []string{"logs", "list-labels", "--start-time", "2026-10-01T00:00:00Z", "--end-time", "2026-10-01T01:00:00Z"}, + data: map[string]any{"logs_list_labels": []any{}}, + query: LogsListLabelsQuery, + wantVars: func(t *testing.T, v map[string]any) { + assert.Equal(t, map[string]any{"accountId": "acc-1", "query": "start=1790812800000000000&end=1790816400000000000"}, v) + }, + }, + { + name: "logs list-label-values", + args: []string{"logs", "list-label-values", "--label-name", "app", + "--start-time", "2026-10-01T00:00:00Z", "--end-time", "2026-10-01T01:00:00Z"}, + data: map[string]any{"logs_list_label_values": []any{}}, + query: LogsListLabelValuesQuery, + wantVars: func(t *testing.T, v map[string]any) { + assert.Equal(t, map[string]any{"accountId": "acc-1", "labelName": "app", "query": "start=1790812800000000000&end=1790816400000000000"}, v) + }, + }, + { + name: "logs query", + args: []string{"logs", "query", "--query", `{app="api"}`, "--limit", "500", "--offset", "100", + "--start-time", "2026-10-01T00:00:00Z", "--end-time", "2026-10-01T01:00:00Z"}, + data: map[string]any{"logs_list": map[string]any{"logs": []any{}}}, + query: LogsQueryQuery, + wantVars: func(t *testing.T, v map[string]any) { + assert.Equal(t, map[string]any{"request": map[string]any{ + "account_id": "acc-1", + "query": `{app="api"}`, + "start_time": float64(1790812800000), + "end_time": float64(1790816400000), + "limit": float64(500), + "offset": float64(100), + }}, v) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, reqs := runCapturing(t, tt.data, tt.args...) + require.Len(t, reqs, 1, "one GraphQL request per command, nothing else") + assert.Equal(t, "/api/graphql", reqs[0].Path) + assert.Equal(t, "Bearer sk-nb-test", reqs[0].Auth) + assert.Equal(t, tt.query, reqs[0].Query) + tt.wantVars(t, reqs[0].Variables) + }) + } +} + +func TestMetricsQueryOmitsStepByDefault(t *testing.T) { + _, reqs := runCapturing(t, map[string]any{"metrics_list": map[string]any{"results": []any{}}}, + "metrics", "query", "--query", "up") + require.Len(t, reqs, 1) + assert.NotContains(t, reqs[0].Variables["request"], "step_interval") +} + +func TestMetricsQueryJSONIsBackendResults(t *testing.T) { + results := `[{"query_key":"query","query":"up","payload":[{"metric":{"__name__":"up","job":"api"},"timestamps":[1790812800,1790812860],"values":[1,null],"non_finite":{"nan":1}}]}]` + var data any + require.NoError(t, json.Unmarshal([]byte(`{"metrics_list":{"results":`+results+`}}`), &data)) + + out, _ := runCapturing(t, data, "metrics", "query", "--query", "up", "-o", "json") + assert.JSONEq(t, results, out) +} + +func TestMetricsQueryJSONEmpty(t *testing.T) { + out, _ := runCapturing(t, map[string]any{"metrics_list": map[string]any{"results": nil}}, + "metrics", "query", "--query", "up", "-o", "json") + assert.JSONEq(t, `[]`, out) +} + +func TestLogsQueryJSONIsBackendLogs(t *testing.T) { + logs := `[{"timestamp":"2026-10-01T00:00:01Z","severity":"error","message":"boom","labels":{"app":"api","pod":"api-1"}}]` + var data any + require.NoError(t, json.Unmarshal([]byte(`{"logs_list":{"logs":`+logs+`}}`), &data)) + + out, _ := runCapturing(t, data, "logs", "query", "--query", `{app="api"}`, "-o", "json") + assert.JSONEq(t, logs, out) + + out, _ = runCapturing(t, map[string]any{"logs_list": map[string]any{"logs": []any{}}}, + "logs", "query", "--query", `{app="api"}`, "-o", "json") + assert.JSONEq(t, `[]`, out) +} + +func TestRestrictCommands(t *testing.T) { + root := &cobra.Command{Use: "nbctl"} + for _, name := range []string{"metrics", "logs", "tickets", "workflow", "version", "completion"} { + root.AddCommand(&cobra.Command{Use: name}) + } + names := func() []string { + var out []string + for _, c := range root.Commands() { + out = append(out, c.Name()) + } + return out + } + + restrictCommands(root, "") + assert.Len(t, names(), 6, "an empty list changes nothing") + + restrictCommands(root, " metrics, logs ,") + assert.ElementsMatch(t, []string{"metrics", "logs", "version", "completion"}, names()) +} diff --git a/pkg/client/client.go b/pkg/client/client.go index e63e1be..ea907d0 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -11,6 +11,7 @@ import ( "net/http" "net/http/httputil" "os" + "strconv" "strings" "sync" "time" @@ -58,16 +59,36 @@ type loggingTransport struct { logger *log.Logger } +// sensitiveHeaders are never written to the verbose log. The log lands in the +// current directory, which may be kept or shared (e.g. a Nubi workspace). +var sensitiveHeaders = []string{"Authorization", "Proxy-Authorization", "Cookie", "Set-Cookie", "X-Api-Key"} + +// redactHeaders returns a copy of h with sensitive values replaced. +func redactHeaders(h http.Header) http.Header { + out := h.Clone() + for _, name := range sensitiveHeaders { + if out.Get(name) != "" { + out.Set(name, "[REDACTED]") + } + } + return out +} + func (t *loggingTransport) RoundTrip(req *http.Request) (*http.Response, error) { - // Log the request - reqDump, err := httputil.DumpRequestOut(req, true) + // Log the request. Dump a clone with redacted headers, then send the clone + // with the real headers: DumpRequestOut consumes and restores the body of + // the request it is given, so the clone is the one with a readable body. + logged := req.Clone(req.Context()) + logged.Header = redactHeaders(req.Header) + reqDump, err := httputil.DumpRequestOut(logged, true) if err != nil { t.logger.Printf("Error dumping request: %v", err) } else { t.logger.Printf("Request:\n%s", reqDump) } + logged.Header = req.Header.Clone() - resp, err := t.wrapped.RoundTrip(req) + resp, err := t.wrapped.RoundTrip(logged) if err != nil { t.logger.Printf("Error sending request: %v", err) return nil, err @@ -87,8 +108,10 @@ func (t *loggingTransport) RoundTrip(req *http.Request) (*http.Response, error) // Create a new response with the same body, so it can be read again. resp.Body = io.NopCloser(bytes.NewBuffer(body)) - // Dump the response for logging. - respDump, dumpErr := httputil.DumpResponse(resp, true) + // Dump the response for logging, without sensitive headers. + loggedResp := *resp + loggedResp.Header = redactHeaders(resp.Header) + respDump, dumpErr := httputil.DumpResponse(&loggedResp, true) if dumpErr != nil { t.logger.Printf("Error dumping response: %v", dumpErr) } else { @@ -202,10 +225,31 @@ func newTransport(apiKey string) http.RoundTripper { return transport } +// DefaultHTTPTimeout bounds each HTTP request unless http-timeout is set. +const DefaultHTTPTimeout = 30 * time.Second + +// httpTimeout reads http-timeout (flag --http-timeout, env NUDGEBEE_HTTP_TIMEOUT) +// as a Go duration ("50s", "2m") or a number of seconds. 0 disables the timeout. +// An invalid value falls back to the default with a warning. +func httpTimeout() time.Duration { + raw := strings.TrimSpace(viper.GetString("http-timeout")) + if raw == "" { + return DefaultHTTPTimeout + } + if d, err := time.ParseDuration(raw); err == nil && d >= 0 { + return d + } + if secs, err := strconv.ParseFloat(raw, 64); err == nil && secs >= 0 { + return time.Duration(secs * float64(time.Second)) + } + fmt.Fprintf(os.Stderr, "Warning: invalid http-timeout %q, using %s\n", raw, DefaultHTTPTimeout) + return DefaultHTTPTimeout +} + func newHTTPClient(config clientOptions) *http.Client { return &http.Client{ Transport: newTransport(config.apiKey), - Timeout: 30 * time.Second, + Timeout: httpTimeout(), } } diff --git a/pkg/client/logging_test.go b/pkg/client/logging_test.go new file mode 100644 index 0000000..c4b3e38 --- /dev/null +++ b/pkg/client/logging_test.go @@ -0,0 +1,84 @@ +package client + +import ( + "bytes" + "context" + "io" + "log" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/spf13/viper" +) + +func TestVerboseLogRedactsCredentials(t *testing.T) { + const apiKey = "sk-nb-secret-value" + var seenAuth, seenBody string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + seenAuth = r.Header.Get("Authorization") + b, _ := io.ReadAll(r.Body) + seenBody = string(b) + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Set-Cookie", "session=cookie-secret") + _, _ = w.Write([]byte(`{"data": {"ok": true}}`)) + })) + defer srv.Close() + + var logBuf bytes.Buffer + transport := &loggingTransport{ + wrapped: &authTransport{apiKey: apiKey, wrapped: http.DefaultTransport}, + logger: log.New(&logBuf, "", 0), + } + c := &Client{endpoint: srv.URL + "/api/graphql", apiKey: apiKey, httpClient: &http.Client{Transport: transport}} + + req := NewRequest(`query { ok }`) + req.Header("Authorization", "Bearer "+apiKey) // a caller-set header must be redacted too + req.Header("Cookie", "session=cookie-secret") + var resp map[string]any + if err := c.Run(context.Background(), req, &resp); err != nil { + t.Fatalf("run failed: %v", err) + } + + if seenAuth != "Bearer "+apiKey { + t.Fatalf("server should still get the real Authorization header, got %q", seenAuth) + } + if !strings.Contains(seenBody, "query { ok }") { + t.Fatalf("server should get the full request body, got %q", seenBody) + } + logged := logBuf.String() + for _, secret := range []string{apiKey, "cookie-secret"} { + if strings.Contains(logged, secret) { + t.Fatalf("verbose log contains %q:\n%s", secret, logged) + } + } + if !strings.Contains(logged, "[REDACTED]") || !strings.Contains(logged, "query { ok }") { + t.Fatalf("verbose log should keep the request with redacted headers:\n%s", logged) + } +} + +func TestHTTPTimeout(t *testing.T) { + defer viper.Set("http-timeout", "") + cases := map[string]time.Duration{ + "": DefaultHTTPTimeout, + "50s": 50 * time.Second, + "2m": 2 * time.Minute, + "45": 45 * time.Second, + "0": 0, + "nope": DefaultHTTPTimeout, + "-5s": DefaultHTTPTimeout, + } + for raw, want := range cases { + viper.Set("http-timeout", raw) + if got := httpTimeout(); got != want { + t.Errorf("http-timeout %q: got %s, want %s", raw, got, want) + } + } + + viper.Set("http-timeout", "55s") + if got := NewHTTPClient(WithApiKey("k")).Timeout; got != 55*time.Second { + t.Errorf("client timeout: got %s, want 55s", got) + } +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 6ff028f..c3d8ffc 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -17,10 +17,11 @@ func Reset() { viper.Reset() } +// IsConfigured reports whether the settings every API call needs are present. +// username is not required: the API key authenticates on its own. func IsConfigured() bool { return viper.GetString("endpoint") != "" && viper.GetString("api-key") != "" && - viper.GetString("username") != "" && viper.GetString("account-id") != "" } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 0a23e27..c29405f 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -132,7 +132,7 @@ func TestIsConfigured(t *testing.T) { assert.False(t, IsConfigured()) }) - t.Run("returns false when username is missing", func(t *testing.T) { + t.Run("returns true when username is missing", func(t *testing.T) { defer WithViper(map[string]any{ "endpoint": "http://test.com", "api-key": "test-key", @@ -140,7 +140,7 @@ func TestIsConfigured(t *testing.T) { "account-id": "test-account", })() - assert.False(t, IsConfigured()) + assert.True(t, IsConfigured()) }) t.Run("returns false when account-id is missing", func(t *testing.T) { diff --git a/pkg/format/format.go b/pkg/format/format.go index ca1f65c..ea4a926 100644 --- a/pkg/format/format.go +++ b/pkg/format/format.go @@ -1,6 +1,7 @@ package format import ( + "bytes" "encoding/json" "fmt" "io" @@ -83,6 +84,18 @@ func (f *Format) printJSON(obj any) { } } +// PrintRawJSON writes JSON from the API as is, only indented, so values keep +// their original types and fields instead of passing through Go structs. +func (f *Format) PrintRawJSON(raw json.RawMessage) error { + var buf bytes.Buffer + if err := json.Indent(&buf, raw, "", " "); err != nil { + return fmt.Errorf("invalid JSON from the API: %w", err) + } + buf.WriteByte('\n') + _, err := f.writer.Write(buf.Bytes()) + return err +} + func (f *Format) printText(obj any) { if tabularData, ok := obj.(TabularData); ok { f.printTabularData(tabularData) From 9b9dc58cafc85de3b813c100a706b83f6197d83a Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 8 Oct 2026 12:49:34 +0530 Subject: [PATCH 2/6] fix(config): NUDGEBEE_* env vars take precedence over profile settings A profile in ~/.nudgebee/config.yaml was applied with viper.Set and so overrode env, e.g. the endpoint and token Nubi's workspace sets per command. Co-Authored-By: Claude Opus 5.5 --- pkg/config/config.go | 21 ++++++++++++++------- pkg/config/config_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index c3d8ffc..286beac 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -63,10 +63,7 @@ func InitConfig() { fmt.Fprintf(os.Stderr, "Error: profile '%s' not found\n", profile) os.Exit(1) } - profileSettings := viper.GetStringMapString(fmt.Sprintf("profiles.%s", profile)) - for key, value := range profileSettings { - viper.Set(key, value) - } + applyProfile(profile) return } @@ -88,9 +85,19 @@ func InitConfig() { } if currentProfile != "" { - profileSettings := viper.GetStringMapString(fmt.Sprintf("profiles.%s", currentProfile)) - for key, value := range profileSettings { - viper.Set(key, value) + applyProfile(currentProfile) + } +} + +// applyProfile copies a profile's settings into viper. A setting also given as +// a NUDGEBEE_* environment variable keeps the env value, so an environment that +// configures nbctl by env is not overridden by a config file in $HOME. +func applyProfile(name string) { + for key, value := range viper.GetStringMapString(fmt.Sprintf("profiles.%s", name)) { + envName := "NUDGEBEE_" + strings.ToUpper(strings.ReplaceAll(key, "-", "_")) + if _, set := os.LookupEnv(envName); set { + continue } + viper.Set(key, value) } } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index c29405f..43ace0c 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -96,6 +96,30 @@ current-profile: prof1 assert.Equal(t, "prof1-user", viper.GetString("username")) assert.Equal(t, "prof1-account", viper.GetString("account-id")) }) + + t.Run("env overrides profile", func(t *testing.T) { + tmpdir := t.TempDir() + configPath := filepath.Join(tmpdir, ".nudgebee") + require.NoError(t, os.MkdirAll(configPath, 0755)) + require.NoError(t, os.WriteFile(filepath.Join(configPath, "config.yaml"), []byte(` +profiles: + prof1: + endpoint: http://prof1.com + api-key: prof1-key + account-id: prof1-account +current-profile: prof1 +`), 0644)) + t.Setenv("HOME", tmpdir) + t.Setenv("NUDGEBEE_ENDPOINT", "http://env.com") + t.Setenv("NUDGEBEE_API_KEY", "env-key") + + Reset() + InitConfig() + + assert.Equal(t, "http://env.com", viper.GetString("endpoint")) + assert.Equal(t, "env-key", viper.GetString("api-key")) + assert.Equal(t, "prof1-account", viper.GetString("account-id")) + }) } func TestIsConfigured(t *testing.T) { From 0be37eaf2a777fae06987fe9a7d44c922148407a Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 8 Oct 2026 18:26:42 +0530 Subject: [PATCH 3/6] refactor(cmd): iterate over a copy of commands in restrictCommands Co-Authored-By: Claude Opus 5.5 --- cmd/root.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/root.go b/cmd/root.go index dbc5f16..73e27ef 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -166,7 +166,8 @@ func restrictCommands(root *cobra.Command, enabled string) { if len(allowed) == 0 { return } - for _, c := range root.Commands() { + // Iterate over a copy so the loop does not depend on how cobra stores commands. + for _, c := range append([]*cobra.Command(nil), root.Commands()...) { if !allowed[c.Name()] && !alwaysEnabledCommands[c.Name()] { root.RemoveCommand(c) } From 1c9bd75e4345d08fddd675e27708b8c8a2a71f52 Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 8 Oct 2026 18:31:00 +0530 Subject: [PATCH 4/6] fix(metrics,config): pass unexpected result shapes through in -o json; ignore empty env vars over profiles - metrics query -o json no longer fails when results don't fit the typed structs (e.g. a non-string label value); decoding is only needed for text output and the stderr warnings - an empty NUDGEBEE_* env var no longer masks the profile setting (viper ignores empty env vars, so the setting ended up blank) Co-Authored-By: Claude Opus 5.5 --- cmd/metrics_query.go | 7 +++++-- cmd/workspace_contract_test.go | 11 +++++++++++ pkg/config/config.go | 4 +++- pkg/config/config_test.go | 1 + 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/cmd/metrics_query.go b/cmd/metrics_query.go index e43ca54..2d8cebd 100644 --- a/cmd/metrics_query.go +++ b/cmd/metrics_query.go @@ -137,8 +137,11 @@ var metricsQueryCmd = &cobra.Command{ raw = json.RawMessage("[]") } + // JSON output passes results through, so only text output needs them to + // decode; for JSON the decode is best-effort, for the warnings below. + jsonOutput := format.GetFormat().Get() == "json" var results []MetricsResponse - if err := json.Unmarshal(raw, &results); err != nil { + if err := json.Unmarshal(raw, &results); err != nil && !jsonOutput { return fmt.Errorf("failed to decode metrics results: %w", err) } for _, r := range results { @@ -150,7 +153,7 @@ var metricsQueryCmd = &cobra.Command{ } // JSON output is the backend's results, unchanged, so scripts can use it as is. - if format.GetFormat().Get() == "json" { + if jsonOutput { return format.GetFormat().PrintRawJSON(raw) } diff --git a/cmd/workspace_contract_test.go b/cmd/workspace_contract_test.go index 1ed12e9..0b35c01 100644 --- a/cmd/workspace_contract_test.go +++ b/cmd/workspace_contract_test.go @@ -174,6 +174,17 @@ func TestMetricsQueryJSONIsBackendResults(t *testing.T) { assert.JSONEq(t, results, out) } +func TestMetricsQueryJSONPassesThroughUnexpectedShapes(t *testing.T) { + // A label value that is not a string would not decode into MetricsResult; + // JSON output must still print it unchanged. + results := `[{"query_key":"query","payload":[{"metric":{"le":0.5,"job":"api"},"timestamps":[1],"values":[2]}]}]` + var data any + require.NoError(t, json.Unmarshal([]byte(`{"metrics_list":{"results":`+results+`}}`), &data)) + + out, _ := runCapturing(t, data, "metrics", "query", "--query", "up", "-o", "json") + assert.JSONEq(t, results, out) +} + func TestMetricsQueryJSONEmpty(t *testing.T) { out, _ := runCapturing(t, map[string]any{"metrics_list": map[string]any{"results": nil}}, "metrics", "query", "--query", "up", "-o", "json") diff --git a/pkg/config/config.go b/pkg/config/config.go index 286beac..ac69be1 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -95,7 +95,9 @@ func InitConfig() { func applyProfile(name string) { for key, value := range viper.GetStringMapString(fmt.Sprintf("profiles.%s", name)) { envName := "NUDGEBEE_" + strings.ToUpper(strings.ReplaceAll(key, "-", "_")) - if _, set := os.LookupEnv(envName); set { + // Non-empty only: viper ignores an empty env var, so skipping the + // profile for one would leave the setting blank. + if os.Getenv(envName) != "" { continue } viper.Set(key, value) diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 43ace0c..d2e1c34 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -112,6 +112,7 @@ current-profile: prof1 t.Setenv("HOME", tmpdir) t.Setenv("NUDGEBEE_ENDPOINT", "http://env.com") t.Setenv("NUDGEBEE_API_KEY", "env-key") + t.Setenv("NUDGEBEE_ACCOUNT_ID", "") // empty env does not mask the profile Reset() InitConfig() From c5baa1cd7006a206d81963d917d005b441241df4 Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 8 Oct 2026 21:00:22 +0530 Subject: [PATCH 5/6] feat(metrics,logs): explain empty results and a hit --limit on stderr - list-metrics, list-labels, list-label-values, metrics query and logs query say on stderr when they found nothing (with the label/metric and window), so an empty stdout is not mistaken for a silent failure; -o json still prints [] (metrics query: the results unchanged) - logs query warns when it returned exactly --limit lines and gives the --offset for the next page - contract test documents the request fields the llm-server proxy allows Co-Authored-By: Claude Opus 5.5 --- README.md | 4 +- cmd/list_output.go | 24 ++++++++++ cmd/logs_list_label_values.go | 4 +- cmd/logs_list_labels.go | 4 +- cmd/logs_query.go | 34 ++++++++++---- cmd/metrics_list_label_values.go | 5 +- cmd/metrics_list_labels.go | 5 +- cmd/metrics_list_metrics.go | 4 +- cmd/metrics_query.go | 8 +++- cmd/workspace_contract_test.go | 81 +++++++++++++++++++++++++++++++- 10 files changed, 145 insertions(+), 28 deletions(-) create mode 100644 cmd/list_output.go diff --git a/README.md b/README.md index b854ac9..c6e8471 100644 --- a/README.md +++ b/README.md @@ -543,7 +543,9 @@ Queries logs from the Nudgebee API based on various filters. * `--offset `: Specifies an offset for pagination. Default is 0. * `--only-message`: If set, only the log messages are displayed, without timestamp, severity, or labels. -With `-o json`, the backend's log entries are printed unchanged (an array of `{timestamp, severity, message, labels}`). A backend suggestion for an empty result is printed on stderr. +With `-o json`, the backend's log entries are printed unchanged (an array of `{timestamp, severity, message, labels}`). When the result has exactly `--limit` lines, a warning on stderr says it is probably cut off and gives the `--offset` for the next page. + +The `metrics` and `logs` commands report an empty result on stderr (e.g. `No values found for log label "severity" ...`), so an empty stdout is never ambiguous; with `-o json` stdout is still `[]`. Example: diff --git a/cmd/list_output.go b/cmd/list_output.go new file mode 100644 index 0000000..198af69 --- /dev/null +++ b/cmd/list_output.go @@ -0,0 +1,24 @@ +package cmd + +import ( + "encoding/json" + "fmt" + + "github.com/nudgebee/nbctl/pkg/format" + "github.com/spf13/cobra" +) + +// printRows prints a list result. An empty result prints nothing in text mode +// and [] in JSON mode, and explains itself with emptyMsg on stderr, so callers +// (people and scripts alike) can tell "nothing found" from a silent failure. +func printRows(cmd *cobra.Command, table format.TabularData, count int, emptyMsg string) error { + if count > 0 { + format.GetFormat().Print(table) + return nil + } + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), emptyMsg) + if format.GetFormat().Get() == "json" { + return format.GetFormat().PrintRawJSON(json.RawMessage("[]")) + } + return nil +} diff --git a/cmd/logs_list_label_values.go b/cmd/logs_list_label_values.go index 3024b58..e1bc274 100644 --- a/cmd/logs_list_label_values.go +++ b/cmd/logs_list_label_values.go @@ -76,9 +76,7 @@ var logsListLabelValuesCmd = &cobra.Command{ {Header: "Value", Field: "Value"}, }, } - format.GetFormat().Print(table) - - return nil + return printRows(cmd, table, len(respData.LogsListLabelValues), fmt.Sprintf("No values found for log label %q between %s and %s (it may be a field inside log lines rather than an indexed label).", labelName, startTime.Format(time.RFC3339), endTime.Format(time.RFC3339))) }, } diff --git a/cmd/logs_list_labels.go b/cmd/logs_list_labels.go index 5bd2478..a075aed 100644 --- a/cmd/logs_list_labels.go +++ b/cmd/logs_list_labels.go @@ -74,9 +74,7 @@ var logsListLabelsCmd = &cobra.Command{ {Header: "Label", Field: "Label"}, }, } - format.GetFormat().Print(table) - - return nil + return printRows(cmd, table, len(respData.LogsListLabels), fmt.Sprintf("No log labels found between %s and %s.", startTime.Format(time.RFC3339), endTime.Format(time.RFC3339))) }, } diff --git a/cmd/logs_query.go b/cmd/logs_query.go index 617d337..452dd8f 100644 --- a/cmd/logs_query.go +++ b/cmd/logs_query.go @@ -89,23 +89,39 @@ var logsQueryCmd = &cobra.Command{ raw = json.RawMessage("[]") } - // JSON output is the backend's log entries, unchanged. - if format.GetFormat().Get() == "json" { - return format.GetFormat().PrintRawJSON(raw) - } - var logs []struct { Timestamp string `json:"timestamp"` Severity string `json:"severity"` Message string `json:"message"` Labels json.RawMessage `json:"labels"` } - if err := json.Unmarshal(raw, &logs); err != nil { - return fmt.Errorf("failed to decode logs: %w", err) + // JSON output passes entries through, so for JSON only count them. + jsonOutput := format.GetFormat().Get() == "json" + count := 0 + if jsonOutput { + var entries []json.RawMessage + _ = json.Unmarshal(raw, &entries) + count = len(entries) + } else { + if err := json.Unmarshal(raw, &logs); err != nil { + return fmt.Errorf("failed to decode logs: %w", err) + } + count = len(logs) } - if len(logs) == 0 { - _, _ = fmt.Fprintln(cmd.OutOrStdout(), "No logs found.") + window := fmt.Sprintf("between %s and %s", startTime.Format(time.RFC3339), endTime.Format(time.RFC3339)) + switch { + case count == 0: + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "No logs found %s.\n", window) + case limit > 0 && count >= limit: + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Returned %d lines = --limit; results are probably cut off. Narrow --start-time/--end-time or the query, or page with --offset %d.\n", count, offset+count) + } + + // JSON output is the backend's log entries, unchanged. + if jsonOutput { + return format.GetFormat().PrintRawJSON(raw) + } + if count == 0 { return nil } diff --git a/cmd/metrics_list_label_values.go b/cmd/metrics_list_label_values.go index f67c5d9..acd0f4d 100644 --- a/cmd/metrics_list_label_values.go +++ b/cmd/metrics_list_label_values.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "fmt" "github.com/nudgebee/nbctl/pkg/client" "github.com/nudgebee/nbctl/pkg/format" @@ -49,9 +50,7 @@ var metricsListLabelValuesCmd = &cobra.Command{ {Header: "Value", Field: "Value"}, }, } - format.GetFormat().Print(table) - - return nil + return printRows(cmd, table, len(respData.MetricsListLabelValues), fmt.Sprintf("No values found for label %q.", label)) }, } diff --git a/cmd/metrics_list_labels.go b/cmd/metrics_list_labels.go index f4aa736..1e32e6e 100644 --- a/cmd/metrics_list_labels.go +++ b/cmd/metrics_list_labels.go @@ -2,6 +2,7 @@ package cmd import ( "context" + "fmt" "github.com/nudgebee/nbctl/pkg/client" "github.com/nudgebee/nbctl/pkg/format" @@ -49,9 +50,7 @@ var metricsListLabelsCmd = &cobra.Command{ {Header: "Label", Field: "Label"}, }, } - format.GetFormat().Print(table) - - return nil + return printRows(cmd, table, len(respData.MetricsListLabels), fmt.Sprintf("No labels found for metric %q.", metric)) }, } diff --git a/cmd/metrics_list_metrics.go b/cmd/metrics_list_metrics.go index 105152e..26c760a 100644 --- a/cmd/metrics_list_metrics.go +++ b/cmd/metrics_list_metrics.go @@ -46,9 +46,7 @@ var metricsListMetricsCmd = &cobra.Command{ {Header: "Metric", Field: "Metric"}, }, } - format.GetFormat().Print(table) - - return nil + return printRows(cmd, table, len(respData.MetricsList), "No metrics found for this account.") }, } diff --git a/cmd/metrics_query.go b/cmd/metrics_query.go index 2d8cebd..1f68fd6 100644 --- a/cmd/metrics_query.go +++ b/cmd/metrics_query.go @@ -144,8 +144,11 @@ var metricsQueryCmd = &cobra.Command{ if err := json.Unmarshal(raw, &results); err != nil && !jsonOutput { return fmt.Errorf("failed to decode metrics results: %w", err) } + series, failed := 0, false for _, r := range results { + series += len(r.Payload) if r.Error != nil && *r.Error != "" { + failed = true _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: query %q failed: %s\n", r.QueryKey, *r.Error) } else if r.Note != "" { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Note: %s\n", r.Note) @@ -153,12 +156,15 @@ var metricsQueryCmd = &cobra.Command{ } // JSON output is the backend's results, unchanged, so scripts can use it as is. + if series == 0 && !failed { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "No data: the query returned no series between %s and %s.\n", startTime.Format(time.RFC3339), endTime.Format(time.RFC3339)) + } + if jsonOutput { return format.GetFormat().PrintRawJSON(raw) } if len(results) == 0 { - format.GetFormat().Print("No Data") return nil } diff --git a/cmd/workspace_contract_test.go b/cmd/workspace_contract_test.go index 0b35c01..655f179 100644 --- a/cmd/workspace_contract_test.go +++ b/cmd/workspace_contract_test.go @@ -1,8 +1,10 @@ package cmd import ( + "bytes" "encoding/json" "net/http" + "strings" "testing" "github.com/nudgebee/nbctl/pkg/testutil" @@ -54,8 +56,18 @@ func runCapturing(t *testing.T, data any, args ...string) (string, []capturedReq return out, reqs } -// The workspace proxy in llm-server allowlists exactly these documents; keep -// them in sync with its contract test when they change. +// The workspace proxy in llm-server (nudgebee-enterprise#40619) allowlists +// these documents and refuses any request field beyond these, per action: +// +// metrics_list_names: account_id +// metrics_list_labels: account_id, metric +// metrics_list_label_values: account_id, label +// metrics_list: account_id, queries{query}, instant, start_time, end_time, step_interval +// logs_list_labels: account_id, request{query} +// logs_list_label_values: account_id, label_name, request{query} +// logs_list: account_id, query, start_time, end_time, limit, offset +// +// Adding or renaming a field here needs the same change in the proxy. func TestWorkspaceCommandsGraphQLContract(t *testing.T) { tests := []struct { name string @@ -223,3 +235,68 @@ func TestRestrictCommands(t *testing.T) { restrictCommands(root, " metrics, logs ,") assert.ElementsMatch(t, []string{"metrics", "logs", "version", "completion"}, names()) } + +// runCapturingStderr is runCapturing that also returns what went to stderr. +func runCapturingStderr(t *testing.T, data any, args ...string) (string, string) { + t.Helper() + var errBuf bytes.Buffer + rootCmd.SetErr(&errBuf) + defer rootCmd.SetErr(nil) + out, _ := runCapturing(t, data, args...) + return out, errBuf.String() +} + +func TestEmptyResultsExplainOnStderr(t *testing.T) { + window := []string{"--start-time", "2026-10-01T00:00:00Z", "--end-time", "2026-10-01T01:00:00Z"} + tests := []struct { + name string + args []string + data any + wantErr string + }{ + {"metrics list-metrics", []string{"metrics", "list-metrics"}, + map[string]any{"metrics_list_names": []any{}}, "No metrics found"}, + {"metrics list-labels", []string{"metrics", "list-labels", "--metric", "up"}, + map[string]any{"metrics_list_labels": nil}, `No labels found for metric "up"`}, + {"metrics list-label-values", []string{"metrics", "list-label-values", "--label", "job"}, + map[string]any{"metrics_list_label_values": []any{}}, `No values found for label "job"`}, + {"logs list-labels", append([]string{"logs", "list-labels"}, window...), + map[string]any{"logs_list_labels": []any{}}, "No log labels found between 2026-10-01T00:00:00Z and 2026-10-01T01:00:00Z"}, + {"logs list-label-values", append([]string{"logs", "list-label-values", "--label-name", "severity"}, window...), + map[string]any{"logs_list_label_values": []any{}}, `No values found for log label "severity"`}, + {"logs query", append([]string{"logs", "query", "--query", "x"}, window...), + map[string]any{"logs_list": map[string]any{"logs": []any{}}}, "No logs found between"}, + {"metrics query", append([]string{"metrics", "query", "--query", "up"}, window...), + map[string]any{"metrics_list": map[string]any{"results": []any{map[string]any{"query_key": "query", "payload": []any{}}}}}, "No data"}, + } + for _, tt := range tests { + t.Run(tt.name+" text", func(t *testing.T) { + out, stderr := runCapturingStderr(t, tt.data, tt.args...) + assert.Empty(t, strings.TrimSpace(out)) + assert.Contains(t, stderr, tt.wantErr) + }) + t.Run(tt.name+" json", func(t *testing.T) { + out, stderr := runCapturingStderr(t, tt.data, append(tt.args, "-o", "json")...) + if tt.name == "metrics query" { + assert.JSONEq(t, `[{"query_key":"query","payload":[]}]`, out) + } else { + assert.JSONEq(t, `[]`, out) + } + assert.Contains(t, stderr, tt.wantErr) + }) + } +} + +func TestLogsQueryWarnsWhenLimitReached(t *testing.T) { + entry := map[string]any{"timestamp": "t", "severity": "info", "message": "m", "labels": map[string]any{}} + data := map[string]any{"logs_list": map[string]any{"logs": []any{entry, entry}}} + + out, stderr := runCapturingStderr(t, data, "logs", "query", "--query", "x", "--limit", "2", "--offset", "4", "-o", "json") + assert.Contains(t, stderr, "Returned 2 lines = --limit; results are probably cut off") + assert.Contains(t, stderr, "--offset 6") + var parsed []any + require.NoError(t, json.Unmarshal([]byte(out), &parsed), "stdout must stay valid JSON") + + _, stderr = runCapturingStderr(t, data, "logs", "query", "--query", "x", "--limit", "3") + assert.NotContains(t, stderr, "cut off") +} From 79d7086a194d0ef0bd3e75300e007adcfd8d31c2 Mon Sep 17 00:00:00 2001 From: shiv Date: Thu, 8 Oct 2026 21:23:01 +0530 Subject: [PATCH 6/6] fix(metrics,logs): don't report empty when -o json results don't decode - metrics query / logs query -o json: when the results don't fit the typed structs, the count is unknown, not zero; no longer print a false 'No data' / 'No logs found' - instant queries say 'at ' instead of a window Co-Authored-By: Claude Opus 5.5 --- cmd/logs_query.go | 9 +++++---- cmd/metrics_query.go | 16 +++++++++++----- cmd/workspace_contract_test.go | 19 +++++++++++++++++++ 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/cmd/logs_query.go b/cmd/logs_query.go index 452dd8f..49e2d45 100644 --- a/cmd/logs_query.go +++ b/cmd/logs_query.go @@ -97,11 +97,12 @@ var logsQueryCmd = &cobra.Command{ } // JSON output passes entries through, so for JSON only count them. jsonOutput := format.GetFormat().Get() == "json" - count := 0 + count := -1 // unknown until decoded if jsonOutput { var entries []json.RawMessage - _ = json.Unmarshal(raw, &entries) - count = len(entries) + if json.Unmarshal(raw, &entries) == nil { + count = len(entries) + } } else { if err := json.Unmarshal(raw, &logs); err != nil { return fmt.Errorf("failed to decode logs: %w", err) @@ -121,7 +122,7 @@ var logsQueryCmd = &cobra.Command{ if jsonOutput { return format.GetFormat().PrintRawJSON(raw) } - if count == 0 { + if count <= 0 { return nil } diff --git a/cmd/metrics_query.go b/cmd/metrics_query.go index 1f68fd6..b50884c 100644 --- a/cmd/metrics_query.go +++ b/cmd/metrics_query.go @@ -141,8 +141,9 @@ var metricsQueryCmd = &cobra.Command{ // decode; for JSON the decode is best-effort, for the warnings below. jsonOutput := format.GetFormat().Get() == "json" var results []MetricsResponse - if err := json.Unmarshal(raw, &results); err != nil && !jsonOutput { - return fmt.Errorf("failed to decode metrics results: %w", err) + decodeErr := json.Unmarshal(raw, &results) + if decodeErr != nil && !jsonOutput { + return fmt.Errorf("failed to decode metrics results: %w", decodeErr) } series, failed := 0, false for _, r := range results { @@ -155,11 +156,16 @@ var metricsQueryCmd = &cobra.Command{ } } - // JSON output is the backend's results, unchanged, so scripts can use it as is. - if series == 0 && !failed { - _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "No data: the query returned no series between %s and %s.\n", startTime.Format(time.RFC3339), endTime.Format(time.RFC3339)) + // Only when the results decoded: otherwise series is unknown, not zero. + if decodeErr == nil && series == 0 && !failed { + when := fmt.Sprintf("between %s and %s", startTime.Format(time.RFC3339), endTime.Format(time.RFC3339)) + if instant { + when = "at " + endTime.Format(time.RFC3339) + } + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "No data: the query returned no series %s.\n", when) } + // JSON output is the backend's results, unchanged, so scripts can use it as is. if jsonOutput { return format.GetFormat().PrintRawJSON(raw) } diff --git a/cmd/workspace_contract_test.go b/cmd/workspace_contract_test.go index 655f179..0fff2f3 100644 --- a/cmd/workspace_contract_test.go +++ b/cmd/workspace_contract_test.go @@ -287,6 +287,25 @@ func TestEmptyResultsExplainOnStderr(t *testing.T) { } } +func TestNoEmptyNoteWhenJSONDoesNotDecode(t *testing.T) { + // Results that don't fit the typed structs still pass through in -o json, + // and must not be reported as empty. + var data any + require.NoError(t, json.Unmarshal([]byte(`{"metrics_list":{"results":[{"query_key":"q","payload":[{"metric":{"le":0.5},"timestamps":[1],"values":[2]}]}]}}`), &data)) + _, stderr := runCapturingStderr(t, data, "metrics", "query", "--query", "up", "-o", "json") + assert.NotContains(t, stderr, "No data") + + _, stderr = runCapturingStderr(t, map[string]any{"logs_list": map[string]any{"logs": map[string]any{"unexpected": true}}}, + "logs", "query", "--query", "x", "-o", "json") + assert.NotContains(t, stderr, "No logs found") +} + +func TestMetricsQueryInstantEmptyNote(t *testing.T) { + _, stderr := runCapturingStderr(t, map[string]any{"metrics_list": map[string]any{"results": []any{}}}, + "metrics", "query", "--query", "up", "--instant", "--end-time", "2026-10-01T01:00:00Z") + assert.Contains(t, stderr, "No data: the query returned no series at 2026-10-01T01:00:00Z.") +} + func TestLogsQueryWarnsWhenLimitReached(t *testing.T) { entry := map[string]any{"timestamp": "t", "severity": "info", "message": "m", "labels": map[string]any{}} data := map[string]any{"logs_list": map[string]any{"logs": []any{entry, entry}}}