In principle, we needed to iterate through all permutations, mirroring
the same that is happening in the code. For lack of time, I only
picked one of the cases currently buggy.
As said, this really needs code generation, should we ever find
ourselves touching this again.
Previously, the pickDelegator function was not returning a
*hijackerDelegator so the return value did not implement the Hijacker
interface. As a result, code that attempts to hijack the connection
would fail when using a type assertion.
All the other cases returned the hijackerDelegator correctly.
Original discussion see
https://github.com/prometheus/client_golang/pull/362 .
Assuming that the most frequently used method of a `Gauge` is `Set`
and the most frequently used method of a `Conuter` is `Inc`, this
separates the implementation of both metric types. `Inc` and integral
`Add` of a counter is now handled in a separate `uint64`. This would
create a race in `Set`, but luckily, there is no `Set` anymore in a
counter.
All attempts to solve above race (to use the same idea for a `Gauge`)
slow down `Set`, So we just stick with the old implementation
(formerly `value`) for `Gauge`.
The idea behind it is described in detail in
https://github.com/prometheus/client_golang/issues/320 .
This commit also updates the example given in
promhttp/instrument_server_test.go , which nicely illustrates the
benefit of this change.
So far, currying could be emulated by creating different metric vec's
with different values in their ConstLabels. This was quite difficult
to grasp - which is essentially what was done in the example mentioned
above. Now that this use case can be solved without ConstLabels, we
can safely declare ConstLabels as rarely used. (Perhaps we can
deprecate them entirely one day, but I'll take a raincheck on that
when the changes of v0.10 have materialized.) This commit thus also
updates the ConstLabel doc comments in the various Opts. (It contained
fairly outdated stuff anyway.)
The "panic in case of error" code was so far in metricVec. This pulls
it up into the exported types like CounterVec. This is code
replication, but it avoids an explicit type conversion. Mostly,
however, this is preparation to make the wrapped metricVec an
interface (required for curried vec's).
Specifically @beorn7 pointed out that the previous implementation had
some shortcomings around large numbers. I've changed the code to match
the suggestion in review, as well as added a few test cases.
MetricVec is un-experted by now. godoc handles that correctly by
showing the methods of the embedded un-exported metricVec with the
exported type (CounterVec, SummaryVec, ...) that embeds metricVec.
This is in preparation for "curried" metric vecs, as discussed.
And it's a good thing anyway. The exported MetricVec was from a time
when I thought people would define own Metric types and then create
Vecs of it. That has never happened.
As it turned out, it's not that esay to guess "common" combination of
interface upgrades. So I decided to just implement all 32 possible
combination of interface upgrades. (Only 16 with Go 1.7 and earlier.)
Clearly, this calls for code generation. But right now, we still need
to find out what's the best form of the code. For later additions,
implementing code generation might be useful.
Note that newDelegator is called for each HTTP request. Thus, this
commit aims to make the upgrade selection quick. (After the type
checks, it's just directly accessing an element in a slice.)
The example method is assumed to be used as main() function. As a main()
function doesn't have any return values, the example doesn't compile and
is invalid.
This also updates all tests and examples to use explicitly set
objectives.
In v0.10, DefObjectives will be completely removed, and the default
Summary will have no objectives then.
Fixes#118
This finally makes the (presumably) common simple case as simple as it
gets.
The still quite common (but less common) case of using a Gauge is
slightly more verbose now, but not needing to provide a separate
constructor is totally worth it.
Finally, the advanced use case is not really more verbose as in my
original suggestion. However, the logic to decide which Observer to
use is now all in the ObserverFunc handed in at construction
time. This is deliberate and desired. It makes sure the selection
mechanism is all spelled out there. No surprises buried deep in the
function code somewhere.
- 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.
All was a mess, we had duplicates of the REs for label name and metric
names here, and we sometimes used them, sometimes we used those from
common/model.
Now we are consistently using the fast checking functions from common/model.
(Tests for leading colons are included there, see
https://github.com/prometheus/common/pull/66 .)
That's the "soft" part of the deprecation: Everything that has been
marked deprecated in v0.8 or earlier and is straight-forward to
replace by a non-deprecated way, is removed here.
Sadly, this does not include the HTTP part. We first need to provide a
replacement for HTTP instrumentation (as planned for v0.8) to then
remove the deprecated parts in v0.9.
After increasing unit test coverage, it was found that the split
function call nature of metric matching wasn't working well in many
cases. By increasing test coverage, we've ensured that both the fast
path and fallback collision path are working appropriately.
With these changes, there is a further performance hit, but now the
results are ensured to be correct.
Signed-off-by: Stephen J Day <stephen.day@docker.com>
While hash collisions are quite rare, the current state of the client
library carries a risk of merging two separate label values into a
single metric bucket. The effects are near impossible to detect and the
result will be missing or incorrect counters.
This changeset handles hash collisions by falling back to collision
resolution if multiple label values hash to the same value. This works
similar to separate chaining using a slice. Extra storage is minimized
to only the value key slice to that metrics can be differentiated
within a bucket.
In general, the cost of handling collisions is completely minimized
under normal operation. Performance does show slight increases in
certain areas, but these are more likely statistically anomalies. More
importantly, zero allocation behavior for metrics is preserved on the
fast path. Minimal allocations may be made during collision handling but
this has minimal effect.
Benchmark comparisons with and without collision resolution follow.
```
benchmark old ns/op new ns/op delta
BenchmarkCounterWithLabelValues-4 99.0 107 +8.08%
BenchmarkCounterWithLabelValuesConcurrent-4 79.6 91.0 +14.32%
BenchmarkCounterWithMappedLabels-4 518 542 +4.63%
BenchmarkCounterWithPreparedMappedLabels-4 127 137 +7.87%
BenchmarkCounterNoLabels-4 19.5 19.1 -2.05%
BenchmarkGaugeWithLabelValues-4 97.4 110 +12.94%
BenchmarkGaugeNoLabels-4 12.4 10.3 -16.94%
BenchmarkSummaryWithLabelValues-4 1204 915 -24.00%
BenchmarkSummaryNoLabels-4 936 847 -9.51%
BenchmarkHistogramWithLabelValues-4 147 147 +0.00%
BenchmarkHistogramNoLabels-4 50.6 49.3 -2.57%
BenchmarkHistogramObserve1-4 37.9 37.5 -1.06%
BenchmarkHistogramObserve2-4 122 137 +12.30%
BenchmarkHistogramObserve4-4 310 352 +13.55%
BenchmarkHistogramObserve8-4 691 729 +5.50%
BenchmarkHistogramWrite1-4 3374 3097 -8.21%
BenchmarkHistogramWrite2-4 5310 5051 -4.88%
BenchmarkHistogramWrite4-4 12094 10690 -11.61%
BenchmarkHistogramWrite8-4 19416 17755 -8.55%
BenchmarkHandler-4 11934304 13765894 +15.35%
BenchmarkSummaryObserve1-4 1119 1105 -1.25%
BenchmarkSummaryObserve2-4 3679 3430 -6.77%
BenchmarkSummaryObserve4-4 10678 7982 -25.25%
BenchmarkSummaryObserve8-4 22974 16689 -27.36%
BenchmarkSummaryWrite1-4 25962 14680 -43.46%
BenchmarkSummaryWrite2-4 38019 35073 -7.75%
BenchmarkSummaryWrite4-4 78027 56816 -27.18%
BenchmarkSummaryWrite8-4 117220 132248 +12.82%
BenchmarkMetricVecWithLabelValuesBasic-4 138 133 -3.62%
BenchmarkMetricVecWithLabelValues2Keys10ValueCardinality-4 150 144 -4.00%
BenchmarkMetricVecWithLabelValues4Keys10ValueCardinality-4 263 256 -2.66%
BenchmarkMetricVecWithLabelValues2Keys100ValueCardinality-4 145 155 +6.90%
BenchmarkMetricVecWithLabelValues10Keys100ValueCardinality-4 606 634 +4.62%
BenchmarkMetricVecWithLabelValues10Keys1000ValueCardinality-4 746 641 -14.08%
benchmark old allocs new allocs delta
BenchmarkCounterWithLabelValues-4 0 0 +0.00%
BenchmarkCounterWithLabelValuesConcurrent-4 0 0 +0.00%
BenchmarkCounterWithMappedLabels-4 2 2 +0.00%
BenchmarkCounterWithPreparedMappedLabels-4 0 0 +0.00%
BenchmarkCounterNoLabels-4 0 0 +0.00%
BenchmarkGaugeWithLabelValues-4 0 0 +0.00%
BenchmarkGaugeNoLabels-4 0 0 +0.00%
BenchmarkSummaryWithLabelValues-4 0 0 +0.00%
BenchmarkSummaryNoLabels-4 0 0 +0.00%
BenchmarkHistogramWithLabelValues-4 0 0 +0.00%
BenchmarkHistogramNoLabels-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValuesBasic-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValues2Keys10ValueCardinality-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValues4Keys10ValueCardinality-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValues2Keys100ValueCardinality-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValues10Keys100ValueCardinality-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValues10Keys1000ValueCardinality-4 0 0 +0.00%
benchmark old bytes new bytes delta
BenchmarkCounterWithLabelValues-4 0 0 +0.00%
BenchmarkCounterWithLabelValuesConcurrent-4 0 0 +0.00%
BenchmarkCounterWithMappedLabels-4 336 336 +0.00%
BenchmarkCounterWithPreparedMappedLabels-4 0 0 +0.00%
BenchmarkCounterNoLabels-4 0 0 +0.00%
BenchmarkGaugeWithLabelValues-4 0 0 +0.00%
BenchmarkGaugeNoLabels-4 0 0 +0.00%
BenchmarkSummaryWithLabelValues-4 0 0 +0.00%
BenchmarkSummaryNoLabels-4 0 0 +0.00%
BenchmarkHistogramWithLabelValues-4 0 0 +0.00%
BenchmarkHistogramNoLabels-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValuesBasic-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValues2Keys10ValueCardinality-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValues4Keys10ValueCardinality-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValues2Keys100ValueCardinality-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValues10Keys100ValueCardinality-4 0 0 +0.00%
BenchmarkMetricVecWithLabelValues10Keys1000ValueCardinality-4 0 0 +0.00%
```
Signed-off-by: Stephen J Day <stephen.day@docker.com>
This allows to finally get rid of the infamous injection hook in the
interface. The old SetMetricFamilyInjectionHook still exist as a
deprecated function but is now implemented with the new plumbing under
the hood.
Now that we have multiple Gatherer implementation, I renamed
push.Registry to push.FromGatherer.
This commit also improves the consistency checks, which happened as a
byproduct of the refactoring to allow checking in both the "merge
gatherer" Gatherers as well as in the normal Registry.
To keep backwards compatibility while not creating circular import
chains, some code had to be duplicated. But all functions using it
have been declared deprecated hereby.
The new ways of instrumenting handlers will all go into the new
package, and ultimately, the prometheus package itself will be
completely igorant of HTTP.
- Moved the Deliverer parameter to the end of the list to mirror
Collectors in push.Collectors.
- Improved doc comment and added an example for push.Registry.
Registry is now a struct, which implements two interfaces, Registrerer
and Deliverer. The latter is particularly important as it is now the
argument type for pushes and HTTP handler construction (i.e. it is
easy to implement a custom Deliverer for testing or other
purposes). The Registerer interface is not used as a parameter type
but can (and should) be used by users of custom registries so that
they can easily do things like mocking it out for testing purposes.
With the broken up interfaces, adding MustRegister to the interface is
not such a big deal anymore (interface is still small). And since
setting the injection hook is such a rare thing to happen, it is
acceptable to not have it in any of the interfaces.
The renaming from `Collect` to `Deliver` was done to avoid confusion
with Collectors. (The registry _collects_ from the Collectors, and
then _delivers_ to the exposition mechanism.)
beorn7