reflect.DeepEqual is not suitable for zero occurrences of repeated
proto messages. This changes the comparison to act on the string
representation of proto messages.
Signed-off-by: beorn7 <beorn@soundcloud.com>
So that also users of those can benefit. Obviously, we will end
updating deprecated functions one day (at latest once v0.10 is out).
Signed-off-by: beorn7 <beorn@soundcloud.com>
This is an attempt to expose
https://github.com/istio/istio/issues/8906 . The failure to do so
makes me believe the error is either already fixed in current
client_golang, or something weird I haven't spotted yet is happening
in the istio code.
Signed-off-by: beorn7 <beorn@soundcloud.com>
it appears there is a copy/paste error in the exponential buckets test failure message which is fixed here.
Signed-off-by: David Worth <dworth@strava.com>
This is in line with
https://prometheus.io/docs/instrumenting/writing_clientlibs/#metric-description-and-help
Since the zero value of a string in Go is `""`, we cannot distinguish
between a Help string not set and an empty Help string. Thus, we just
make it formally optional here with an encouragement to set it in the
doc comment.
In v0.10, the Help string will probably become a "normal" argument of
the constructor rather than a field in an Opts struct.
Signed-off-by: beorn7 <beorn@soundcloud.com>
It now uses the new WrapWith function instead of ConstLabels. Describe
is now implemented via DescribeByCollect.
Signed-off-by: beorn7 <beorn@soundcloud.com>
This unifies both constructors in one with an options argument.
The options argument allows to switch on error reporting, as discussed
in #219.
The change of the contructor signature is breaking, but only mildly
so. Plus, the process collector is rarely used explicitly. I used
Sourcegraph to search for public usages, with the following results:
- 2 occurrences of NewProcessCollectorPIDFn, once in @discordianfish's
glimpse, once in @fabxc's etcd_exporter (deprecated anyway). Both
are Prom veterans and will simply do the one line change if needed.
- 8 occurrences of NewProcessCollector, of which 7 are of the form
NewProcessCollector(os.Getpid(), "")
Thus, it's a very easy change, which I even hinted at in the doc
comment.
Signed-off-by: beorn7 <beorn@soundcloud.com>
The only known external usage of it was in prometheus/pushgateway,
where it was removed by
https://github.com/prometheus/pushgateway/pull/200 .
Originally, the expectation was that users would implement the Metric
interface now and then. As we know now, neither it is happening, nor
would it make a lot of sense. (Users implement the Collector interface
instead.) By now, LabelPairSorter is essentially noise in the already
quite cluttered namespace in the prometheus package.
Signed-off-by: beorn7 <beorn@soundcloud.com>
This is for types we don't want to export but which are used in
different packages within client_golang.
Currently, that's only NormalizeMetricFamilies (used in the prometheus
package and in the testutil package). More to be added as needed.
Signed-off-by: beorn7 <beorn@soundcloud.com>
- Expected text format is now read from an io.Reader.
- Metrics are gathered from a Gatherer.
- Added a convenience wrapper to collect from a Collector.
Signed-off-by: beorn7 <beorn@soundcloud.com>
`testutil` is more in line with stdlib naming conventions.
The package should be below `prometheus` as it only provides utils to
test exposition code, not to test HTTP client code.
Signed-off-by: beorn7 <beorn@soundcloud.com>
So far, if a gauge was named `xxx_count`, and a summary or histogram
`xxx`, this would have led to a legal protobuf exposition but would
have created a name collision on `xxx_count` in the text format and
within the Prometheus server.
Signed-off-by: beorn7 <beorn@soundcloud.com>
Also, clarify in the doc comment.
Previously, the assumption was that inconsistent label dimensions are
violating the exposition format spec. However, especially with the
knowledge that OpenMetrics will explicitly allow inconsistent label
dimensions in expositions, we should allow it in client_golang, too.
Note that registration with proper Descs provided will still check for
consistont label dimensions. However, you can "cheat" with custom
Collectors as you can collect metrics that don't follew the provided
Desc (this will be made more explicit and less cheaty once #47 is
fixed). You can also create expositions with inconsistent label
dimensions by merging Gatherers with the Gatherers slice type. (The
latter is used in the Pushgateway.)
Effectively, normal direct instrumentation will always have consistent
label dimensions in this way, but you can cover special use cases with
custom collectors or merging of different Gatherers.
Signed-off-by: beorn7 <beorn@soundcloud.com>
While not strictly correct, it can easily happen that proto messages
are created that use nil pointers instead of pointers in empty strings
to denote an empty string.
Signed-off-by: beorn7 <beorn@soundcloud.com>
Fixes:
http.go:118: declaration of "part" shadows declaration at http.go:117
http_test.go:50: declaration of "respBody" shadows declaration at http_test.go:25
promhttp/http.go:305: declaration of "part" shadows declaration at promhttp/http.go:304
Signed-off-by: Karsten Weiss <knweiss@gmail.com>
The test case requires the /proc filesystem. The change prevents this skip
message during "go test -v" on platforms other than Linux:
=== RUN TestProcessCollector
--- SKIP: TestProcessCollector (0.00s)
process_collector_test.go:15: skipping TestProcessCollector, procfs not available: could not read /proc: stat /proc: no such file or directory
Signed-off-by: Karsten Weiss <knweiss@gmail.com>
Fixes:
prometheus/http_test.go:117:5⚠️ should use resp.Body.String() instead of string(resp.Body.Bytes()) (S1030) (megacheck)
prometheus/http_test.go:118:56⚠️ should use resp.Body.String() instead of string(resp.Body.Bytes()) (S1030) (megacheck)
Signed-off-by: Karsten Weiss <knweiss@gmail.com>
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.
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.
beorn7