hashicorp / memberlist

Golang package for gossip based membership and failure detection
Mozilla Public License 2.0
3.61k stars 435 forks source link

Upgrade armon/go-metrics to hashicorp/go-metrics #287

Open mkeeler opened 1 year ago

mkeeler commented 1 year ago

This will require a major version bump in the library as it changes which global metrics handler gets used (due to the module renaming)

aknuds1 commented 1 year ago

@mkeeler are you sure a major version bump is required, since according to semver you're free to make breaking changes so long as you're on major version 0?

mkeeler commented 1 year ago

@aknuds1 From an exported API perspective nothing would be broken. The main reason I would advocate for a major version bump would be to signal to consumers that they may need to take some action to make things work properly. Because hashicorp/go-metrics and armon/go-metrics look like distinct modules to Go, both could potentially be compiled into one tool. If the application is configuring metrics handlers for armon/go-metrics (which has its own global metrics instance) and memberlist emits metrics to hashicorp/go-metrics, then the memberlist metrics likely would not show up in how the application is recording them (prometheus, dogstatsd etc.)

IMO all this points to some larger changes being needed in these libraries. Instead of using a default global metrics instance the library ought to allow configuring and interface or a set of callbacks to be used when emitting metrics. That way the application consuming the library is in full control of metrics emission.

aknuds1 commented 1 year ago

Instead of using a default global metrics instance the library ought to allow configuring and interface or a set of callbacks to be used when emitting metrics.

That sounds like the right approach to me.

balena-zh commented 8 months ago

+1

adamrothman commented 6 months ago

@rboyer @jmurret @huikang @NodyHub can we get some 👀 on this?