feat(llm-api-gateway): expose GET /info endpoint - #710
Conversation
Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe gateway adds a version-backed ChangesGateway info endpoint
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Echo
participant golibversion
Client->>Echo: Request /info
Echo->>golibversion: Invoke Handler()
golibversion-->>Echo: Return version metadata response
Echo-->>Client: Return HTTP response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/invocation-plane-services/llm-api-gateway/go.mod`:
- Around line 24-33: Update the stable OpenTelemetry exporter dependencies in
go.mod from v1.42.0 to v1.44.0, including the OTLP metric, OTLP trace HTTP/gRPC,
stdout trace, and indirect otlptrace modules. Then run go mod tidy to
synchronize go.mod and go.sum.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e8a760dd-5b51-4f2e-b40f-ebb66e6c9d49
⛔ Files ignored due to path filters (1)
src/invocation-plane-services/llm-api-gateway/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
src/invocation-plane-services/llm-api-gateway/api/BUILD.bazelsrc/invocation-plane-services/llm-api-gateway/api/info_test.gosrc/invocation-plane-services/llm-api-gateway/api/routes.gosrc/invocation-plane-services/llm-api-gateway/cmd/llm-api-gateway/BUILD.bazelsrc/invocation-plane-services/llm-api-gateway/go.mod
…assertions Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/invocation-plane-services/llm-api-gateway/api/info_test.go (2)
81-88: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winVerify coverage for every method registered by
e.Any.The table omits
http.MethodConnectandhttp.MethodTrace. If Echo registers those methods for/info, add them to verify the stated all-non-GET contract. Otherwise, document the intentional scope or register only the supported methods.🤖 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/llm-api-gateway/api/info_test.go` around lines 81 - 88, Add http.MethodConnect and http.MethodTrace to the method table used by the `/info` coverage test so it verifies every method registered through e.Any and preserves the all-non-GET contract; if those methods are intentionally unsupported, restrict the route registration instead and document that scope.
41-43: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the unstamped metadata path.
This test only assigns non-empty values. Add a separate test with empty metadata and assert the documented
"unknown"values forservice,version, andcommit. Verify the fallback literals ingolibversion.Handler()before hard-coding the assertions.🤖 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/llm-api-gateway/api/info_test.go` around lines 41 - 43, Extend the tests around golibversion.Handler() with a separate case that clears Service, Version, and GitHash, then assert the handler returns the documented “unknown” fallback for service, version, and commit. Confirm the fallback literals in Handler() before hard-coding the expected values, while preserving the existing stamped-metadata test.
🤖 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.
Inline comments:
In `@src/invocation-plane-services/llm-api-gateway/api/info_test.go`:
- Around line 44-48: Update the test cleanup around golibversion.Service,
golibversion.Version, and golibversion.GitHash to capture their original values
before modification, then restore those saved values in t.Cleanup instead of
assigning empty strings.
---
Nitpick comments:
In `@src/invocation-plane-services/llm-api-gateway/api/info_test.go`:
- Around line 81-88: Add http.MethodConnect and http.MethodTrace to the method
table used by the `/info` coverage test so it verifies every method registered
through e.Any and preserves the all-non-GET contract; if those methods are
intentionally unsupported, restrict the route registration instead and document
that scope.
- Around line 41-43: Extend the tests around golibversion.Handler() with a
separate case that clears Service, Version, and GitHash, then assert the handler
returns the documented “unknown” fallback for service, version, and commit.
Confirm the fallback literals in Handler() before hard-coding the expected
values, while preserving the existing stamped-metadata test.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ef35f50f-231d-4c6c-9378-45867aa73445
📒 Files selected for processing (2)
src/invocation-plane-services/llm-api-gateway/api/BUILD.bazelsrc/invocation-plane-services/llm-api-gateway/api/info_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- src/invocation-plane-services/llm-api-gateway/api/BUILD.bazel
Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
…erage Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/invocation-plane-services/llm-api-gateway/api/info_test.go`:
- Around line 78-99: Update TestInfoEndpoint_GET_UnstampedFallback to save the
version metadata globals, clear them before calling newInfoEngine(), and restore
them via t.Cleanup. Strengthen the response assertions so service and version
equal "unknown", while keeping commit validation consistent with the handler’s
build-info fallback.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 12adb79e-3eb6-45f6-a614-119a7b63fe5e
📒 Files selected for processing (1)
src/invocation-plane-services/llm-api-gateway/api/info_test.go
Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
c176d87 to
2254e75
Compare
…for non-GET methods Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
2254e75 to
25c5971
Compare
|
🎉 This PR is included in version nvcf-llm-api-gateway-v0.12.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
TL;DR
Adds
GET /infoto the llm-api-gateway echo router, serving service name, version, and commit SHA via the shared go-lib handler. The endpoint is registered viaecho.WrapHandler(golibversion.Handler())and runs through echo's existing middleware stack./infois on the same port (8080) as the proxied traffic, so it is externally reachable via the HTTPRoute atllm.<domain>/info.Additional Details
Service,Version, andGitHashx_defs are injected at build time (--stamp) on thecmd/llm-api-gatewaybinary.GitHashuses{STABLE_GIT_COMMIT_FULL}socommitis the full 40-char SHA. Non-GET returns 405 (Allow: GET), covering all methods registered bye.Any.go.modpins the merged go-lib (feat(go-lib/version): add Handler and HandlerFor http.Handler #270). Go MVS pulls otel core to v1.44, grpc to v1.79.3, and aligns OTLP exporters from v1.42 to v1.44.Testing
Built with Bazel, imported into ncp-local k3d, deployed via helmfile, and verified via port-forward:
Unit tests pass under both
go testandbazel test, covering stamped values, unstamped fallback to"unknown", and 405 on all non-GET methods (HEAD, POST, PUT, PATCH, DELETE, OPTIONS, CONNECT, TRACE).References
GET /infoendpoint #275 (helm-reval), feat(grpc-proxy): expose GET /info endpoint #610 (grpc-proxy)Relates to #315