Skip to content

Add profiling command - #11

Merged
alexellis merged 2 commits into
masterfrom
add-profile-command
Aug 21, 2026
Merged

alexellis merged 2 commits into
masterfrom
add-profile-command

Conversation

@alexellis

Copy link
Copy Markdown
Member

Summary

  • add a profile command for recent profiling snapshots
  • support detailed snapshots by GitHub Actions job ID
  • provide table and JSON output for organisation and personal owners
  • give direct reauthentication guidance for rejected PATs

Testing

  • go test ./...
  • go build ./...
  • go run . profile openfaasltd --limit 3
  • go run . profile openfaasltd --id 96357108346

List recent profiling snapshots and inspect full snapshots by GitHub
Actions job ID. Support table and JSON output for organisations and
personal repository owners.

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
@reviewfn

This comment has been minimized.

Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
@reviewfn

reviewfn Bot commented Aug 20, 2026

Copy link
Copy Markdown

AI Pull Request Overview

Disclaimer: This review was generated by automated AI and may contain errors. Do not trust its outputs without human verification.

Summary

  • Adds a new profile OWNER command for listing recent profiling snapshots and viewing a detailed job snapshot.
  • Adds client methods for /api/v1/snapshots and /api/v1/snapshot.
  • Adds table and pretty-printed JSON output paths for profile data.
  • Updates root help and README language so OWNER includes personal accounts as well as organisations.
  • Adds focused tests for profile request construction and selected formatting helpers.
  • One detailed-output unit mismatch appears likely to misrender minimum available memory.

Approval rating (1-10)

7/10. The command is mostly cohesive, but the detailed memory field handling needs clarification or correction before merge.

Summary per file

Summary per file
File path Summary
README.md Documents personal owners and basic profile usage.
cmd/profile.go Adds profile command, response models, and table/JSON rendering.
cmd/profile_test.go Tests memory usage and detailed memory formatting helpers.
cmd/root.go Registers the profile command and updates owner help text.
pkg/client.go Adds profile list/detail API client methods.
pkg/client_profile_test.go Tests profile endpoint paths, query parameters, and response handling.

Overall Assessment

The PR is narrowly scoped and follows the existing CLI/client patterns. The main risk is in the new detailed profile model: minimum available memory is named and tagged as a GB field but rendered as raw bytes, which can produce misleading detailed output if the API returns the field implied by the JSON name. The tests currently construct the struct directly, so they do not catch this wire-format mismatch.

Detailed Review

Detailed Review

cmd/profile.go

Finding: detailed minimum memory can be rendered with the wrong unit

ProfileSnapshot.MinAvailableMemoryBytes is tagged as json:"min_memory_available_gb,omitempty", but printProfileDetails renders it through formatProfileBytes as if the decoded value is raw bytes. If the detailed endpoint returns the same min_memory_available_gb field name used by the list response, a value such as 9.92 GB will be displayed as 9.92B, making the detailed snapshot materially wrong for users comparing RAM capacity and pressure.

The direct struct-construction test in cmd/profile_test.go bypasses JSON decoding, so it does not prove the API field is decoded with the intended units.

If the detailed endpoint returns gigabytes, keep the field as GB and render it with a GB formatter. If it returns bytes, the JSON tag should match the actual wire field and the test should unmarshal a representative detailed response before rendering.

Relevant lines:

{"RAM minimum available", formatProfileBytes(snapshot.MinAvailableMemoryBytes)},
...
MinAvailableMemoryBytes *float64 `json:"min_memory_available_gb,omitempty"`

AI agent details.

Agent processing time: 1m23.268s
Environment preparation time: 3.154s
Total time from webhook: 1m28.91s

@alexellis
alexellis merged commit 0710c2e into master Aug 21, 2026
3 checks passed
@alexellis

Copy link
Copy Markdown
Member Author

Will follow-up on the finding.

@alexellis
alexellis deleted the add-profile-command branch August 21, 2026 09:01
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.

1 participant