feat(vanity-gateway): expose GET /info endpoint - #709
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe vanity gateway embeds service, version, and Git commit metadata and exposes it through a telemetry-wrapped ChangesGateway version information
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant GatewayRouter
participant TelemetryMiddleware
participant GolibversionHandler
Client->>GatewayRouter: GET /info
GatewayRouter->>TelemetryMiddleware: wrap request
TelemetryMiddleware->>GolibversionHandler: serve version metadata
GolibversionHandler-->>Client: JSON service, version, and commit
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/invocation-plane-services/vanity-gateway/gateway/info_test.go (1)
50-68: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winStrengthen the
/infometadata assertion.The test checks only that the fields exist and are non-empty. It will pass if
service,version, andcommitare swapped or incorrect. Use deterministic test metadata and compare the complete response map. Restore package-level test state witht.Cleanup.The shared handler test in
src/libraries/go/lib/pkg/version/handler_test.go(Lines 105-129) demonstrates exact-value assertions. Keep a separate stamped Bazel build check for thex_defsvalues.As per coding guidelines, code changes must include tests, and the PR objective requires the complete service metadata contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/invocation-plane-services/vanity-gateway/gateway/info_test.go` around lines 50 - 68, Strengthen TestBuildChiMux_Info by setting deterministic package-level metadata before serving the request, registering t.Cleanup to restore the original state, and asserting the complete decoded response map with exact service, version, and commit values. Follow the exact-value pattern from the shared handler test while preserving a separate stamped Bazel check for x_defs-provided values.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/invocation-plane-services/vanity-gateway/gateway/info_test.go`:
- Around line 50-68: Strengthen TestBuildChiMux_Info by setting deterministic
package-level metadata before serving the request, registering t.Cleanup to
restore the original state, and asserting the complete decoded response map with
exact service, version, and commit values. Follow the exact-value pattern from
the shared handler test while preserving a separate stamped Bazel check for
x_defs-provided values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5cb74730-24f6-4184-9bed-2a32f38a6145
⛔ Files ignored due to path filters (1)
src/invocation-plane-services/vanity-gateway/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
src/invocation-plane-services/vanity-gateway/BUILD.bazelsrc/invocation-plane-services/vanity-gateway/gateway/BUILD.bazelsrc/invocation-plane-services/vanity-gateway/gateway/h2.gosrc/invocation-plane-services/vanity-gateway/gateway/info_test.gosrc/invocation-plane-services/vanity-gateway/go.mod
Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
…ssertions Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
fdb8cff to
73bb0b4
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
🎉 This PR is included in version nvcf-ai-api-gateway-service-v1.32.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
3 participants
TL;DR
Adds
GET /infoto the vanity-gateway chi router (port 10081), serving service name, version, and commit SHA via the shared go-lib handler. The endpoint runs inside the existing serverTelemetry middleware group so it is metered, traced, and logged like all proxied traffic.Additional Details
golibversion.Handler()atGET /infoviar.With(serverTelemetry).Get(...). Non-GET returns 405 (Allow: GET).Service,Version, andGitHashx_defs are injected at build time (--stamp).GitHashuses{STABLE_GIT_COMMIT_FULL}socommitis the full 40-char SHA.go.modpins the merged go-lib (feat(go-lib/version): add Handler and HandlerFor http.Handler #270). Go MVS bumps otel core to v1.44 and grpc to v1.79.3 (both required by go-lib)./infois on the same port as proxied traffic (10081), so it is externally reachable when the addon is enabled.Testing
Built with Bazel, imported into ncp-local k3d, enabled the vanityGateway addon via helmfile, and verified via port-forward:
References
GET /infoendpoint #275 (helm-reval), feat(grpc-proxy): expose GET /info endpoint #610 (grpc-proxy)Relates to #315
Summary by CodeRabbit
New Features
GET /infoendpoint that reports the service name, version, and commit identifier.Bug Fixes
GETrequests and return appropriate method-not-allowed responses for other methods.Tests