Rework type structure #223
Labels
Milestone
Comments
This was referenced Aug 19, 2016
Closed
beorn7
pushed a commit
that referenced
this issue
Nov 18, 2016
- Point from Inc and Dec to Add and Sub in doc comments. - Deprecate Untyped for direct instrumentation. - Add a SetToCurrentTime method to Gauge Note that adding the SetToCurrentTime method is not really following Go's principle of lean interfaces. However, the Gauge interface is already quite fat. (The only methods really required are Set and Add. Everything else could be expressed in terms of those two.) So we have already quite a few "convenience" methods traditionally, so I think we should stay consistent here. The alternatives would be: - Not support SetToCurrentTime at all (it's only a SHOULD in the guidelines). - A top level function `SetToCurrentTime(Gauge)`. - Just a helper `CurrentTime()` that returns the curent unix time in seconds as a float (which is pretty verbose using the standard library, see code in this commit). This would allow `myGauge.Set(CurrentTime)`. Weighing all circumstances, I believe the way in this commit is the least evil. Issue #223 could be used to rework interfaces more fundamentally in a breaking change if feasible.
beorn7
pushed a commit
that referenced
this issue
Nov 18, 2016
- Deprecate Untyped for direct instrumentation. - Add a SetToCurrentTime method to Gauge Note that adding the SetToCurrentTime method is not really following Go's principle of lean interfaces. However, the Gauge interface is already quite fat. (The only methods really required are Set and Add. Everything else could be expressed in terms of those two.) So we have already quite a few "convenience" methods traditionally, so I think we should stay consistent here. The alternatives would be: - Not support SetToCurrentTime at all (it's only a SHOULD in the guidelines). - A top level function `SetToCurrentTime(Gauge)`. - Just a helper `CurrentTime()` that returns the curent unix time in seconds as a float (which is pretty verbose using the standard library, see code in this commit). This would allow `myGauge.Set(CurrentTime)`. Weighing all circumstances, I believe the way in this commit is the least evil. Issue #223 could be used to rework interfaces more fundamentally in a breaking change if feasible.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Current issues:
UntypedandGauge;HistogramandSummary; all theXxxFuncmetrics), they are the same for Go (you could assign a Histogram to a Summary variable). The number of interfaces can be reduced a lot here.Metriccan be way less exposed as it is essentially only used internally by the registry. It's probably used by normal using mostly for testing, but it is planned to provide some test tools to make that unnecessary.GaugeandCountershould just have the methods to manipulate the metrics value in instrumentation.CollectorandMetricshould be separate, and then have aGaugeCollectorto return byNewGaugeand such.XxxVecinterfaces so that we can hideMetricVec.Overall, the number of exported types should be reduced drastically.
The text was updated successfully, but these errors were encountered: