Add Metric to Track Greatest Epoch Final View

[danuio]

Resolves https://github.com/dapperlabs/flow-go/issues/5373.

[jordanschalm]

One note to use the existing metrics field, otherwise looks great 👍

[franklywatson]

Hi Danu. Would we not want to have unit tests for these changes?

[jordanschalm]

Hey Danu, you can include tests for opening the state here and for bumping the view upon committing an epoch in the epoch flow test by using the mock metrics object rather than the noop.

[vishalchangrani]

lgtm - just a reminder, you will have to Greenlist the metric as well by following this: https://www.notion.so/dapperlabs/Prometheus-metric-greenlisting-da74d2352b8f440185b1113270ba8bc5

I am also doing it for a metric that I have added - https://github.com/dapperlabs/dapper-flow-hosting/pull/174

[AlexHentschel]

Added some comments. I would like to flag that I think the metric will not have a value once we restart the consensus nodes (we sometimes do this during mainnet fires):

• the metric is only set during bootstrapping or once an EpochCommit service event is encountered. The latter could happen only once during an Epoch.
• So if we restart the consensus node, the value for the metric might not be set anymore for the remainder of the epoch, which kind of defeats its purpose.

The suggestion I made will partially mitigate this issue, as the metric is then updated on every finalized block. However, we might also consider setting the metric value when recovering from a node crash, i.e. in this method:

flow-go/state/protocol/badger/state.go

Line 398 in 861c833

func OpenState(

[AlexHentschel]

thanks for the revisions. Looks good. Just a couple minor comments.

Another suggested I wanted to make is:

• shall we also call

  flow-go/state/protocol/badger/state.go

  Line 490 in 9e2a2b6

  func updateCommittedEpochFinalView(state *State) error {

  at the end of Bootstrap , e.g. here:

  flow-go/state/protocol/badger/state.go

  Line 118 in 9e2a2b6

  return state, nil

  (not sure if that makes a difference). But then we set the metric each time we construct a State instance.

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (master@746b50c). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #698   +/-   ##
=========================================
  Coverage          ?   56.43%           
=========================================
  Files             ?      423           
  Lines             ?    24816           
  Branches          ?        0           
=========================================
  Hits              ?    14006           
  Misses            ?     8915           
  Partials          ?     1895           

Flag	Coverage Δ
unittests	56.43% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 746b50c...ccc7b1c. Read the comment docs.

[jordanschalm]

Greenlist metric in https://github.com/dapperlabs/dapper-flow-hosting/pull/198