feat: update metrics #367
feat: update metrics #367
Conversation
CodSpeed Performance ReportMerging #367 will not alter performanceComparing Summary
|
frisitano
left a comment
frisitano
left a comment
There was a problem hiding this comment.
This is great, thanks! I left one comment inline regarding the sequencer metric implementation.
| .as_mut() | ||
| .expect("signer must be present") | ||
| .sign_block(block.clone())?; | ||
| self.metric_handler.finish_block_building_recording(); |
There was a problem hiding this comment.
if we don't allow empty blocks then the finalize method below will return None. We should handle that case.
self
.sequencer
.as_mut()
.expect("sequencer must be present")
.finalize_payload_building(payload_id, &mut self.engine)There was a problem hiding this comment.
On second thought, maybe it was your intention to skip it such that we don't report metrics for empty blocks?
There was a problem hiding this comment.
Yes the idea was only to record the actual block building duration for blocks that get added to the chain. I think we could however also have a metric for block building regardless of whether they get added to the chain, and also for the duration between 2 consecutive block added to the chain.
There was a problem hiding this comment.
I think these could both be valuable to have
No description provided.