Skip to content

fix: align dashboard uptime with boot time - #13410

Merged
ssongliu merged 2 commits into
dev-v2from
fix/dashboard-uptime-consistency
Jul 29, 2026
Merged

fix: align dashboard uptime with boot time#13410
ssongliu merged 2 commits into
dev-v2from
fix/dashboard-uptime-consistency

Conversation

@ssongliu

@ssongliu ssongliu commented Jul 29, 2026

Copy link
Copy Markdown
Member

Refs #13386

Copilot AI review requested due to automatic review settings July 29, 2026 07:01

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.

🟡 Not ready to approve

It removes an apparently unrelated nginx module test suite and the new uptime calculation needs a safer guard/fallback to avoid pathological values when BootTime is missing/invalid.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

Adjusts the agent dashboard’s “running time/uptime” calculation to be derived from host boot time (instead of the cached host.Info().Uptime value), addressing incorrect runtime reporting in /api/v2/dashboard/current/all/all (refs #13386).

Changes:

  • Compute DashboardCurrent.Uptime from shotTime.Unix() - hostInfo.BootTime and reuse a single snapshot timestamp (shotTime) for the response.
  • Update RunningTime to be computed from the newly derived Uptime.
  • Remove agent/app/service/nginx_module_test.go (appears unrelated to the dashboard uptime fix).
File summaries
File Description
agent/app/service/dashboard.go Recomputes dashboard uptime from boot time and aligns snapshot timestamp usage.
agent/app/service/nginx_module_test.go Deletes the nginx module test suite file (unrelated to uptime fix).
Review details

Comments suppressed due to low confidence (1)

agent/app/service/nginx_module_test.go:1

  • The PR description/issue reference is about dashboard uptime, but this change deletes the entire nginx module test suite file. No equivalent tests appear elsewhere in the agent (e.g., the removed test names are not found), so this looks like an accidental deletion and also reduces coverage for nginx module behavior.
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

Comment thread agent/app/service/dashboard.go Outdated
Copilot AI review requested due to automatic review settings July 29, 2026 08:34
@ssongliu
ssongliu merged commit 357d778 into dev-v2 Jul 29, 2026
4 checks passed
@ssongliu
ssongliu deleted the fix/dashboard-uptime-consistency branch July 29, 2026 08:36

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.

🟡 Not ready to approve

It includes an unrelated deletion of the nginx module test suite without a visible replacement or rationale, which is a blocking scope/coverage regression for this fix PR.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Review details

Comments suppressed due to low confidence (1)

agent/app/service/dashboard.go:189

  • This PR removes agent/app/service/nginx_module_test.go (appears to be the only Go test file under agent/app/**), which is unrelated to the dashboard uptime fix and reduces coverage for nginx_module.go without a replacement. Please either restore/move those tests in this PR, or split the deletion into a separate PR with rationale.
	shotTime := time.Now()
	hostInfo, err := psutil.HOST.GetHostInfo(false)
	if err != nil {
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Low

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

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.

2 participants