Skip to content

Remove api.HostStats#945

Open
peterjan wants to merge 1 commit intomasterfrom
pj/remove-prometheus
Open

Remove api.HostStats#945
peterjan wants to merge 1 commit intomasterfrom
pj/remove-prometheus

Conversation

@peterjan
Copy link
Copy Markdown
Member

Closes #940

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes the legacy ?response=prometheus query parameter behavior from the admin JSON endpoints now that Prometheus metrics are consolidated under /prometheus/metrics, and simplifies host-stats Prometheus marshalling to avoid wrapper types.

Changes:

  • Replace writeResponse(..., resp) usage with direct jc.Encode(...) across admin endpoints (removing per-endpoint Prometheus formatting).
  • Remove the HostStats wrapper type and make HostStatsResponse directly a []hosts.HostStats, updating Prometheus marshalling accordingly.
  • Add a knope changeset documenting the user-facing change.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
api/admin/types.go Removes the HostStats wrapper and updates HostStatsResponse to use hosts.HostStats directly.
api/admin/prometheus.go Refactors host stats Prometheus metric generation to work directly with hosts.HostStats.
api/admin/admin.go Removes writeResponse and switches handlers to always return JSON via jc.Encode; updates /prometheus/metrics to wrap host stats correctly.
.changeset/remove_prometheus_response_format_from_stats_endpoints.md Adds release-note entry for the removed query parameter behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .changeset/remove_prometheus_response_format_from_stats_endpoints.md Outdated
Comment thread api/admin/admin.go
@peterjan peterjan marked this pull request as ready for review April 20, 2026 11:55
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

Copy link
Copy Markdown
Member

@ChrisSchinnerl ChrisSchinnerl left a comment

Choose a reason for hiding this comment

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

Looking good but before we merge this we should update the deploy repo to pull in the stats from the new route.

Comment thread .changeset/remove_prometheus_response_format_from_stats_endpoints.md Outdated
Comment thread api/admin/admin.go
Comment thread api/admin/admin.go
Copy link
Copy Markdown
Member

@n8mgr n8mgr left a comment

Choose a reason for hiding this comment

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

Why are you removing the response format from the other endpoints?

@peterjan peterjan force-pushed the pj/remove-prometheus branch from e019ec4 to 7cad772 Compare April 20, 2026 16:08
@peterjan peterjan changed the title Remove prometheus response format Remove api.HostStats Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

Remove prometheus arg and fix silliness

5 participants