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

Wrong import at m3db/prometheus_client_golang/prometheus/promhttp/http.go:46 #78

Open
oakad opened this issue May 21, 2018 · 5 comments

Comments

@oakad
Copy link

oakad commented May 21, 2018

The file in question imports "github.com/prometheus/client_golang/prometheus" package at line 46. This results in a wrong link for prometheus.DefaultRegistry symbol. The wrong link, in turn, results in a very annoying issue, whereupon prometheus metric endpoint seemingly works, but no user registered metrics are available on it.

@darxtrix
Copy link

darxtrix commented Jun 12, 2018

Yes it happens with me also, apparently, I fixed by changing the import statement to github.com/m3db/client_golang/prometheus which let's use the correct Registry that is being pointed in the http.go file and expose the user-defined metrics. This is because the uber tally writes to the DefaultRegistry in github.com/m3db/client_golang/prometheus package whereas the http module uses the DefaultRegistry of github.com/prometheus/client_golang/prometheus package.

I think it can be avoided by using the prometheus package rather than the m3db unless done purposefully.

@oakad
Copy link
Author

oakad commented Jun 13, 2018

There's a compatibility issue between tally and newer versions of prometheus api, hence this fork.

@darxtrix
Copy link

okay, thanks for pointing this out.

@robskillington
Copy link
Contributor

Yes it's unfortunate. To avoid having dependency management fight over differing versions of Prometheus client library (which are incompatible with each other), we ended up just taking a copy of the library.

This was truly the only way to ensure end users wouldn't face build issues if they relied on any other package that reference the Prometheus client library.

In the future we'd ideally move the tally Prometheus reporter into its own repository so that it can be updated each time there is a new Prometheus client library release. This would decouple the reporter from tally itself and avoid tally having to make "breaking changes" by bumping Prometheus dependency, and end users could use the version of the Prometheus reporter that matches their Prometheus client library version.

@aud10slave
Copy link

Any chance this upstream fix to client_golang can be cherry-picked to fork?

prometheus/statsd_exporter#76 (comment)

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

No branches or pull requests

4 participants