Skip to content

fix: include custom metrics in cog predict --json output#2897

Merged
markphelps merged 2 commits intomainfrom
fix/custom-metrics-cli-response
Apr 1, 2026
Merged

fix: include custom metrics in cog predict --json output#2897
markphelps merged 2 commits intomainfrom
fix/custom-metrics-cli-response

Conversation

@markphelps
Copy link
Copy Markdown
Contributor

The Go Response struct was missing a Metrics field, causing json.Decoder to silently discard custom metrics from the coglet HTTP response. Add the field and an integration test that exercises the full CLI path (record_metric -> coglet -> Go CLI -> JSON output).

The Go Response struct was missing a Metrics field, causing
json.Decoder to silently discard custom metrics from the coglet
HTTP response. Add the field and an integration test that exercises
the full CLI path (record_metric -> coglet -> Go CLI -> JSON output).
@markphelps markphelps requested a review from a team as a code owner March 31, 2026 19:59
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 31, 2026

Now I understand the full picture. The PR adds:

  1. Metrics field to Response struct - This fixes the bug where custom metrics were being silently dropped during JSON decoding
  2. Bonus improvements to GetSchema() - Better context handling and fixes a resource leak by adding defer resp.Body.Close()
  3. Integration test - Verifies the end-to-end flow works correctly

All changes look correct:

  • The omitempty tag maintains backward compatibility
  • map[string]any is appropriate for arbitrary metrics
  • The test exercises the full path from record_metric() → coglet → Go CLI → JSON output
  • Resource leak in GetSchema() is fixed

LGTM

github run

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 31, 2026

LGTM

github run

@markphelps markphelps added this pull request to the merge queue Apr 1, 2026
Merged via the queue into main with commit f3eaa07 Apr 1, 2026
35 checks passed
@markphelps markphelps deleted the fix/custom-metrics-cli-response branch April 1, 2026 13:17
markphelps added a commit that referenced this pull request Apr 8, 2026
…icate/cog into mphelps/push-phase-progress

* 'mphelps/push-phase-progress' of https://github.com/replicate/cog: (95 commits)
  feat: add metric name validation (#2911)
  Rename `cog run` to `cog exec` (#2916)
  chore(deps): bump github.com/google/go-containerregistry (#2884)
  fix: replace removed libgl1-mesa-glx in tensorflow integration test (#2914)
  ci: enforce stub freshness in CI, fix existing stub drift (#2912)
  feat: add schema-compare command to test harness (#2891)
  chore(deps): bump uuid from 1.22.0 to 1.23.0 in /crates (#2887)
  chore(deps): bump github.com/hashicorp/go-version from 1.7.0 to 1.9.0 (#2909)
  chore(deps): bump insta from 1.46.3 to 1.47.2 in /crates (#2908)
  fix: support list[X] | None inputs + integration tests for PEP 604 union File/Path coercion (#2882)
  ci: exclude Dependabot PRs from auto-code review (#2910)
  chore(deps): bump actions/checkout from 4 to 6 (#2904)
  chore(deps): bump github.com/testcontainers/testcontainers-go/modules/registry (#2886)
  fix: metrics bugs in coglet prediction server (#2896)
  Bump version to 0.17.2 (#2903)
  fix(coglet): propagate metric scope to async event loop thread (#2902)
  chore: remove unnecessary nolint directive in test (#2803)
  feat(coglet): add Sentry error reporting for infrastructure errors (#2865)
  fix: homebrew cask postflight xattr references wrong binary name (#2899)
  fix: include custom metrics in cog predict --json output (#2897)
  ...
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