expvar: add Map.Delete

[jacksontj]

I've been using expvar for a few metrics related cases, and its great. The one thing I really see as missing is that there is no mechanism to remove items from an expvar.Map. I had created a PR with a simple Remove method which returns the Var associated with key-- or nil. I don't see any reason that it shouldn't-- so I created a simple patch (#13490) which adds a Remove method to expvar.Map

[ianlancetaylor]

Thanks. We don't take github pull requests. To contribute your patch please follow the procedure at https://golang.org/doc/contribute.html . Note that at this point it will have to wait for the Go 1.7 release.

[dsymonds]

The Remove method was not in the original API because monitoring software of the kind that scrapes such output can often get confused by a disappearing map key.

I think any change to add a Remove method would need to justify its usefulness as outweighing potentially malfunctioning monitoring.

[jacksontj]

Well, the particular use case I have highlights this fairly well IMO. I have a service which maintains state of downstream services. The list of all downstream services is fluid-- since it is determined by a source of truth service. As such my service has some metrics for each downstream. Since I cannot call "remove" i effectively leak N metrics per service that appears and then gets removed, since I cannot remove the metrics associated with that service.

I don't think adding a Remove method immediately constitutes "breaking monitoring" as it would still require the developer to actually call Remove.

[jonsyu1]

Is there any disagreement with or recommended workaround for @jacksontj's use case?

[tsuna]

I have exactly the same issue as @jacksontj. I don't buy the "borgmon can't handle it so we didn't add this API" argument. We expose a wealth of information over expvar that is useful for troubleshooting problems, how this gets consumed/stored by a particular monitoring backend isn't really the point.

Looks like the Kapacitor project already forked the code of expvar for this reason https://github.com/influxdata/kapacitor/blob/master/expvar/expvar.go (and made some other nice improvements such as influxdata/kapacitor#394). See this post for more info: https://www.influxdata.com/blog/influxdb-debugvars-endpoint/

The first reason they give why they forked the code:

it does not allow you to remove published variables.

[agnivade]

Moving this to proposal so that it gets some visibility.

[rsc]

OK, it does seem like this is missing from expvar.Map's methods.
It should be called Delete though (like the builtin).