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

noop mode #196

Closed
fabxc opened this issue Apr 3, 2016 · 8 comments
Closed

noop mode #196

fabxc opened this issue Apr 3, 2016 · 8 comments

Comments

@fabxc
Copy link
Contributor

fabxc commented Apr 3, 2016

It should be possible to have a no-op registry that does nothing.

Many things should be instrumented. But despite being fast, there can still be performance impact. Having a way to make instrumentation calls no-ops will provide quick benchmark feedback on the instrumentation overhead.

Instrumenting packages/libs is tricky since it might turn some potential users away, who don't want to use it and see it as "bloat". So this would also provide a simple way to effectively turn off instrumentation via a configuration parameter of your library.

@fabxc
Copy link
Contributor Author

fabxc commented Apr 3, 2016

To clarify: This does not only affect registries, there would have to be a general no-op mode. Also affecting metrics etc.

One option would be having interfaces where ever necessary that and implementations that resolve to empty functions. The other one checking bools everywhere.

That would have to be evaluated performance-wise.

@beorn7 beorn7 added v0.9 and removed v0.9 labels Aug 16, 2016
@beorn7 beorn7 added this to the v0.10 milestone Aug 19, 2016
@beorn7
Copy link
Member

beorn7 commented Aug 19, 2016

This requires #223 (we have to switch the various XxxVecs into interfaces to back them with no-op implementations) and therefore has to go into 0.10.

@brian-brazil
Copy link
Contributor

I'm unsure we should be adding the amount of complexity this requires for such a rare use case.

How about a dummy noop Prometheus package that you could import in a file instead of the main Prometheus package. It'd only need to support direct instrumentation.

@beorn7
Copy link
Member

beorn7 commented Aug 23, 2016

That would definitely be a possibility. But it requires code changes for the switch.

I see the most frequent use case for this with libraries that provide Prometheus instrumentation, but not everybody wants to use the instrumentation and thus might not be willing to pay for the performance overhead, may the price be ever so low.

My plan is to add the noop as an option to XxxOpts, which doesn't get into the way of those not interested in noop mode but enables a switch without code changes. Code complexity is the same as with the dummy package solution, just that we don't need an extra package.

@brian-brazil
Copy link
Contributor

That sounds like a reasonable approach.

@beorn7
Copy link
Member

beorn7 commented Oct 25, 2018

Now that the work on v0.10 is getting rolling, I would like to revisit this.

As already stated above: In addition to the single metric types, also the metric-vector types will become interfaces (#223).

With that change done, it will be simple to mock-out metrics, may it be for testing or, like in this case, for creating "no-op" implementations. What I'm now thinking is if we really need additional support this use case in client_golang.

Most metric methods are really just atomic increments of numbers and thus so fast that the replacement by no-op metrics will only matter in very few extreme cases. The only metric type that is significantly slower is the Summary with quantiles. We can implement a summary without quantiles in the same way as the histogram and thus make it lock-free, too. That means that the only significant performance drain of metrics could be removed by (optionally) removing the quantiles from the Summary.

In those very few use cases where performance still matters enough, we can ask the user to invest the additional effort of creating custom constructor functions that take a configuration option into account and then provide either the usual implementation of the various metric interfaces or a custom no-op one (which is trivial to write).

As suggested above, it would be easy to add an option to the various CounterOpts etc., but I would actually like to get rid of CounterOpts and GaugeOpts, more about that later. And I don't think this no-op mode should make things more complicated in any way for the majority of users that don't need it.

Thoughts? @fabxc who filed this, @diafour who thumb'd this. I guess @brian-brazil shares my concerns.

@beorn7
Copy link
Member

beorn7 commented Oct 26, 2018

I should also mention that among all the feedback I got for client_golang, nobody asked for this feature. (The "interface everything" was in fact requested, but for different reasons.)

@beorn7
Copy link
Member

beorn7 commented Nov 13, 2018

I take silence from @fabxc and @diafour as consent and close this. Please comment/reopen if you want to intervene.

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

No branches or pull requests

3 participants