feat(blocklistener): Block height metrics for canonical chain vs target node

[onelapahead]

To help to detect if either the underlying node infrastructure or the block listener itself is unreliable / out-of-sync, we add two new gauges:

• the "target" block height - what we observe on an independent monitor go routine using eth_blockNumber
• the "canonical" block height - when we're building a canonical chain via block filters / newHeads subscriptions (when in full chain tracking mode), we want to know the height of that chain.

Using such metrics we can alert on several different scenarios:

1. is the target block height not incrementing - depending on the underlying consensus/EVM implementation, this might indicate the node is failing to sync the chain.
2. are the block listener canonical height and the target height out of sync within some toleration/expectation - this would either indicate the endpoint has a bad filter/subscription and is failing to notify the listener of new blocks, or the listener itself is somehow failing to track the chain otherwise.
3. if we are able to monitor the chain height itself via other means (a synced node we're monitoring, a block explorer, etc.), we can detect the difference of the target/canonical with the monitor to detect any lag.

[Chengxuan]

@onelapahead can you elaborate on the thinking behind here? The metrics-emitting logic does functional logic (updates the chain head), rather than purely capturing metrics.

Have you considered making the metrics-emitting logic do a no-op or emit an "unavailable/error" status metric when the main block height setting loop is in an unready state?

[onelapahead]

That is not the case nor intent - getCanonicalBlockHeight() acquires a RLock to read the listener's state but otherwise does not update anything as I understand it.

I've renamed refreshHighestBlockFromRPC() bc that was misleading, its named queryBlockHeightFromRPC and its just purely an eth_blokcNumber call, but it doesn't change any listener state.

[Chengxuan]

@onelapahead Thanks for the renaming.

So what was added in the PR is a separate goroutine that

1. runs on the same timer as the main listener goroutine.
2. doing a separate & extra eth_blockNumber call on the same timer

That approach has its merit, which is very easy to understand as a small patch, but doesn't feel like the most efficient way of capturing the diagnostic information. Also, lacks the next level of granularity to pin down the problem for diagnosis, because the metrics are not associated with any variable that's driving the functional logic.

Breaking down the situation you are trying to resolve, I'd give the following suggestions:

1. is the target block height not incrementing (node out of sync) - depending on the underlying consensus/EVM implementation, this might indicate the node is failing to sync the chain.

it's a node running problem, metrics and alert should be added in the JSON-RPC node.

2. are the block listener canonical height and the target height out of sync within some toleration/expectation - this would either indicate the endpoint has a bad filter/subscription and is failing to notify the listener of new blocks, or the listener itself is somehow failing to track the chain otherwise.

metrics should be added to track those failures. e.g. checkAndSetHighestBlock (and/orqueryBlockHeightFromRPC) should emit failure metrics and gauge metrics for new head, so when the head stays stale, it's obvious. It can be used as a trigger to escalate to the JSON-RPC node provider ops to check their metrics as well if the metric is from queryBlockHeightFromRPC.

4. if we are able to monitor the chain height itself via other means (a synced node we're monitoring, a block explorer, etc.), we can detect the difference of the target/canonical with the monitor to detect any lag.

this should be achieveable by adding metrics to checkAndSetHighestBlock and queryBlockHeightFromRPC

So I'd suggest replacing the new goroutine with metrics in checkAndSetHighestBlock and queryBlockHeightFromRPC. If you feel there is a need to do queryBlockHeightFromRPC in full-tracking mode, we could think about where to add it in the existing logic to improve the function logic. (Suggest a separate PR for this to make the current PR single-purposed)

[onelapahead]

it's a node running problem, metrics and alert should be added in the JSON-RPC node.

Be mindful - EVM connect user don't always own the JSON/RPC node or endpoint they are connecting to. So the target_chain_height metric is very much meant to be a sentinel for such use cases. We have observed users connecting to unstable/questionable infrastructure from time to time.

metrics should be added to track those failures

Great suggestion - I agree. I added failure counters for all the RPC methods we poll in the listener (deliberately not touching other packages).

However, we did observe a filter continually come back empty for an endpoint, but when TM restarted and saw a a much higher block height than what the canonical was tracking, and it went into catch up mode instead. So (although older I'm willing to bet) clients have the potential to "dead air" a filter potentially. Thats where have the target vs canonical can be very helpful to see in the metrics and logs as a sanity check.

I've incorporate your other feedback as well:

1. Got rid of the separate Goroutine, for full mode, we poll eth_blockNumber at the top of the listener loop. Light mode was always polling it. Whenever we poll it via queryBlockHeightFromRPC, we set target_chain_height. But currently, in full mode we don't do anything different yet if we notice a larger difference in that block height reported by the endpoint, and our listener's highestBlock. I think @peterbroadhurst will be making changes to help there instead potentially.
2. I moved canonical_chain_height emissions to checkAndSetHighestBlock and setHighestBlock funcs, to track what the subscription/filter is telling us is the highest block at any point.

[Chengxuan]

@onelapahead thanks for adding metrics. Agree with the intention of detecting out-of-sync / unreilable block listener head update. However, adding block head update logic in the metrics logic voilates single responsibilty practice, which make the logic more complex and bugs in the block listener logic harder to spot.

[peterbroadhurst]

Can I just check the metric emission hasn't been made specific to light mode

[onelapahead]

No - no matter what the metrics are emitted in all modes.

What you're seeing is a metric behavior difference between light mode and full mode - bc light mode does not track a canonical chain, the height it reports for the canonical chain height is the RPC's own block height.

[onelapahead]

I've refactored this slightly based on Cheng's feedback (detailed in #216 (comment) above), so hopefully its clearer and simpler now.