Can we make sweeper struct customizable?

[ArtemVasilevMIPT]

Hi everyone,

Thanks for the awesome library! I've been playing with it for the purpose of tracking bandwidth in my libp2p-based project, it works like a charm and was very easy to modify the code. However, I found that it is difficult to customize how metrics are calculated without manually changing the source code. It would be really nice if we could gather extra statistics for libp2p by defining an interface or callbacks or similar.

I'd really appreciate if you let me modify go-flow-metrics to make it easier to modify Sweeper behavior, by making it into an interface. Or, if you know a better option to achieve this, i'd be happy to implement that as well. Naturally, this will not break compatibility with the rest of libp2p ecosystem.

Would it be okay if i open a pull-request — and hopefully merge it to make go-flow-metrics more flexible by default? Or is this project developed only by libp2p staff?

[marten-seemann]

Would it be okay if i open a pull-request — and hopefully merge it to make go-flow-metrics more flexible by default?

Of course! Even better if you discuss the design here first before opening the PR.

However, I found that it is difficult to customize how metrics are calculated without manually changing the source code. It would be really nice if we could gather extra statistics for libp2p by defining an interface or callbacks or similar.

I'd really appreciate if you let me modify go-flow-metrics to make it easier to modify Sweeper behavior, by making it into an interface.

How would making the sweeper allow you to collect more stats?

[ArtemVasilevMIPT]

How would making the sweeper allow you to collect more stats?

As far as I understand, the only method of a Sweeper that is exposed to the outside code is Register, so by creating a sweeper interface with this method we can allow collection of additional stats by customizing an underlying algorithm. However, right now I am stuck because globalSweeper is a global variable that is used in Meter and it can't be redefined in other packages. So current implementation of Meter doesn't allow usage of custom Sweeper. I am considering creating additional interface for Meter to solve this problem.

[Stebalien]

Could you give a specific example of how you'd like to customize it?

I'm guessing the best way to go about this will be to let the Meter internally specify a custom Sweeper (in which case, you'd need to make the Sweeper an interface as you described). However, this looks like it'll require a significant refactor as both the sweeper and meter tend to reach into each other's state directly.

[ArtemVasilevMIPT]

My idea is to move update logic from sweeper to meter, so meter interface would look like that

type MeterInterface interface {
    Update(timeDifference time.Duration)
    IsIdle() bool
    SetIdle()
    SetActive()

   // The rest of Meter methods...
}

There methods IsIdle, SetIdle, SetActive are responsible for marking meter as idle or active and method Update recalculates stats in Snapshot based on provided time since last update.
In that case sweeper update loop will look like that:

for i, m := range sw.meters[:sw.activeMeters] {
	m.Update(tdiff)
	if m.IsIdle() {
		sw.meters[i] = nil
	}
}

[Stebalien]

Maybe that'll work, but we'd want to make the sweeper an interface, not the meter.

But my question is: what kinds of changes do you want to make? Without that, there's no way to know what the correct solution is.

[ArtemVasilevMIPT]

I've proposed changes because I want to be able to calculate user defined statistics (e.g. maximum rate)
At first, I've thought that it would be enough to change only the sweeper, but after some experiments I've realized that it's easier to modify meter as described above.