Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mkeeler
Copy link
Member

@mkeeler mkeeler commented Jun 8, 2023

Also bumping hashicorp/memberlist to the corresponding version which also uses hashicorp/go-metrics

This will require a new major version to signify that metrics are using different global handlers.

TODO:

Also bumping hashicorp/memberlist to the corresponding version which also uses hashicorp/go-metrics
@mkeeler mkeeler requested a review from a team as a code owner June 8, 2023 16:57
@mkeeler mkeeler requested review from JadhavPoonam and rboyer and removed request for a team, JadhavPoonam and rboyer June 8, 2023 16:57
@puffpuf
Copy link

puffpuf commented Jul 25, 2023

can someone merge it, please?

@peczenyj
Copy link

peczenyj commented Nov 8, 2023

+1 here

I need to the line below several go.mod files and it is annoying

replace github.com/armon/go-metrics v0.5.1 => github.com/hashicorp/go-metrics v0.5.1

@peczenyj
Copy link

peczenyj commented Nov 8, 2023

fix issue #707

@alex-savin
Copy link

+1 here

@peczenyj
Copy link

any reason to not merge it ?

@adamrothman
Copy link

@mkeeler @hashi-derek can we get some 👀 on this?

@frebib
Copy link

frebib commented Apr 28, 2024

Bump please; without this patch, this library is completely unusable/broken for anyone using it, or pulling in something that uses it

@shane-ns1
Copy link

Wow this PR is more than a year old! Is the repository completely unmaintained? 😬

@peczenyj
Copy link

peczenyj commented Jul 3, 2024

@shane-ns1 looks like ...

@peczenyj
Copy link

peczenyj commented Jul 3, 2024

perhaps we should start to ping @dhiaayachi and other people that can merge on this repo

@shane-ns1
Copy link

perhaps we should start to ping @dhiaayachi and other people that can merge on this repo

Or even @armon? 😆

@jmurret
Copy link
Member

jmurret commented Jul 5, 2024

Hi all, we will post a more detailed thread on why this is complicated, but the short gist is that this change has to be consistently updated across the dependency chains of libraries used in an application. go-metrics relies on a singleton sink in global state. If any applications have a mix of armon metrics and hashicorp due to underlying dependencies not aligning on only one of them, metrics will be split among different sinks in the global state.
This change has to be coordinated with changing and releasing this dependency across multiple libraries and applications in a thoughtful way. This PR is taking longer than expected due to the prioritization and coordination of those efforts to ensure a smooth transition. Thank you for your patience and understanding.

@shane-ns1
Copy link

It's been 18 months since this was opened, and 5 months since the last comment, but I think that this is still an issue.

Is there anything that anyone can do to help?

@mkeeler
Copy link
Member Author

mkeeler commented Dec 6, 2024

After authoring these PRs originally, we came to the realization that the problem wasn't quite so simple and we were going to have a difficult time rolling out the changes in a safe way across many libraries, products, services and applications. Safe here meaning where we don't accidentally lose metrics.

Recently I have spent some more time working on the problem and found an approach that I think will be better. The gist is that I am introducing a compatibility package into hashicorp/go-metrics. This exposes an identical API to that of the latest armon/go-metrics tag. The compat package can be controlled with build tags to choose which metrics package is actually used (either armon/go-metrics with a armonmetrics tag or hashicorp/go-metrics with a hashicorpmetrics tag - default is still armon/go-metrics). I am now updating all our libraries which use armon/go-metrics to use this compatibility package instead. As a consumer of this library you are free to set the build tags as you wish to control which metrics library is compiled in.

The big advantage I see with the build tagged approach is that the top level application performing the go build is in control of which metrics library is used for all the libraries. This is especially important for many repos within the hashicorp namespace that each use many of the libraries.

I am hoping to have all the libraries PR'ed and merged before the end of the month.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants