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

Add a package promauto that provides auto-registering metrics #385

Merged
merged 1 commit into from
Feb 16, 2018

Conversation

beorn7
Copy link
Member

@beorn7 beorn7 commented Feb 15, 2018

Finally, I found an easy solution to provide the "evil"
auto-registration without getting death threats from the wardens of Go
purity. The reasoning can be found in the package's doc comment.

@brian-brazil I think you will like this.
@grobie next time you give an instrumentation introduction, you can use this package and ignore the registry with its Registerer and Gatherer interfaces altogether.

@grobie
Copy link
Member

grobie commented Feb 15, 2018

Oh nice!

Copy link
Contributor

@brian-brazil brian-brazil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a fan of this.

Another option that I've been pondering recently is adding a MustRegister method to Counter&friends which then returns the Counter. This which avoids having to do the separate init() block as that's often forgotten.

// The only way is panicking. While panicking on invalid input provided by the
// programmer is certainly fine, things are a bit more subtle in this case: You
// might just add another package to the program, and that package (in its init
// function) happens to register a metric with the same name as your code. Now,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd change this to cover it happening at runtime due to loading some new code somehow (plugin?). Crashing out at startup isn't that bad, as it'll be pretty obvious from unittests. When you're already up and running however it's an outage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add something along the lines "even worse, during runtime". But I want to keep the panic-at-startup scenario in there as well as many proficient Go devs feel strongly about it, too.

@beorn7
Copy link
Member Author

beorn7 commented Feb 16, 2018

Another option that I've been pondering recently is adding a MustRegister method to Counter&friends which then returns the Counter.

Which I had considered earlier, but rejected it, as idiomatic Go tends to reduce the number of methods in an interface. You would essentially pollute the interface for everybody who doesn't want to deal with global state anyway. That's why I'm so happy with the package solution here. Whoever wants to go down the "easy" path, just uses this package. Slightly different name when calling the constructors, that's very little friction, while the purists will never even notice that the package exists.

Finally, I found an easy solution to provide the "evil"
auto-registration without getting death threats from the wardens of Go
purity. The reasoning can be found in the package's doc comment.
@beorn7
Copy link
Member Author

beorn7 commented Feb 16, 2018

Improved the package doc comment to include comments above.

@brian-brazil
Copy link
Contributor

👍

@beorn7 beorn7 merged commit e69720d into master Feb 16, 2018
@beorn7 beorn7 deleted the beorn7/auto branch February 16, 2018 13:12
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.

None yet

3 participants