Skip to content

fix(logs,metrics,client): make incomplete or failed results visible to scripts; fewer flags - #123

Merged
blue4209211 merged 6 commits into
mainfrom
fix/script-safe-results
Oct 21, 2026
Merged

blue4209211 merged 6 commits into
mainfrom
fix/script-safe-results

Conversation

@blue4209211

@blue4209211 blue4209211 commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #122 from a failure-mode review of Nubi's workspace path (nudgebee/nudgebee-enterprise#40463). Scripts often drop stderr, so a cut-off, partial or failed result could read as the whole answer.

Changes

  • logs query -o json has one shape: always {logs, series, truncated, partial, total, total_relation, series_note, suggestion}. Keys the api-server didn't send are left out. A script that drops stderr still sees whether the result is complete. Breaking for scripts that parsed the bare log array; no release has shipped that array since v0.0.3.
  • Fewer flags: --query-type leaves --help for logs query and metrics query; use --param query_type=kql|ppl|dsl. It still works as a deprecated alias (stderr note), because about 1 in 12 nbctl calls Nubi made on dev used it. The workspace proxy already defaults Elasticsearch to DSL.
  • Flag errors list the valid flags: an unknown flag now prints the command's own flags (unknown flag: --metric / flags for nbctl metrics list-label-values: --account-id, --label). Set once on the root, so every command gets it; Nubi guessed --metric there 4 times on dev.
  • --limit: defaults to 1000 (llm-server's logs default) and must be at least 1. With no limit, or 0, the api-server would fall back to each provider's default (10 to 5,000 lines), and a cut-off result couldn't be detected. The warning now names the limit: Returned N lines = --limit N; results may be cut off.
  • partial: logs query now selects it and warns on stderr when the provider didn't finish.
  • metrics query: exits non-zero when a query in the result failed, after printing its output (text or JSON). A failure no longer looks like "no data".
  • 401:
    • prints the server's message when it explains the failure (e.g. an expired workspace execution token);
    • keeps the "create a new API token" advice for a bare Unauthorized, a one-word code such as the app gateway's not_authenticated, or an empty body.

Not changed

A DSL "size" can return more lines than --limit. That's Elasticsearch's own knob and stays as it is.

Testing

  • make build passes (lint, go test -race ./..., build).
  • New tests cover:
    • the single -o json object, and --query-type hidden from --help but still working as a deprecated alias;
    • completeness keys on the series object;
    • the partial warning;
    • the metrics exit code in JSON and text, plus the success case;
    • 401 messages for each body format, bare codes, and an empty body.
  • Live on dev:
    • Elasticsearch lines give truncated: true, total: 10000 gte, partial: false; counts give 116 series with an exact total; --param query_type=kql works for logs, and query_type=dsl for metrics;
    • Loki gives the same object shape;
    • a failed PromQL query exits 1 with the JSON printed, and a good one exits 0;
    • a bad key through the app gateway still gets the API-key hint.

🤖 Generated with Claude Code

…o scripts

From a failure-mode review of Nubi's workspace path: scripts often drop
stderr, so warnings there were easy to lose and a cut-off, partial or
failed result could read as the whole answer.

- logs query --envelope (-o json): always an object with logs, series and
  completeness keys (truncated, total, total_relation, partial,
  series_note, suggestion); the series object carries the same keys. The
  bare array stays the default for scripts that parse it.
- logs query selects partial and warns on stderr when it is set
- metrics query exits non-zero when a query in the result failed, after
  printing its output, so a failure is not read as "no data"
- 401: show the server's message when it says why (e.g. an expired
  workspace token) instead of advice to create an API token; keep that
  advice for a bare "Unauthorized" or a one-word code (not_authenticated)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces several enhancements to the CLI, including a new --envelope flag for logs queries to output completeness metadata (such as truncation and partial results) in JSON format, and updates metrics queries to exit with a non-zero status upon failure. Additionally, the HTTP client now extracts and displays detailed server-side error messages for 401 Unauthorized responses. Feedback suggests using strings.Fields in isBareUnauthorized to more robustly check if an error message consists of a single word.

Comment thread pkg/client/client.go Outdated
blue4209211 and others added 2 commits October 10, 2026 17:56
Review feedback: handles every whitespace character, not just space and tab.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e and --query-type

Fewer options for people and for Nubi to choose between:
- logs query -o json is always {logs, series, truncated, partial, total,
  total_relation, series_note, suggestion} (keys the api-server did not
  send are left out); --envelope is gone. Breaking for scripts that parsed
  the bare array; no release has shipped it since v0.0.3.
- --query-type is gone from logs query and metrics query; the workspace
  proxy defaults Elasticsearch to dsl, and the rare KQL/PPL case is
  --param query_type=kql|ppl
- help, README and tests updated

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@blue4209211 blue4209211 changed the title fix(logs,metrics,client): make incomplete or failed results visible to scripts fix(logs,metrics,client): make incomplete or failed results visible to scripts; fewer flags Oct 10, 2026
blue4209211 and others added 3 commits October 10, 2026 19:15
…in the cut-off warning

Limits live in the caller: with no limit (or 0) the api-server falls back
to each provider's own default (10 to 5,000 lines) and a cut-off result
could not be told from a complete one. --limit now defaults to 1000 to
match llm-server's logs default, values below 1 are rejected, and the
warning reads 'Returned N lines = --limit N; results may be cut off'.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…alid flags

- --query-type still works for logs query and metrics query, as a deprecated
  alias for --param query_type=<value>: hidden from --help, with a stderr note.
  About 1 in 12 nbctl calls Nubi made on dev used it; removing it outright
  would have turned each into a failed call.
- An unknown or bad flag now names the command's own flags, e.g.
  "unknown flag: --metric\nflags for nbctl metrics list-label-values:
  --account-id, --label (see ... --help)". Set once on the root, so every
  command, current and future, gets it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@blue4209211
blue4209211 merged commit 7ec3342 into main Oct 21, 2026
2 checks passed
@blue4209211
blue4209211 deleted the fix/script-safe-results branch October 21, 2026 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants