Relax consistency checks during gathering #417
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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>
brian-brazil
reviewed
Jun 7, 2018
prometheus/registry.go
Outdated
| // prevented any meaningful metric collection) or contain a number of | ||
| // MetricFamily protobufs, some of which might be incomplete, and some | ||
| // might be missing altogether. The returned error (which might be a | ||
| // MultiError) explains the details. In scenarios where complete |
There was a problem hiding this comment.
I know this is an existing comment. Collection should be an all or nothing thing by default as you're going to get unworkable semantics otherwise (e.g. that cadvisor bug), this wording implies that that's not usually the case rather than being a use case so rare we've yet to see it. I'd suggest something like "the returned MetricFamily protobufs should almost always be discarded"
There was a problem hiding this comment.
I have changed the wording to something stronger.
Signed-off-by: beorn7 <beorn@soundcloud.com>
This was referenced Sep 7, 2018
|
Hi! I'd like to upgrade statsd_exporter to a version of client_golang with this change, so that a metric producer does not need to select a globally unique metric name. Is it possible to include this in a release? Thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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