Skip to content

fix(server): install the metrics recorder before startup reconciliation - #4524

Merged
pimlock merged 1 commit into
NVIDIA:mainfrom
EmilienM:fix/4523-startup-lock-metrics/EmilienM
Oct 16, 2026
Merged

pimlock merged 1 commit into
NVIDIA:mainfrom
EmilienM:fix/4523-startup-lock-metrics/EmilienM

Conversation

@EmilienM

@EmilienM EmilienM commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The endpoint-status startup sweep ran before the gateway installed its Prometheus recorder, so the mutation-lock waits and timeouts it recorded were dropped. The recorder is now installed before startup reconciliation, so a sweep that succeeds shows up in the lock metrics.

Related Issue

Closes #4523 (needs state:accepted for the issue acceptance check)
Part of #4307

Changes

  • run_server installs the recorder before startup reconciliation, still only when metrics_bind_address is set. The metrics listener binds where it did.
  • The startup lock-timeout test captures metrics, waits for the sweep's first timeout instead of a fixed sleep, and checks the sandbox timeout and wait samples.
  • The debug skill says which startup timeouts reach the metrics and how to read them.
  • Limits: the listener still binds after the sweep, so a new pod's first scrape already holds those samples. They show in the raw counter and _count values, not in rate() or increase(). A sweep that fails still exits before any listener binds, so its timeouts stay log-only.
  • Merge note: feat(config): deliver gateway configuration over the supervisor session #4320 inserts backfill_legacy_policy_history at the same spot. Keep both, with the recorder install first.

Testing

  • mise run rust:lint, rust:format:check, license:check and markdown:lint pass. cargo test -p openshell-server --features test-support passes, and the sweep tests passed 5 runs in a row.
  • The new assertions pin what the sweep records. The call order in run_server has no test harness, so no test here fails on main.
  • Checks appropriate to the affected code and behavior pass
  • Unit tests added/updated (if applicable)
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

The endpoint-status startup sweep ran before the gateway installed its
Prometheus recorder, so the lock waits, timeouts, errors and hold times
it recorded went to the no-op recorder and only its warnings reached
the log. The sweep retries lock timeouts for up to three lock waits
before startup fails, and a slow sweep that still succeeds is what
operators want to see in the lock metrics.

When metrics are enabled, the recorder is now installed before startup
reconciliation, and the metrics listener still binds with the other
listeners. A new pod's first scrape therefore already holds the sweep's
samples, so they show in the raw counter and histogram count values,
not in rate() or increase(). A sweep that fails still exits before any
listener binds, so its timeouts remain log-only.

The startup lock-timeout test now waits for the sweep's timeout counter
instead of a fixed sleep and checks its timeout and wait samples, and
the debug skill says which startup timeouts reach the metrics and how
to read them.

Part of NVIDIA#4307
Closes NVIDIA#4523

Signed-off-by: Emilien Macchi <emacchi@redhat.com>
@copy-pr-bot

copy-pr-bot Bot commented Oct 14, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@mrunalp

mrunalp commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 6a3ca1e

@mrunalp mrunalp added the test:e2e Requires end-to-end coverage label Oct 14, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied, but pull-request/4524 does not exist yet. A maintainer needs to comment /ok to test 6a3ca1eaee8646009a29e5dcf71eaf7e2bd7ebbb to mirror this PR. Once the mirror exists, re-apply the label or re-run Branch E2E Checks from the Actions tab.

@FrostGod

Copy link
Copy Markdown
Collaborator

LG to me

@FrostGod
FrostGod requested review from FrostGod, drew, elezar and sjenning and removed request for sjenning October 15, 2026 12:10
@FrostGod

Copy link
Copy Markdown
Collaborator

/ok to test 6a3ca1e

@copy-pr-bot

This comment was marked as outdated.

@pimlock

pimlock commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 6a3ca1e

@pimlock
pimlock added this pull request to the merge queue Oct 16, 2026
Merged via the queue into NVIDIA:main with commit b8526eb Oct 16, 2026
123 of 125 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(server): count startup-sweep lock waits in the lock metrics

4 participants