feat(environment:logs): read logs from the Observability API - #189
Open
pjcdawkins wants to merge 9 commits into
Open
pjcdawkins wants to merge 9 commits into
pjcdawkins wants to merge 9 commits into
Conversation
The log command now reads from the Observability Pipeline logs API (as used by Console) instead of running `tail` over SSH. - Log types map to API log kinds: access, app, cron, deploy, platform. "error" filters by severity ERROR or higher. Without a type, all kinds except "platform" are shown, and no interactive prompt is needed. - New options: --service, --severity (minimum), --since, --until, and --format (text, raw, or json lines). - --lines queries time windows of increasing size (from the API's recommended default range, up to 7 days), because wide ranges can be rejected with "499 Too much data fetched". A 499 halves the window. - --tail polls every 2 seconds in ascending order using the cursor, 15 seconds behind the current time, as ingestion takes 5-12 seconds. - SSH is still used when the API is unavailable for the environment (entrypoint 404 or no logs_query link), when --worker, --instance or --task is given, or when the type is not a known log kind. Supporting changes: - Add an Observability service for the entrypoint and GET requests, and use it in the metrics commands. - Add SelectorConfig::$selectRemoteContainer and Selector::withRemoteContainer(), so the app is only selected (and prompted for) when falling back to SSH. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
For log lines with a JSON `context`, the text format now appends the fields as dimmed key=value pairs after the message. Nested objects are flattened with dotted keys, and lists and other non-string values are encoded as JSON. Top-level keys that duplicate the displayed time, severity and message (e.g. time, level, msg) are skipped, as are trace and span IDs. The json format still includes the full context. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Filter by the app from the selection, so an app from the project URL or the APPLICATION_NAME environment variable is respected, as it is over SSH. - When tailing, start each poll a second before the previous one ended (the cursor prevents duplicates), rather than at the last matching line. Quiet streams previously queried ever-growing windows. Each poll is also capped at 7 days, to catch up gradually after a suspend. - Poll every 5 seconds instead of 2, to reduce API load. The added latency is small next to the 15 second ingestion delay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![upsun-dispatch[bot]](https://avatars.githubusercontent.com/in/4153575?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 3 warnings · 🔵 1 minor point
🔍 Full review · 7 files reviewed
Verification
- In fetchRecent, a 499 retry only happens before any cursor is used (
$cursor !== nullrethrows), so a page can never be retried in the middle of pagination. - queryPage clears
_has_more_resultswhen the returned cursor is unchanged, so the do/while loops cannot spin on the same query. - The imports left in MetricsCommandBase (Request, BadResponseException, ApiResponseException, Environment) are still used, so no_unused_imports is satisfied.
- Selector only skips remote-container selection when
selectRemoteContaineris false, and withRemoteContainer builds a new Selection with the same config, project, environment and app name.
The new Go integration tests in integration-tests/environment_log_test.go cover the API path against a mocked query endpoint: default and larger --lines, the error type, JSON output, filters, and the app env var. They also cover the SSH fallback on a 404 and the rejection of API-only options there. Nothing tests the 499 window-halving, --tail polling, or entrypoint errors other than 404.
Review details
- Commit: ff24d5f
- Model: claude-opus-5-5
Review 1 of 10 for this pull request · View the full run
- Read a type ending in ".log" (e.g. "error.log") as a file over SSH, even when the logs API is available, so the error log file can still be read. "error" still filters by severity via the API. - Fall back to SSH, with a warning, if the observability entrypoint fails for reasons other than a 404 (e.g. 403, 5xx or a connection error). A 401 is still an error. - Keep rows without a cursor instead of deduplicating them into one. - With --tail, a --since time later than the tail delay now starts tailing instead of failing. Without --tail, the error no longer mentions --until unless it was given. - Test the 499 window halving, the entrypoint error fallback, file names over SSH, and invalid time ranges. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 2 warnings
🔁 Incremental · 2 files reviewed
Verification
log error.lognow makessshReason()return 'a log file name was given' before.logis stripped, so SSH reads /var/log/error.log.- A 401 from the entrypoint is still rethrown, while 403, 5xx and ConnectException now fall back to SSH with a warning.
- Rows with an empty cursor now get unique
no-cursor-Nkeys infetchRecent(), so they are no longer collapsed into one row. - Without
--tail, the$from >= $toerror names--untilonly when--untilwas actually passed.
Integration tests in integration-tests/environment_log_test.go now cover the 499 window halving, the two invalid-range messages, the 403→SSH fallback and error.log going over SSH. No test covers --tail with a recent or future --since, or a non-JSON entrypoint response.
Review 2 of 10 for this pull request · View the full run
…d entrypoint responses - With --tail, a future --since is now an error, as without --tail. A --since time within the tail delay skips the initial fetch and polls from that time, instead of from slightly earlier. - Fall back to SSH if the observability entrypoint response cannot be decoded (e.g. an HTML page from a proxy), as for other API errors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
upsun-dispatch
Bot
dismissed stale reviews from themself
September 28, 2026 00:51
Superseded: the latest Upsun Dispatch review no longer requests changes.
… available Fall back to SSH only if the observability entrypoint is not found (404) or has no logs_query link. Other errors, such as a 403, a 5xx or an invalid response, are now reported instead of being hidden behind the SSH fallback. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…terminals Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Add the api.log_protocol config option (and the <PREFIX>LOG_PROTOCOL environment variable). It defaults to "ssh", which keeps the previous behavior. "auto" uses the Observability API where it is available. API-only options give an error with the "ssh" protocol, saying how to enable the API. - Add --fields, to choose the fields shown in the text format, including "context.<key>" for fields of structured log lines. Colors match the default format. - Limit the "context" fields in the text format to 8 fields, 60 characters per value and 200 characters in total, noting how many were omitted. Fields chosen with --fields are not truncated. - List the valid formats in the error for an invalid --format. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
📋 PR Summary Adds an opt-in way for Changes
|
There was a problem hiding this comment.
Warning
Changes suggested — 🟡 1 warning
🔁 Incremental · 4 files reviewed
Verification
sshReason()checks for a.logsuffix beforeexecute()strips it, solog error.logstill reads /var/log/error.log over SSH.LOG_PROTOCOLis added to the override map in Config.php and defaults tosshin config-defaults.yaml.execute()rejects any value other thansshorauto.- With the
sshprotocol,--fieldsfalls under API_OPTIONS and gives a "cannot be used" error instead of being silently ignored. - In
formatContext, the length check only runs once$partsis non-empty, so at least one context field is always shown. After the first omitted field, every later field is omitted too, and they all go into the(+N more)count.
The diff extends the integration tests TestEnvironmentLogAPI and TestEnvironmentLogSSHFallback. They cover the protocol default, --fields, and the context limits, and they run in the integration-test job in ci.yml. No test covers context keys that are numeric.
Review 6 of 10 for this pull request · View the full run
PHP converts numeric array keys such as "404" to integers, which caused a TypeError under strict types when displaying context fields. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
upsun-dispatch
Bot
dismissed
their stale review
September 28, 2026 07:26
Superseded: the latest Upsun Dispatch review no longer requests changes.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
logcommand can now read from the Observability API (the same logs API used by Console) instead of runningtailover SSH.This is behind a flag: the new
api.log_protocolconfig option, or theUPSUN_CLI_LOG_PROTOCOLenvironment variable. It defaults tossh, which keeps the current behavior. Withauto, the API is used where it is available for the environment, and SSH otherwise.With the API:
access,app,cron,deploy,platform.errormeans severity ERROR or higher, across all kinds. Without a type, all kinds exceptplatformare shown, with no interactive prompt.--service/-s,--severity(minimum),--since,--until,--format(text,raw, orjsonlines), and--fields.key=valuepairs. Fields that are already displayed and trace/span IDs are skipped. The output is limited to 8 fields, 60 characters per value, and 200 characters in total.--fieldschooses the fields to display, e.g.--fields datetime,service,content,context.method,context.path,context.status.--linesqueries time windows of increasing size (15 minutes, growing to at most 7 days), since wide ranges can be rejected with499 Too much data fetched. A 499 halves the window and retries.--tailpolls every 5 seconds, 15 seconds behind the current time, because logs take 5-12 seconds to become queryable. The API has no streaming endpoint.logs_querylink), with--worker,--instanceor--task, or for a type that is not a known log kind or ends in.log(e.g.error.log). Other API errors are reported. API-only options give an error when SSH is used.Supporting changes:
Observabilityservice fetches the entrypoint and makes requests, and the metrics commands use it too.SelectorConfig::$selectRemoteContainerandSelector::withRemoteContainer()defer app selection, so the app prompt only appears when SSH is used.Note: the API's OpenAPI spec documents the sort parameter as
order, but the server readsorder_by.🤖 Generated with Claude Code