Enable customization of metrics calculation#26
Conversation
| @@ -0,0 +1 @@ | |||
| .vscode No newline at end of file | |||
|
This is a very large change and isn't obviously correct (it touches a lot of complex parallel code). I'll try to get to it eventually, it'll likely take a while. |
| return m.Snapshot().String() | ||
| } | ||
|
|
||
| func (m *Meter) Update(tdiff time.Duration) { |
There was a problem hiding this comment.
Please pass now here as well. Getting the current time isn't always free.
| String() string | ||
| Mark(count uint64) | ||
| Snapshot() Snapshot | ||
| Reset() |
There was a problem hiding this comment.
I'd drop all of these from this interface and focus on what the sweeper requires. Different meter implementations will need different interfaces (e.g., different data in their snapshots).
There was a problem hiding this comment.
Can't drop Snapshot() because it's used in MeterRegistry.walkIdle()
There was a problem hiding this comment.
I ran some tests and discovered that my changes made the library incompatible with the current version of libp2p. Turns out that BandwidthCounter from libp2p uses method MeterRegistry.ForEach(). In my version this method accepts func(string, MeterInterface) but originally it accepts func(string, *Meter). I plan to explicitly specify the right version of MeterRegistry.ForEach() for compatibility with libp2p. Do you have better suggestions?
There was a problem hiding this comment.
@Stebalien Gentle reminder, can you, please, check my last commit and say if it's fine with you?
3 participants
As it was proposed in issue#23 I have modified the code to allow user to customize the metrics calculation.
Major changes:
meterstruct calledMeterInterfacemetertoMeterInterfacesweeper.update()toMeterInterface.Update(). Nowsweeper.update()only callsMeterInterface.Update()in a loopMinor changes:
approxEqof with==, so for consistency I have changed all comparisons toapproxEq