Skip to content
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.

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

feat(sync): networkHead callback metric #131

Conversation

renaynay
Copy link
Member

As per request

celestiaorg/celestia-node#2921 (comment)

While this isn't super high priority - it is true that totalSynced metric does not correlate to what the current networkHead is. There is no way currently to determine what the netHead of the node is via metric (local head does not suffice as it's only reporting store head).

@codecov-commenter
Copy link

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (28ff21c) 63.60% compared to head (b6fd46e) 63.41%.

Files Patch % Lines
sync/metrics.go 17.39% 19 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #131      +/-   ##
==========================================
- Coverage   63.60%   63.41%   -0.20%     
==========================================
  Files          39       39              
  Lines        3366     3389      +23     
==========================================
+ Hits         2141     2149       +8     
- Misses       1060     1075      +15     
  Partials      165      165              

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

@Wondertan
Copy link
Member

I think we will have to close this one in favour of another incoming syncer metric PR

@renaynay
Copy link
Member Author

is networkHead added as a metric there? @Wondertan

@Wondertan
Copy link
Member

There will be a subjective head(or just head) that will represent the current network head. On the code, we need to differentiate between subjective and network, but on metrics, the subjective will be the network head as it's applied almost instantly. So there will be head and the state syncing or not, which will be enough for the requested purpose

@Wondertan Wondertan closed this Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants