Skip to content

fix(ci): Always use submodule for npm cache key in setup-node.js action#8654

Merged
yurishkuro merged 1 commit into
jaegertracing:mainfrom
yurishkuro:fix/snapshot-ui-setup-node
May 28, 2026
Merged

fix(ci): Always use submodule for npm cache key in setup-node.js action#8654
yurishkuro merged 1 commit into
jaegertracing:mainfrom
yurishkuro:fix/snapshot-ui-setup-node

Conversation

@yurishkuro

Copy link
Copy Markdown
Member

Fixes the CI failure introduced by #8653.

Root Cause

In #8653, setup-node.js was updated to use ${JAEGER_UI_DIR}/package-lock.json as cache-dependency-path. On snapshot builds (push to main), JAEGER_UI_DIR is set by the checkout-ui-for-snapshot.sh step — but actions/setup-node evaluated the path at step startup before the git fetch+checkout in that script had finished writing the working tree to disk. The file wasn't there, producing:

##[error]Some specified paths were not resolved, unable to cache dependencies.

Fix

Revert setup-node.js to always use the submodule's jaeger-ui/package-lock.json for both node version detection and the npm cache key. The submodule is always present and its version matches jaeger-ui/main closely enough that the cache remains effective. The snapshot build does a full npm ci anyway, so cache key precision is not critical there.

🤖 Generated with Claude Code

Using JAEGER_UI_DIR for cache-dependency-path caused setup-node to fail
on snapshot builds because the snapshot checkout (which runs just before)
had not yet written package-lock.json to disk by the time setup-node
evaluated the path.

The submodule's package-lock.json is always present and its node version
matches main, so it is a reliable cache key for both PR and snapshot builds.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Copilot AI review requested due to automatic review settings May 28, 2026 20:14
@yurishkuro yurishkuro requested a review from a team as a code owner May 28, 2026 20:14

Copilot AI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@yurishkuro yurishkuro added the changelog:ci Change related to continuous integration / testing label May 28, 2026
@codecov

codecov Bot commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.59%. Comparing base (98cd20f) to head (2a0d3ad).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8654      +/-   ##
==========================================
- Coverage   96.62%   96.59%   -0.03%     
==========================================
  Files         334      334              
  Lines       17782    17782              
==========================================
- Hits        17181    17177       -4     
- Misses        452      455       +3     
- Partials      149      150       +1     
Flag Coverage Δ
badger_direct 8.86% <ø> (ø)
badger_e2e 1.03% <ø> (ø)
cassandra-4.x-direct-manual 14.42% <ø> (ø)
cassandra-4.x-e2e-auto 1.02% <ø> (ø)
cassandra-4.x-e2e-manual 1.02% <ø> (ø)
cassandra-5.x-direct-manual 14.42% <ø> (ø)
cassandra-5.x-e2e-auto 1.02% <ø> (ø)
cassandra-5.x-e2e-manual 1.02% <ø> (ø)
clickhouse-direct 8.89% <ø> (ø)
clickhouse-e2e 1.17% <ø> (ø)
elasticsearch-6.x-direct 16.72% <ø> (ø)
elasticsearch-7.x-direct 16.76% <ø> (ø)
elasticsearch-8.x-direct 16.90% <ø> (ø)
elasticsearch-8.x-e2e 1.03% <ø> (ø)
elasticsearch-9.x-e2e 1.03% <ø> (ø)
grpc_direct 7.83% <ø> (ø)
grpc_e2e 1.05% <ø> (ø)
kafka-3.x-v2 1.03% <ø> (-0.07%) ⬇️
memory_v2 1.05% <ø> (ø)
opensearch-1.x-direct 16.80% <ø> (ø)
opensearch-2.x-direct 16.80% <ø> (ø)
opensearch-2.x-e2e 1.03% <ø> (ø)
opensearch-3.x-e2e 1.03% <ø> (ø)
query 1.05% <ø> (ø)
tailsampling-processor 0.54% <ø> (ø)
unittests 94.92% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yurishkuro yurishkuro merged commit 6aec031 into jaegertracing:main May 28, 2026
71 of 72 checks passed
@yurishkuro yurishkuro deleted the fix/snapshot-ui-setup-node branch May 28, 2026 20:27
yurishkuro added a commit that referenced this pull request May 28, 2026
…orrect UI version (#8655)

Follow-up to #8653 and #8654.

## Problems

1. **`git fetch --unshallow`**: `rebuild-ui.sh` detects the shallow
clone created by `checkout-ui-for-snapshot.sh` and runs `git fetch
--unshallow` followed by `git fetch --all --tags`, downloading the
entire jaeger-ui history. This is only needed to check whether the HEAD
is exactly on a release tag — which can never be true for a snapshot
commit.

2. **Wrong UI version in About panel**: The About panel showed "Jaeger
UI v2.18.0" even when the snapshot was built from a commit 28 commits
ahead of that release, because `package.json` only gets updated on
official releases.

## Fixes

### Skip the tag-check block (`rebuild-ui.sh`)

Introduce `JAEGER_UI_SKIP_RELEASE_CHECK` env var. When set,
`rebuild-ui.sh` skips the unshallow + fetch + `git describe` block and
goes straight to `npm ci`. `checkout-ui-for-snapshot.sh` sets this
variable since snapshot commits are never release tags. The submodule/PR
path is unchanged.

### Patch `package.json` version with short SHA
(`checkout-ui-for-snapshot.sh`)

After checking out the snapshot, patch `packages/jaeger-ui/package.json`
in the ephemeral `$WORK_DIR` to append the short SHA to the version
string, e.g. `2.18.0` → `2.18.0-1c082b77`. The About panel then shows
"Jaeger UI v2.18.0-1c082b77", making it clear which exact UI commit is
embedded. The submodule and all other files are untouched.

## Variables exported by `checkout-ui-for-snapshot.sh`

| Variable | Purpose |
|---|---|
| `JAEGER_UI_DIR` | Path to the snapshot checkout; picked up by `make
build-ui` via `JAEGER_UI_DIR ?= jaeger-ui` |
| `JAEGER_UI_SKIP_RELEASE_CHECK` | Tells `rebuild-ui.sh` to skip the tag
lookup and go straight to npm |

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
vic-comm pushed a commit to vic-comm/jaeger that referenced this pull request Jun 11, 2026
…on (jaegertracing#8654)

Fixes the CI failure introduced by jaegertracing#8653.

## Root Cause

In jaegertracing#8653, `setup-node.js` was updated to use
`${JAEGER_UI_DIR}/package-lock.json` as `cache-dependency-path`. On
snapshot builds (push to main), `JAEGER_UI_DIR` is set by the
`checkout-ui-for-snapshot.sh` step — but `actions/setup-node` evaluated
the path at step startup before the git `fetch`+`checkout` in that
script had finished writing the working tree to disk. The file wasn't
there, producing:

```
##[error]Some specified paths were not resolved, unable to cache dependencies.
```

## Fix

Revert `setup-node.js` to always use the submodule's
`jaeger-ui/package-lock.json` for both node version detection and the
npm cache key. The submodule is always present and its version matches
`jaeger-ui/main` closely enough that the cache remains effective. The
snapshot build does a full `npm ci` anyway, so cache key precision is
not critical there.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Victor Chidera Obiezue <obiezuechidera@gmail.com>
vic-comm pushed a commit to vic-comm/jaeger that referenced this pull request Jun 11, 2026
…orrect UI version (jaegertracing#8655)

Follow-up to jaegertracing#8653 and jaegertracing#8654.

## Problems

1. **`git fetch --unshallow`**: `rebuild-ui.sh` detects the shallow
clone created by `checkout-ui-for-snapshot.sh` and runs `git fetch
--unshallow` followed by `git fetch --all --tags`, downloading the
entire jaeger-ui history. This is only needed to check whether the HEAD
is exactly on a release tag — which can never be true for a snapshot
commit.

2. **Wrong UI version in About panel**: The About panel showed "Jaeger
UI v2.18.0" even when the snapshot was built from a commit 28 commits
ahead of that release, because `package.json` only gets updated on
official releases.

## Fixes

### Skip the tag-check block (`rebuild-ui.sh`)

Introduce `JAEGER_UI_SKIP_RELEASE_CHECK` env var. When set,
`rebuild-ui.sh` skips the unshallow + fetch + `git describe` block and
goes straight to `npm ci`. `checkout-ui-for-snapshot.sh` sets this
variable since snapshot commits are never release tags. The submodule/PR
path is unchanged.

### Patch `package.json` version with short SHA
(`checkout-ui-for-snapshot.sh`)

After checking out the snapshot, patch `packages/jaeger-ui/package.json`
in the ephemeral `$WORK_DIR` to append the short SHA to the version
string, e.g. `2.18.0` → `2.18.0-1c082b77`. The About panel then shows
"Jaeger UI v2.18.0-1c082b77", making it clear which exact UI commit is
embedded. The submodule and all other files are untouched.

## Variables exported by `checkout-ui-for-snapshot.sh`

| Variable | Purpose |
|---|---|
| `JAEGER_UI_DIR` | Path to the snapshot checkout; picked up by `make
build-ui` via `JAEGER_UI_DIR ?= jaeger-ui` |
| `JAEGER_UI_SKIP_RELEASE_CHECK` | Tells `rebuild-ui.sh` to skip the tag
lookup and go straight to npm |

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Victor Chidera Obiezue <obiezuechidera@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:ci Change related to continuous integration / testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants