search

celestiaorg / go-header

Go library with all the services needed to request, sync and store blockchain headers.
Apache License 2.0
19 stars 18 forks source link

feat(sync): networkHead callback metric #131

Closed renaynay closed 10 months ago

renaynay commented 10 months ago

As per request

https://github.com/celestiaorg/celestia-node/issues/2921#issuecomment-1808209246

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 commented 10 months ago

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 :warning:
Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Wondertan commented 10 months ago

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

renaynay commented 10 months ago

is networkHead added as a metric there? @Wondertan

Wondertan commented 10 months ago

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