Skip to content

[management] fix utc difference on last seen status for a peer#5348

Open
crn4 wants to merge 1 commit intomainfrom
fix/peershandling
Open

[management] fix utc difference on last seen status for a peer#5348
crn4 wants to merge 1 commit intomainfrom
fix/peershandling

Conversation

@crn4
Copy link
Contributor

@crn4 crn4 commented Feb 16, 2026

Describe your changes

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

bug fix

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Bug Fixes
    • Improved timestamp synchronization in server request processing to ensure consistent UTC time reference across internal operations.

Copilot AI review requested due to automatic review settings February 16, 2026 16:35
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 16, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

Introduces syncStart as a UTC-normalized version of the request start time and propagates it through downstream function calls (accountManager.SyncAndMarkPeer, cancelPeerRoutinesWithoutLock, handleUpdates) in place of the original reqStart reference.

Changes

Cohort / File(s) Summary
Timestamp Synchronization
management/internals/shared/grpc/server.go
Introduces syncStart variable as the UTC version of request start time and replaces reqStart with syncStart in accountManager synchronization calls, peer routine cancellation paths, and update handling to ensure consistent timestamp references throughout the peer lifecycle.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • pascal-fischer

Poem

🐰 A timestamp's journey, once tangled in time,
Now synced up to UTC, all perfectly aligned,
Through sync and through cancel, through update and care,
The rabbit hops faster with clocks everywhere! ⏰✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description follows the template structure but lacks critical implementation details; the 'Describe your changes' section is empty despite being required. Fill in the 'Describe your changes' section with details explaining the bug, the root cause, and how the fix addresses it. Add the issue ticket number and link.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: fixing a UTC time difference issue affecting peer last-seen status.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/peershandling

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@sonarqubecloud
Copy link

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a UTC timezone inconsistency bug in peer "last seen" timestamp tracking. The issue occurred when the sync timestamp wasn't converted to UTC before being passed to downstream functions that perform timestamp comparisons with stored peer status data.

Changes:

  • Introduced syncStart variable by converting reqStart to UTC in the Sync function
  • Replaced reqStart with syncStart in all calls that pass timestamps for peer status tracking
  • Preserved reqStart for local timing measurements (metrics and logging)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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