ethstats: report processing time #33231
Conversation
core/events.go
Outdated
|
|
||
| type ChainHeadEvent struct { | ||
| Header *types.Header | ||
| Time uint64 |
There was a problem hiding this comment.
Should be renamed to ProcessingTime or something, and the type should be time.Duration.
|
Overall, I like the idea, but it'd be good to implement OpenTelemetry tracing in the engine API as well. |
|
cc @Savid |
| defer func() { | ||
| if lastCanon != nil && bc.CurrentBlock().Hash() == lastCanon.Hash() { | ||
| bc.chainHeadFeed.Send(ChainHeadEvent{Header: lastCanon.Header()}) | ||
| bc.chainHeadFeed.Send(ChainHeadEvent{Header: lastCanon.Header(), ProcessingTime: mclock.Now().Sub(stats.startTime)}) |
There was a problem hiding this comment.
I tested locally the other day and actually needed to add it to this one SetCanonical (which already has a start time too)
I'm wondering if we actually make a new ethstats event focused around engine_newPayloadVX calls and do the timing there?
The issue with existing block ethstats event is chainHeadFeed can be fired from 5+ different locations which would all require timing (which i think won't make sense in some cases) and the ethstats report also sends current head block which doesn't have timing value.
This new event block_new_payload (?) would only fire when valid block comes over the engine api. thoughts?
There was a problem hiding this comment.
i guess it's also fine if we accept block event sometimes returns no processingTime and just handle that on the ethstats server side
3 participants
Implements https://notes.ethereum.org/@savid/block-observability
Draft for now, looking for feedback on it