Skip to content

fix(tracing-versions): stamp service versions for tracing #383

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
borao wants to merge 1 commit into
base: main
Choose a base branch
from
+42 −7
Open

fix(tracing-versions): stamp service versions for tracing #383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/image-push-manual.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ jobs:
env:
SVC_PATH: ${{ github.event.inputs.service_path }}
TAG: ${{ steps.meta.outputs.tag }}
NVCF_VERSION: ${{ steps.meta.outputs.tag }}
REGISTRY: ${{ secrets.NCP_DEV_REGISTRY }}
run: |
set -euo pipefail
Expand All @@ -145,7 +146,7 @@ jobs:
mkdir -p ci-ghcr
printf 'load("@rules_oci//oci:defs.bzl", "oci_push")\n\noci_push(\n name = "push",\n image = "%s",\n repository = "%s",\n remote_tags = ["%s", "latest-dispatch"],\n)\n' \
"$tgt" "$dest" "$TAG" > ci-ghcr/BUILD.bazel
bazel run --remote_cache= --disk_cache="${BAZEL_DISK_CACHE}" //ci-ghcr:push
bazel run --stamp --remote_cache= --disk_cache="${BAZEL_DISK_CACHE}" //ci-ghcr:push
rm -rf ci-ghcr
done
echo "Done: pushed ${#indexes[@]} image(s) under ${REGISTRY}/ at tag ${TAG}"
23 changes: 20 additions & 3 deletions src/control-plane-services/function-autoscaler/tools/workspace_status.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,28 @@ if [ "$COMMIT" != "unknown" ] && [ -n "$(git status --porcelain 2>/dev/null)" ];
fi

# CI overrides for VERSION (from git tag or MR sha) and BUILD_USER.
# When unset, fall back to the values the legacy nvcf-cli Makefile computed.
# When unset, use only tags that belong to this service. `git describe
# --exact-match` is unsafe in the monorepo because a commit can also have
# stack/chart tags such as deploy/stacks/nvcf-compute-plane/v...
TAG_PREFIX="src/control-plane-services/function-autoscaler/v"
LEGACY_TAG_PREFIX="nvcf-function-autoscaler-v"

version_from_exact_tag() {
local prefix tag
for prefix in "$@"; do
tag=$(git tag --points-at HEAD --list "${prefix}*" 2>/dev/null | sort -V | tail -n1 || true)
if [ -n "${tag}" ]; then
printf '%s\n' "${tag#"${prefix}"}"
return 0
fi
done
return 1
Comment on lines +33 to +42

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fix cross-prefix version selection in both workspace scripts.

Both helpers return after the first prefix match, so they do not reliably select the highest matching service tag when canonical and legacy tags coexist.

  • src/control-plane-services/function-autoscaler/tools/workspace_status.sh#L33-L42: collect suffixes from both prefixes before semantic sorting, or explicitly enforce canonical-prefix precedence.
  • src/invocation-plane-services/http-invocation/tools/workspace_status.sh#L33-L42: apply the same fix and add regression coverage for both-prefix resolution.
📍 Affects 2 files
  • src/control-plane-services/function-autoscaler/tools/workspace_status.sh#L33-L42 (this comment)
  • src/invocation-plane-services/http-invocation/tools/workspace_status.sh#L33-L42
🤖 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/control-plane-services/function-autoscaler/tools/workspace_status.sh`
around lines 33 - 42, Update version_from_exact_tag in both
src/control-plane-services/function-autoscaler/tools/workspace_status.sh (lines
33-42) and
src/invocation-plane-services/http-invocation/tools/workspace_status.sh (lines
33-42) to evaluate matching tags across all prefixes before selecting the
highest semantic version, or explicitly enforce canonical-prefix precedence. Add
regression coverage for cases where canonical and legacy prefixes both match,
verifying identical correct resolution in both scripts.

}

if [ -n "${NVCF_VERSION:-}" ]; then
VERSION="${NVCF_VERSION}"
elif TAG=$(git describe --tags --exact-match HEAD 2>/dev/null); then
VERSION="${TAG}"
elif VERSION=$(version_from_exact_tag "${TAG_PREFIX}" "${LEGACY_TAG_PREFIX}"); then
:
else
VERSION="mr-${COMMIT}"
fi
Expand Down
23 changes: 20 additions & 3 deletions src/invocation-plane-services/http-invocation/tools/workspace_status.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,28 @@ if [ "$COMMIT" != "unknown" ] && [ -n "$(git status --porcelain 2>/dev/null)" ];
fi

# CI overrides for VERSION (from git tag or MR sha) and BUILD_USER.
# When unset, fall back to the values the legacy nvcf-cli Makefile computed.
# When unset, use only tags that belong to this service. `git describe
# --exact-match` is unsafe in the monorepo because a commit can also have
# stack/chart tags such as deploy/stacks/nvcf-compute-plane/v...
TAG_PREFIX="src/invocation-plane-services/http-invocation/v"
LEGACY_TAG_PREFIX="nvcf-invocation-service-v"

version_from_exact_tag() {
local prefix tag
for prefix in "$@"; do
tag=$(git tag --points-at HEAD --list "${prefix}*" 2>/dev/null | sort -V | tail -n1 || true)
if [ -n "${tag}" ]; then
printf '%s\n' "${tag#"${prefix}"}"
return 0
fi
done
return 1
}

if [ -n "${NVCF_VERSION:-}" ]; then
VERSION="${NVCF_VERSION}"
elif TAG=$(git describe --tags --exact-match HEAD 2>/dev/null); then
VERSION="${TAG}"
elif VERSION=$(version_from_exact_tag "${TAG_PREFIX}" "${LEGACY_TAG_PREFIX}"); then
:
Comment on lines +33 to +48

@coderabbitai coderabbitai Bot Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Add repository-native regression coverage for the version contract.

The stated validation covers shell syntax and diff formatting, but not canonical-only, legacy-only, both-prefix, no-tag fallback, or NVCF_VERSION propagation into the stamped Bazel invocation. Add these checks or explain why tests are not applicable; use Bazel validation for this subtree.

As per coding guidelines, code changes must include tests or explain why tests are not applicable, run the repository-native test runner, and use Bazel as the canonical validation path for src/invocation-plane-services/http-invocation/**/*.

🤖 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/http-invocation/tools/workspace_status.sh`
around lines 33 - 48, Add repository-native Bazel regression coverage for the
version-selection contract in workspace_status.sh, covering canonical-only tags,
legacy-only tags, both prefixes, no-tag fallback, and NVCF_VERSION propagation
into the stamped Bazel invocation. Place the checks in the appropriate subtree
test target and run the repository-native Bazel validation for
src/invocation-plane-services/http-invocation/**/*.

Source: Coding guidelines

else
VERSION="mr-${COMMIT}"
fi
Expand Down
Loading