fix(logs): align CLI with log search APIs by swkeever · Pull Request #8 · Kong/volcano-cli

swkeever · 2026-06-12T21:00:57Z

Summary

Regenerate the embedded OpenAPI client for the hosting log API shape from https://github.com/Kong/volcano-hosting/pull/370.
Route function runtime logs through POST /projects/{id}/logs/search with resource_type=function, resource_ids, and opaque cursor pagination.
Update frontend and deployment log pagination from legacy next_token/next URLs to the cursor/next_cursor contract.
Refresh the embedded local-mode Compose asset with DynamoDB Local, Mailpit, and the log-event table environment needed by the hosting server.

Architecture

volcano functions logs --type runtime now reads the project log search API and renders LogSearchResponse events.
Frontend runtime logs and function/frontend deployment logs continue using their list endpoints, but now pass cursor and advance from next_cursor.
volcano start brings up DynamoDB Local and configures the server with DYNAMODB_ENDPOINT and LOG_EVENTS_HOT_PATH_TABLE_NAME so local mode can use the DynamoDB-backed hot log path.
The refreshed asset also includes the upstream local-mode Mailpit service used as the local SMTP catch-all.
The CLI-owned compose asset keeps server-owned first-party local secrets out of the embedded template while retaining the service dependencies required by local mode.

User-visible behavior

Function runtime log output no longer prints partial/region-error warnings because the new LogSearchResponse and ListLogsResponse shapes no longer return partial or region_errors.

Related PRs

Hosting API/backend: https://github.com/Kong/volcano-hosting/pull/370
Web UI: https://github.com/Kong/volcano-web/pull/98

Validation

docker compose -f internal/localmode/assets/docker-compose.template.yml config --quiet
go test ./...

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

tkkhq

Inline findings with severity markers ([P0]=blocker … [P3]=low, nit=optional). Build + targeted go test pass locally. Note: GitHub shows a stale 42-file diff — the real change is one commit / 15 files on top of current main; recommend a rebase.

Behavioral note (not anchorable — removed code): function runtime logs lose the partial/region-error warning. This is correct (the regenerated LogSearchResponse/ListLogsResponse no longer carry partial/region_errors), but worth a release-note line so users know why the signal disappeared. [P3]

tkkhq · 2026-06-19T01:52:59Z

      redis:
        condition: service_healthy
+      dynamodb:
+        condition: service_started


[P2] dynamodb gates the server on service_started, not service_healthy (postgres/redis use health). DynamoDB Local has no healthcheck here, so the server can boot before DynamoDB accepts connections. If the server doesn't retry table setup, a cold volcano start may flake. Either confirm the server retries, or add a simple healthcheck and switch to condition: service_healthy.

Confirmed against the hosting change: local-mode startup calls EnsureFunctionLogDynamoDBTable before initializing the log store, and that helper retries for up to 60s while DynamoDB Local comes up. Since the upstream local-mode template uses service_started and the server-side retry covers cold startup, I left the dependency shape unchanged.

tkkhq · 2026-06-19T01:52:59Z

      REDIS_URL: redis://redis:6379
+      DYNAMODB_ENDPOINT: http://dynamodb:8000
+      FUNCTION_INVOCATIONS_TABLE_NAME: volcano-local-function-invocations
+      LOG_EVENTS_HOT_PATH_TABLE_NAME: volcano-local-log-events


[P2] These table names are configured but nothing in the Compose asset creates the tables — DynamoDB Local boots empty. Fine if the server auto-creates them in LOCAL_MODE; please confirm. This is the classic case that works on a warm volume and fails on a fresh volcano-dynamodb volume.

Confirmed: in LOCAL_MODE, the hosting server auto-creates LOG_EVENTS_HOT_PATH_TABLE_NAME via EnsureFunctionLogDynamoDBTable and waits for the table to exist before continuing. I also removed the stale FUNCTION_INVOCATIONS_TABLE_NAME from this CLI asset, since hosting no longer reads that env var.

tkkhq · 2026-06-19T01:52:59Z

-		return ""
+// PrintSearchLogs renders paginated searched log events from fetch until the
+// response signals no more pages or the cursor cannot be advanced.
+func PrintSearchLogs(w io.Writer, fetch SearchLogsFetcher) error {


[P3] PrintSearchLogs is structurally identical to PrintLogs — only the event renderer and response type differ. Two copies of the pagination/stop logic will drift. Consider a small accessor interface + generics, or at minimum a // keep in sync with PrintLogs comment.

Leaving this as-is for now. The duplicated pagination loops are small and tied to distinct generated response types/renderers; adding a generic abstraction here would make the CLI output path less direct for limited reuse. Both list and search paths have command coverage, so I would revisit if another log pagination variant appears.

tkkhq · 2026-06-19T01:52:59Z

+  # auth_config points its smtp_host at "mailpit" so signup confirmations,
+  # password reset emails, etc. land in Mailpit's inbox at http://localhost:8025
+  # instead of being discarded.
+  mailpit:


nit: Mailpit is added here but the PR summary only mentions DynamoDB. Since it's inherited from the upstream source template, add a line to the description so reviewers know the new SMTP catch-all is intentional.

Updated the PR description to call out Mailpit explicitly as part of the refreshed upstream local-mode asset.

swkeever · 2026-06-19T02:06:26Z

Follow-up on the review body notes: the PR history has been rebuilt on current main, so the GitHub view is now down to the intended log/local-mode files. I also updated the PR description with the user-visible behavior note that function runtime logs no longer print partial/region-error warnings because the new LogSearchResponse/ListLogsResponse shapes do not include partial or region_errors.

tkkhq

Approving. All four review findings addressed:

[P2] dynamodb service_started race — covered by server-side EnsureFunctionLogDynamoDBTable retry (up to 60s); no change needed.
[P2] log table creation — server auto-creates LOG_EVENTS_HOT_PATH_TABLE_NAME in LOCAL_MODE; stale FUNCTION_INVOCATIONS_TABLE_NAME removed in eecdcfe.
nit Mailpit — PR description updated.
[P3] PrintLogs/PrintSearchLogs duplication — reasonable won't-fix.

Core log-search migration is clean, builds, and passes targeted tests. Optional follow-ups: rebase to collapse the stale 42-file diff, and a release-note line about the dropped partial-region warning.

swkeever marked this pull request as ready for review June 12, 2026 21:47

swkeever requested a review from a team as a code owner June 12, 2026 21:47

Copilot AI review requested due to automatic review settings June 12, 2026 21:47

Copilot AI reviewed Jun 12, 2026

Copilot started reviewing on behalf of swkeever June 12, 2026 21:47 View session

swkeever changed the title ~~fix(localmode): include DynamoDB in embedded compose~~ fix(localmode): include DynamoDB for function logs Jun 12, 2026

swkeever enabled auto-merge June 19, 2026 01:44

swkeever changed the title ~~fix(localmode): include DynamoDB for function logs~~ fix(logs): align CLI with log search APIs Jun 19, 2026

fix(logs): align CLI with log search APIs

012e86f

swkeever force-pushed the skeever/update-localmode-dynamodb-assets branch from e5825dd to 012e86f Compare June 19, 2026 01:49

tkkhq reviewed Jun 19, 2026

View reviewed changes

fix(localmode): remove stale invocation table env

eecdcfe

swkeever disabled auto-merge June 19, 2026 02:09

tkkhq approved these changes Jun 19, 2026

View reviewed changes

fix(logs): align activity client with POST contract

6770daa

swkeever added this pull request to the merge queue Jun 20, 2026

Merged via the queue into main with commit 9d0c365 Jun 20, 2026
8 checks passed

swkeever deleted the skeever/update-localmode-dynamodb-assets branch June 20, 2026 16:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(logs): align CLI with log search APIs#8

fix(logs): align CLI with log search APIs#8
swkeever merged 3 commits into
mainfrom
skeever/update-localmode-dynamodb-assets

swkeever commented Jun 12, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

tkkhq left a comment

Uh oh!

tkkhq Jun 19, 2026

Uh oh!

swkeever Jun 19, 2026

Uh oh!

tkkhq Jun 19, 2026

Uh oh!

swkeever Jun 19, 2026

Uh oh!

tkkhq Jun 19, 2026

Uh oh!

swkeever Jun 19, 2026

Uh oh!

tkkhq Jun 19, 2026

Uh oh!

swkeever Jun 19, 2026

Uh oh!

swkeever commented Jun 19, 2026

Uh oh!

tkkhq left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

swkeever commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Architecture

User-visible behavior

Related PRs

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

tkkhq left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swkeever commented Jun 19, 2026

Uh oh!

tkkhq left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

swkeever commented Jun 12, 2026 •

edited

Loading