Skip to content

feat(go-lib/version): add Handler and HandlerFor http.Handler - #270

Open
priyaselvaganesan wants to merge 6 commits into
mainfrom
feat/version-handler-go-lib
Open

feat(go-lib/version): add Handler and HandlerFor http.Handler#270
priyaselvaganesan wants to merge 6 commits into
mainfrom
feat/version-handler-go-lib

Conversation

@priyaselvaganesan

@priyaselvaganesan priyaselvaganesan commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

TL;DR

Adds Handler() and HandlerFor() to go-lib/pkg/version, a shared HTTP handler that serves build metadata as JSON at GET /info. Each service registers one line to expose the endpoint. Service name, version, and commit are injected via Bazel x_defs at build time.

Additional Details

Handler() reads the Service, Version, and GitHash package vars injected at link time via x_defs (set to a hardcoded service name, {STABLE_VERSION}, and {STABLE_GIT_COMMIT_FULL} from tools/workspace_status.sh when building with --stamp). HandlerFor() accepts explicit (service, ver, commit) values and falls back to Go runtime build info for commit when the value is empty, so go build locally returns a real commit SHA. Empty service or version fall back to "unknown". Any non-GET method returns 405 Method Not Allowed.

Response schema: {"service":"nvcf-my-service","version":"v1.2.3","commit":"16769d1988ff11c19071966db971de23b173f289"}

This is a prerequisite for the follow-on PR that wires nvcfversion.Handler() into each Go control plane service.

For the Reviewer

  • handler.go and handler_test.go are new files. BUILD.bazel adds them to the existing go_library and go_test targets.
  • version.go is updated to add a Service package var alongside the existing Version, GitHash, and Dirty vars.

For QA

go test ./pkg/version/... passes. No service changes in this PR.

Issues

Relates to #315

Checklist

  • I am familiar with the Contributing Guidelines.
  • I have signed off my commits for Developer Certificate of Origin (DCO) compliance.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Summary by CodeRabbit

  • New Features

    • Added a build-info HTTP endpoint that returns JSON including service, version, and commit.
    • Supports configuring service/version/commit values, using "unknown" when inputs are missing.
    • Automatically extracts the commit from embedded build metadata when no commit is provided, and returns application/json for successful GET requests.
    • Non-GET requests are rejected with 405 Method Not Allowed (including an Allow: GET header).
  • Tests

    • Added coverage to verify non-GET requests are rejected with 405 Method Not Allowed.

@priyaselvaganesan
priyaselvaganesan requested a review from a team as a code owner July 20, 2026 18:56
@github-actions

Copy link
Copy Markdown
Contributor

🛡️ CodeQL Analysis

🚨 Found 2 issue(s)

Severity Breakdown:

  • 🔴 Errors: 0
  • 🟡 Warnings: 0
  • 🔵 Notes: 0
📋 Top Issues

🔗 View full details in Security tab

🕐 Last updated: 2026-07-20 19:00:38 UTC | Commit: f779787

@priyaselvaganesan priyaselvaganesan self-assigned this Jul 20, 2026

@kristinapathak kristinapathak left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please provide a github issue rather than an internal JIRA issue.

@priyaselvaganesan

Copy link
Copy Markdown
Contributor Author

Please provide a github issue rather than an internal JIRA issue.

Done — created issue #315 and updated the Issues section here.

priyaselvaganesan added a commit that referenced this pull request Jul 21, 2026
Registers nvcfversion.Handler() at GET /info on the management server
(port 8082). Returns service name, semver, and commit SHA injected at
build time via Bazel x_defs.

Blocked on go-lib PR #270 merging. Once that lands, go.mod will be
updated to the published version that includes Handler().

Refs: NVCF-10975
Adds a shared HTTP handler to go-lib/pkg/version that serves build
metadata as JSON at GET /version. Version and commit are injected at
link time via Bazel x_defs (STABLE_VERSION, STABLE_GIT_COMMIT) when
built with --stamp. Falls back to Go runtime build info for commit
when GitHash is not injected, so local go build still returns a real
commit SHA.

Response schema: {"version": "v1.2.3", "commit": "abc1234"}

Refs: NVCF-10975
Adds Service string var injected via x_defs so each service identifies
itself in the response. Renames from GET /version to GET /info following
reviewer feedback that /info is more extensible for future fields.

Response schema: {"service":"nvcf-my-service","version":"v1.2.3","commit":"abc1234"}

Refs: NVCF-10975
@kristinapathak
kristinapathak force-pushed the feat/version-handler-go-lib branch from 50bc92b to da89e1d Compare July 27, 2026 20:44
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1a006370-fb3c-4690-b51f-73d0cb7e208c

📥 Commits

Reviewing files that changed from the base of the PR and between bda28f3 and 159c16c.

📒 Files selected for processing (2)
  • src/libraries/go/lib/pkg/version/BUILD.bazel
  • src/libraries/go/lib/pkg/version/handler.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/libraries/go/lib/pkg/version/handler.go

📝 Walkthrough

Walkthrough

Adds an HTTP handler that serves service, version, and commit metadata as JSON, resolves missing values from defaults or Go build information, exposes a CI-populated service variable, and adds Bazel wiring and tests.

Changes

Version metadata HTTP handler

Layer / File(s) Summary
Handler contract and response
src/libraries/go/lib/pkg/version/version.go, src/libraries/go/lib/pkg/version/handler.go
Adds the exported Service variable, Info JSON model, and GET-only Handler/HandlerFor HTTP handlers.
Metadata fallback resolution
src/libraries/go/lib/pkg/version/handler.go
Defaults empty metadata to "unknown" and reads vcs.revision from embedded Go build information when needed.
Handler tests and Bazel wiring
src/libraries/go/lib/pkg/version/BUILD.bazel, src/libraries/go/lib/pkg/version/handler_test.go
Adds handler sources to Bazel targets and tests configured, fallback, and non-GET responses.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: kristinapathak, sbaum1994, shelleyshen-0

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Handler
  participant HandlerFor
  participant BuildInfo
  Client->>Handler: Request metadata
  Handler->>HandlerFor: Pass Service, Version, GitHash
  HandlerFor->>BuildInfo: Resolve missing commit
  BuildInfo-->>HandlerFor: Return vcs.revision or empty
  HandlerFor-->>Client: JSON Info response
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits and accurately describes the main feature added to go-lib/version.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/version-handler-go-lib

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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/libraries/go/lib/pkg/version/handler_test.go`:
- Around line 105-110: Update TestHandler’s cleanup to capture the initial
Service, Version, and GitHash values before the test and restore those captured
values in t.Cleanup instead of assigning empty strings.

In `@src/libraries/go/lib/pkg/version/handler.go`:
- Around line 45-51: Add the established inbound-request observability baseline
inside the handler returned by HandlerFor: emit a structured request log,
create/propagate an OpenTelemetry span, and record RED metrics using bounded
labels derived from the request and service context. Reuse the repository’s
existing observability facilities and conventions, while preserving the current
JSON response behavior.
🪄 Autofix (Beta)

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: 3520689e-8064-454c-b70f-05cf931eac70

📥 Commits

Reviewing files that changed from the base of the PR and between d67b66b and da89e1d.

📒 Files selected for processing (4)
  • src/libraries/go/lib/pkg/version/BUILD.bazel
  • src/libraries/go/lib/pkg/version/handler.go
  • src/libraries/go/lib/pkg/version/handler_test.go
  • src/libraries/go/lib/pkg/version/version.go

Comment thread src/libraries/go/lib/pkg/version/handler_test.go
Comment thread src/libraries/go/lib/pkg/version/handler.go
Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
Signed-off-by: priyaselvaganesan <pselvaganesa@nvidia.com>
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.

3 participants