interface bss-core metrics to allow for driver-specific metrics extensions#2235
Conversation
The core metrics can still reside within the bss-core package, but now drivers can extend the base object with additional metrics.
🦋 Changeset detectedLatest commit: 87359fd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
cfromknecht
force-pushed
the
bsscore-iface-metrics
branch
from
b59734c to
a3baeb8
Compare
The BatchPruneCount metric is not used by the proposer, so this is moved into he sequencer's extended metrics. Also adds additional tooling for extending metrics.
cfromknecht
force-pushed
the
bsscore-iface-metrics
branch
from
a3baeb8 to
87359fd
Compare
|
|
||
| import "github.com/prometheus/client_golang/prometheus" | ||
|
|
||
| type Metrics interface { |
There was a problem hiding this comment.
Some of these don't make much sense for the teleportr service, I think its going to be hard to define a perfectly abstract interface that could be used for metrics across all the services
| // submission. | ||
| NumElementsPerBatch() prometheus.Summary | ||
|
|
||
| // SubmissionTimestamp tracks the time at which each batch was confirmed. |
There was a problem hiding this comment.
I think batch is still accurate? We do have support for batched withdrawals. IMO "batch" is still correct, even in the degenerate case of 1 element
| SubmissionGasUsedWei() prometheus.Gauge | ||
|
|
||
| // BatchsSubmitted tracks the total number of successful batch submissions. | ||
| BatchesSubmitted() prometheus.Counter |
There was a problem hiding this comment.
Should this be transactions sent? and failed transactions?
Codecov Report
@@ Coverage Diff @@
## develop #2235 +/- ##
========================================
Coverage 73.04% 73.04%
========================================
Files 86 86
Lines 2846 2846
Branches 486 486
========================================
Hits 2079 2079
Misses 767 767
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
|
Generally looks good to me except for the need to decide on whether or not the |
tynes
left a comment
tynes
left a comment
There was a problem hiding this comment.
Looks good to me, ignore the comments re: generalizing it too much
Fixes #2229 --------- Co-authored-by: refcell <abigger87@gmail.com>
4 participants
Description
Modifies the bss-core
Serviceto operate on ametrics.Metricsinterface rather than a concrete struct.The interface is now implemented by
metrics.Basewhich can embedded in driver-specific metrics extensions.Some extra functionality is added to make this process simpler, e.g exposing the
SubsystemName()to ensure the extensions all have the same metrics prefix. To demonstrate the behavior, theBatchPruneCountmetrics is moved into an extended metrics object specific to the sequencer, since the proposer does not have this concept.Metadata