Improve testability #58

beorn7 · 2015-01-27T18:02:05Z

First of all, there is the public Registry interface tracked in #46

But there is more:

Two things come to my mind here, from the email thread:

(1)
Another utility function could compare two collectors (or a collector
to a list of template metrics) and verify that they return the same
set of metrics. In that way, you could simply create the expected
metrics and verify that the collector to be tested collects the same
metrics.
[...]
But one of my suggestions was to have a comparison utility function
that would abstract away the order (as the Prometheus registry does
it, too).
Something like

testutil.Collects(myCollector, []prometheus.Metric{
    prometheus.MustNewConstMetric(...),
}

(2)

func readCounter(m prometheus.Counter) float64 {
    pb := &dto.Metric{}
    m.Write(pb)
    return pb.GetCounter().GetValue()
}

Could be done in a generic way, like this:

package testutil

func ToFloat64(m prometheus.Metric) float64 {
    pb := &dto.Metric{}
    m.Write(pb)
    if pb.Gauge != nil {
        return pb.Gauge.GetValue()
    }
    if pb.Counter != nil {
        return pb.Counter.GetValue()
    }
    if pb.Untyped != nil {
        return pb.Untyped.GetValue()
    }
    return math.NaN()
}

The text was updated successfully, but these errors were encountered:

brian-brazil · 2015-01-27T18:05:23Z

https://github.com/prometheus/client_java/blob/master/simpleclient/src/main/java/io/prometheus/client/CollectorRegistry.java#L102 is one way that the java_client makes things easier to test

pending prometheus/client_golang#58, just check for presence of expected metrics

grobie · 2015-05-14T15:12:42Z

👍

General context and approch --------------------------- This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? -------------------------------- This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ to fat. This commit introduces a very lean `registry.Registry` interface. Most of the existing functionality that is not part of that interface is provided by utility functions (in `registry/util.go`). The top-level function that act on the default registry are mostly retained and kept in the `prometheus` package, to minimize breakage and for convenience. The general idea is here that the basic use case isn't involved with the `registry` package at all. Only if you are going for one of the advanced use cases, you need to import the `registry` package. The default registry is kept in the public variable `registry.Default`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`). Another important part in making the registry lean is the extraction of the HTTP exposition into its own package `promhttp`. Note that the package is not simply called `http` because that would collide with the `http` package from the standard library, which will usually be used in the same source file as the `promhttp` package. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): #46 #100 #170 #205 What future changes does this commit enable? -------------------------------------------- - The separation of the HTTP exposition allows the implementation of other exposition methods as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. #197 - The public `Registry` interface allows to implement convenience tools for testing metric collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. #58 Notable non-goals of this commit -------------------------------- The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers in the new `promhttp` package and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. Cf. #200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. #47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. An unfortunate side effect is that the fields of the current `Desc` had to be exported for the time being. This will be solved in a cleaner way with the upcoming `Desc` changes. Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection, and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion. What are the breaking changes? ------------------------------ - Signature of functions pushing to Pushgateway (long planned anyway, and now that I worked on it anyway, I did it, cf. #100). - TODO

General context and approch --------------------------- This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? -------------------------------- This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ to fat. This commit introduces a very lean `registry.Registry` interface. Most of the existing functionality that is not part of that interface is provided by utility functions (in `registry/util.go`). The top-level function that act on the default registry are mostly retained and kept in the `prometheus` package, to minimize breakage and for convenience. The general idea is here that the basic use case isn't involved with the `registry` package at all. Only if you are going for one of the advanced use cases, you need to import the `registry` package. The default registry is kept in the public variable `registry.Default`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`). Another important part in making the registry lean is the extraction of the HTTP exposition into its own package `promhttp`. Note that the package is not simply called `http` because that would collide with the `http` package from the standard library, which will usually be used in the same source file as the `promhttp` package. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): #46 #100 #170 #205 What future changes does this commit enable? -------------------------------------------- - The separation of the HTTP exposition allows the implementation of other exposition methods as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. #197 - The public `Registry` interface allows to implement convenience tools for testing metric collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. #58 Notable non-goals of this commit -------------------------------- The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers in the new `promhttp` package and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. Cf. #200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. #47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. An unfortunate side effect is that the fields of the current `Desc` had to be exported for the time being. This will be solved in a cleaner way with the upcoming `Desc` changes. Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection, and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion. What are the breaking changes? ------------------------------ - Signature of functions pushing to Pushgateway (long planned anyway, and now that I worked on it anyway, I did it, cf. #100). - `SetMetricFamilyInjectionHook is gone`. A registry with a MetricFamily injection hook has to be created now with `registry.NewWithInjectionHook`. - `PanicOnCollectError` is gone. This behavior can now be configured when creating a `promhttp.Handler`. - `EnableCollectChecks` is gone. A registry with those checks can now be created with `registry.NewTestRegistry`.

General context and approch --------------------------- This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? -------------------------------- This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ to fat. This commit introduces a very lean `registry.Registry` interface. Most of the existing functionality that is not part of that interface is provided by utility functions (in `registry/util.go`). The top-level function that act on the default registry are mostly retained and kept in the `prometheus` package, to minimize breakage and for convenience. The general idea is here that the basic use case isn't involved with the `registry` package at all. Only if you are going for one of the advanced use cases, you need to import the `registry` package. The default registry is kept in the public variable `registry.Default`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`). Another important part in making the registry lean is the extraction of the HTTP exposition into its own package `promhttp`. Note that the package is not simply called `http` because that would collide with the `http` package from the standard library, which will usually be used in the same source file as the `promhttp` package. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): #46 #100 #170 #205 What future changes does this commit enable? -------------------------------------------- - The separation of the HTTP exposition allows the implementation of other exposition methods as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. #197 - The public `Registry` interface allows to implement convenience tools for testing metric collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. #58 Notable non-goals of this commit -------------------------------- The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers in the new `promhttp` package and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. Cf. #200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. #47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. An unfortunate side effect is that the fields of the current `Desc` had to be exported for the time being. This will be solved in a cleaner way with the upcoming `Desc` changes. Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection, and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion. What are the breaking changes? ------------------------------ - Signature of functions pushing to Pushgateway has changed to allow arbitrary grouping (long planned anyway, and now that I worked on it anyway, I did it, cf. #100). Also, the PushCollectors and PushAddCollectors has been moved into the promhttp package, where the new Push and PushAdd functions for custom registries live, too. - `SetMetricFamilyInjectionHook` is gone. A registry with a MetricFamily injection hook has to be created now with `registry.NewWithInjectionHook`. - `PanicOnCollectError` is gone. This behavior can now be configured when creating a `promhttp.Handler`. - `EnableCollectChecks` is gone. A registry with those checks can now be created with `registry.NewTestRegistry` (it is only ever used to test custom Collectors). - `NewProcessCollector`, `NewGoCollector`, and `NewExpvarCollector` have been moved into their own package `collectors`.

General context and approch =========================== This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? ================================ This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ too fat. This commit introduces a very lean `Registry` interface. Most of the existing functionality that is not part of that interface is provided by helper functions, not by methods (e.g. `MustRegisterWith`). The functions that act on the default registry are retained (with very few exceptions) so that most use cases won't see a change. The default registry is kept in the public variable `DefaultRegistry`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`) with the same implications. Another important part in making the registry lean is the extraction of the HTTP exposition, which also allows for customization of the HTTP exposition. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): #46 #100 #170 #205 What future changes does this commit enable? ============================================ The following items are not yet implemented, but this commit opens the possibility of implementing these independently. - The separation of the HTTP exposition allows the implementation of other exposition methods based on the Registry interface, as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. #197 - The public `Registry` interface allows the implementation of convenience tools for testing metrics collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. #58 Notable non-goals of this commit ================================ Non-goals that will be tackled later ------------------------------------ The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. Cf. #200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. #47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. Non-goals that _might_ be tackled later --------------------------------------- There is a strong and understandable urge to divide the `prometheus` package into a number of sub-packages (like `registry`, `collectors`, `http`, `metrics`, …). However, to not run into a multitude of circular import chains, this would need to break every single existing usage of the library. (As just one example, if the ubiquitious `prometheus.MustRegister` (with more than 2,000 uses on GitHub alone) is kept in the `prometheus` package, but the other registry concerns go into a new `registry` package, then the `prometheus` package would import the `registry` package (to call the actual register method), while at the same time the `registry` package needs to import the `prometheus` package to access `Collector`, `Metric`, `Desc` and more. If we moved `MustRegister` into the `registry` package, thousands of code lines would have to be fixed (which would be easy if the world was a mono repo, but it is not).) The main problem is really the top-level functions like `MustRegister`, `Handler`, `Push`, …, which effectively pull everything into one package. Those functions are however very convenient for the easy and very frequent use-cases. This problem has to be revisited later. Non-goals that won't be tackled anytime soon -------------------------------------------- Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection, and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion. What are the breaking changes? ============================== - Signature of functions pushing to Pushgateway has changed to allow arbitrary grouping (long planned anyway, and now that I worked on it anyway, I did it, cf. #100). - `SetMetricFamilyInjectionHook` is gone. A registry with a MetricFamily injection hook has to be created now with `NewRegistryWithInjectionHook`. - `PanicOnCollectError` is gone. This behavior can now be configured when creating a custom HTTP handler. - `EnableCollectChecks` is gone. A registry with those checks can now be created with `NewPedanticRegistry` (it is only ever used to test custom Collectors).

General context and approch =========================== This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? ================================ This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ too fat. This commit introduces a very lean `Registry` interface. Most of the existing functionality that is not part of that interface is provided by helper functions, not by methods (e.g. `MustRegisterWith`). The functions that act on the default registry are retained (with very few exceptions) so that most use cases won't see a change. The default registry is kept in the public variable `DefaultRegistry`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`) with the same implications. Another important part in making the registry lean is the extraction of the HTTP exposition, which also allows for customization of the HTTP exposition. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): #46 #100 #170 #205 What future changes does this commit enable? ============================================ The following items are not yet implemented, but this commit opens the possibility of implementing these independently. - The separation of the HTTP exposition allows the implementation of other exposition methods based on the Registry interface, as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. #197 - The public `Registry` interface allows the implementation of convenience tools for testing metrics collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. #58 Notable non-goals of this commit ================================ Non-goals that will be tackled later ------------------------------------ The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. Cf. #200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. #47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. Non-goals that _might_ be tackled later --------------------------------------- There is a strong and understandable urge to divide the `prometheus` package into a number of sub-packages (like `registry`, `collectors`, `http`, `metrics`, …). However, to not run into a multitude of circular import chains, this would need to break every single existing usage of the library. (As just one example, if the ubiquitious `prometheus.MustRegister` (with more than 2,000 uses on GitHub alone) is kept in the `prometheus` package, but the other registry concerns go into a new `registry` package, then the `prometheus` package would import the `registry` package (to call the actual register method), while at the same time the `registry` package needs to import the `prometheus` package to access `Collector`, `Metric`, `Desc` and more. If we moved `MustRegister` into the `registry` package, thousands of code lines would have to be fixed (which would be easy if the world was a mono repo, but it is not).) The main problem is really the top-level functions like `MustRegister`, `Handler`, `Push`, …, which effectively pull everything into one package. Those functions are however very convenient for the easy and very frequent use-cases. This problem has to be revisited later. For now, I'm trying to keep the amount of exported names in the package as low as possible (e.g. I unexported expvarCollector in this commit because the NewExpvarCollector constructor is enough to export, similar to the other exporters). Non-goals that won't be tackled anytime soon -------------------------------------------- Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection, and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion. What are the breaking changes? ============================== - Signature of functions pushing to Pushgateway has changed to allow arbitrary grouping (long planned anyway, and now that I worked on it anyway, I did it, cf. #100). - `SetMetricFamilyInjectionHook` is gone. A registry with a MetricFamily injection hook has to be created now with `NewRegistryWithInjectionHook`. - `PanicOnCollectError` is gone. This behavior can now be configured when creating a custom HTTP handler. - `EnableCollectChecks` is gone. A registry with those checks can now be created with `NewPedanticRegistry` (it is only ever used to test custom Collectors).

General context and approch =========================== This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? ================================ This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ too fat. This commit introduces a very lean `Registry` interface. Most of the existing functionality that is not part of that interface is provided by helper functions, not by methods (e.g. `MustRegisterWith`). The functions that act on the default registry are retained (with very few exceptions) so that most use cases won't see a change. The default registry is kept in the public variable `DefaultRegistry`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`) with the same implications. Another important part in making the registry lean is the extraction of the HTTP exposition, which also allows for customization of the HTTP exposition. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): #46 #100 #170 #205 What future changes does this commit enable? ============================================ The following items are not yet implemented, but this commit opens the possibility of implementing these independently. - The separation of the HTTP exposition allows the implementation of other exposition methods based on the Registry interface, as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. #197 - The public `Registry` interface allows the implementation of convenience tools for testing metrics collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. #58 Notable non-goals of this commit ================================ Non-goals that will be tackled later ------------------------------------ The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. Cf. #200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. #47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. Non-goals that _might_ be tackled later --------------------------------------- There is a strong and understandable urge to divide the `prometheus` package into a number of sub-packages (like `registry`, `collectors`, `http`, `metrics`, …). However, to not run into a multitude of circular import chains, this would need to break every single existing usage of the library. (As just one example, if the ubiquitious `prometheus.MustRegister` (with more than 2,000 uses on GitHub alone) is kept in the `prometheus` package, but the other registry concerns go into a new `registry` package, then the `prometheus` package would import the `registry` package (to call the actual register method), while at the same time the `registry` package needs to import the `prometheus` package to access `Collector`, `Metric`, `Desc` and more. If we moved `MustRegister` into the `registry` package, thousands of code lines would have to be fixed (which would be easy if the world was a mono repo, but it is not).) The main problem is really the top-level functions like `MustRegister`, `Handler`, `Push`, …, which effectively pull everything into one package. Those functions are however very convenient for the easy and very frequent use-cases. This problem has to be revisited later. For now, I'm trying to keep the amount of exported names in the package as low as possible (e.g. I unexported expvarCollector in this commit because the NewExpvarCollector constructor is enough to export, similar to the other exporters). Non-goals that won't be tackled anytime soon -------------------------------------------- Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection, and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion. What are the breaking changes? ============================== - Signature of functions pushing to Pushgateway has changed to allow arbitrary grouping (which was planned for a long time anyway, and now that I worked on it, I finally did it, cf. #100). - `SetMetricFamilyInjectionHook` is gone. A registry with a MetricFamily injection hook has to be created now with `NewRegistryWithInjectionHook`. - `PanicOnCollectError` is gone. This behavior can now be configured when creating a custom HTTP handler. - `EnableCollectChecks` is gone. A registry with those checks can now be created with `NewPedanticRegistry` (it is only ever used to test custom Collectors).

General context and approch =========================== This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? ================================ This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ too fat. This commit introduces a very lean `Registry` interface. Most of the existing functionality that is not part of that interface is provided by helper functions, not by methods (e.g. `MustRegisterWith`). The functions that act on the default registry are retained (with very few exceptions) so that most use cases won't see a change. The default registry is kept in the public variable `DefaultRegistry`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`) with the same implications. Another important part in making the registry lean is the extraction of the HTTP exposition, which also allows for customization of the HTTP exposition. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): #46 #100 #170 #205 What future changes does this commit enable? ============================================ The following items are not yet implemented, but this commit opens the possibility of implementing these independently. - The separation of the HTTP exposition allows the implementation of other exposition methods based on the Registry interface, as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. #197 - The public `Registry` interface allows the implementation of convenience tools for testing metrics collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. #58 Notable non-goals of this commit ================================ Non-goals that will be tackled later ------------------------------------ The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. Cf. #200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. #47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. Non-goals that _might_ be tackled later --------------------------------------- There is a strong and understandable urge to divide the `prometheus` package into a number of sub-packages (like `registry`, `collectors`, `http`, `metrics`, …). However, to not run into a multitude of circular import chains, this would need to break every single existing usage of the library. (As just one example, if the ubiquitious `prometheus.MustRegister` (with more than 2,000 uses on GitHub alone) is kept in the `prometheus` package, but the other registry concerns go into a new `registry` package, then the `prometheus` package would import the `registry` package (to call the actual register method), while at the same time the `registry` package needs to import the `prometheus` package to access `Collector`, `Metric`, `Desc` and more. If we moved `MustRegister` into the `registry` package, thousands of code lines would have to be fixed (which would be easy if the world was a mono repo, but it is not).) The main problem is really the top-level functions like `MustRegister`, `Handler`, `Push`, …, which effectively pull everything into one package. Those functions are however very convenient for the easy and very frequent use-cases. This problem has to be revisited later. For now, I'm trying to keep the amount of exported names in the package as low as possible (e.g. I unexported expvarCollector in this commit because the NewExpvarCollector constructor is enough to export, similar to the other exporters). Non-goals that won't be tackled anytime soon -------------------------------------------- Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection, and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion. What are the breaking changes? ============================== - Signature of functions pushing to Pushgateway has changed to allow arbitrary grouping (which was planned for a long time anyway, and now that I had to work on the Push code anyway for the registry refurbishment, I finally did it, cf. #100). - The registry is doing more consistency checks be default now. Past creators of inconsistent metrics could have masked them by not setting `EnableCollectChecks`. Those inconsistencies will now be detected. (But note that a "best effort" metrics collection is now possible with `HandlerOpts.ErrorHandling = ContinueOnError`.) - `EnableCollectChecks` is gone. The registry is now performing some of those checks anyway (see previous item), and a registry with all of those checks can now be created with `NewPedanticRegistry` (it is only ever needed to test custom Collectors). - `PanicOnCollectError` is gone. This behavior can now be configured when creating a custom HTTP handler. - `SetMetricFamilyInjectionHook` is gone. A registry with a MetricFamily injection hook has to be created now with `NewRegistryWithInjectionHook`.

General context and approch =========================== This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? ================================ This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ too fat. This commit introduces a very lean `Registry` interface. Most of the existing functionality that is not part of that interface is provided by helper functions, not by methods (e.g. `MustRegisterWith`). The functions that act on the default registry are retained (with very few exceptions) so that most use cases won't see a change. The default registry is kept in the public variable `DefaultRegistry`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`) with the same implications. (This pattern is somewhat disputed within the Go community but I chose to go with the devil you know instead of creating something more complex or even disallowing any changes to the default registry. The current approach gives everybody the freedom to not touch DefaultRegistry or to do everything with a custom registry to play save.) Another important part in making the registry lean is the extraction of the HTTP exposition, which also allows for customization of the HTTP exposition. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): #46 #100 #170 #205 Documentation including examples have been amended as required. What future changes does this commit enable? ============================================ The following items are not yet implemented, but this commit opens the possibility of implementing these independently. - The separation of the HTTP exposition allows the implementation of other exposition methods based on the Registry interface, as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. #197 - The public `Registry` interface allows the implementation of convenience tools for testing metrics collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. #58 Notable non-goals of this commit ================================ Non-goals that will be tackled later ------------------------------------ The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. This leads to weird naming inconsistencies but I have deemed it better to not break the world right now but do it in the change that provides better ways of instrumenting HTTP handlers. Cf. #200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. #47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. - While this commit adds new external dependency, the effort to vendor anything within the library that is not visible in any exported types will have to be done later. Non-goals that _might_ be tackled later --------------------------------------- There is a strong and understandable urge to divide the `prometheus` package into a number of sub-packages (like `registry`, `collectors`, `http`, `metrics`, …). However, to not run into a multitude of circular import chains, this would need to break every single existing usage of the library. (As just one example, if the ubiquitious `prometheus.MustRegister` (with more than 2,000 uses on GitHub alone) is kept in the `prometheus` package, but the other registry concerns go into a new `registry` package, then the `prometheus` package would import the `registry` package (to call the actual register method), while at the same time the `registry` package needs to import the `prometheus` package to access `Collector`, `Metric`, `Desc` and more. If we moved `MustRegister` into the `registry` package, thousands of code lines would have to be fixed (which would be easy if the world was a mono repo, but it is not). If we moved everything else the proposed registry package needs into packages of their own, we would break thousands of other code lines.) The main problem is really the top-level functions like `MustRegister`, `Handler`, `Push`, …, which effectively pull everything into one package. Those functions are however very convenient for the easy and very frequent use-cases. This problem has to be revisited later. For now, I'm trying to keep the amount of exported names in the package as low as possible (e.g. I unexported expvarCollector in this commit because the NewExpvarCollector constructor is enough to export, and it is now consistent with other collectors, like the goCollector). Non-goals that won't be tackled anytime soon -------------------------------------------- Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection; and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion (which hasn't been done so far, without causing any trouble, but might be done in the future without breaking any interfaces). What are the breaking changes? ============================== - Signature of functions pushing to Pushgateway has changed to allow arbitrary grouping (which was planned for a long time anyway, and now that I had to work on the Push code anyway for the registry refurbishment, I finally did it, cf. #100). - The registry is doing more consistency checks by default now. Past creators of inconsistent metrics could have masked the problem by not setting `EnableCollectChecks`. Those inconsistencies will now be detected. (But note that a "best effort" metrics collection is now possible with `HandlerOpts.ErrorHandling = ContinueOnError`.) - `EnableCollectChecks` is gone. The registry is now performing some of those checks anyway (see previous item), and a registry with all of those checks can now be created with `NewPedanticRegistry` (it is only ever needed to test custom Collectors). - `PanicOnCollectError` is gone. This behavior can now be configured when creating a custom HTTP handler. - `SetMetricFamilyInjectionHook` is gone. A registry with a MetricFamily injection hook has to be created now with `NewRegistryWithInjectionHook`.

General context and approch =========================== This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? ================================ This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ too fat. This commit introduces a very lean `Registry` interface. Most of the existing functionality that is not part of that interface is provided by helper functions, not by methods (e.g. `MustRegisterWith`). The functions that act on the default registry are retained (with very few exceptions) so that most use cases won't see a change. The default registry is kept in the public variable `DefaultRegistry`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`) with the same implications. (This pattern is somewhat disputed within the Go community but I chose to go with the devil you know instead of creating something more complex or even disallowing any changes to the default registry. The current approach gives everybody the freedom to not touch DefaultRegistry or to do everything with a custom registry to play save.) Another important part in making the registry lean is the extraction of the HTTP exposition, which also allows for customization of the HTTP exposition. Note that the separation of metric collection and exposition has the side effect that managing the MetricFamily and Metric protobuf objects in a free-list or pool isn't really feasible anymore. By now (with better GC in more recent Go versions), the returns were anyway dimisishing. To be effective at all, scrapes had to happen more often than GC cycles, and even then most elements of the protobufs (everything excetp the MetricFamily and Metric structs themselves) would still cause allocation churn. In a future breaking change, the signature of the Write method in the Metric interface will be adjusted accordingly. In this commit, avoiding breakage is more important. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): #46 #100 #170 #205 Documentation including examples have been amended as required. What future changes does this commit enable? ============================================ The following items are not yet implemented, but this commit opens the possibility of implementing these independently. - The separation of the HTTP exposition allows the implementation of other exposition methods based on the Registry interface, as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. #197 - The public `Registry` interface allows the implementation of convenience tools for testing metrics collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. #58 Notable non-goals of this commit ================================ Non-goals that will be tackled later ------------------------------------ The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. This leads to weird naming inconsistencies but I have deemed it better to not break the world right now but do it in the change that provides better ways of instrumenting HTTP handlers. Cf. #200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. #47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. - While this commit adds new external dependency, the effort to vendor anything within the library that is not visible in any exported types will have to be done later. Non-goals that _might_ be tackled later --------------------------------------- There is a strong and understandable urge to divide the `prometheus` package into a number of sub-packages (like `registry`, `collectors`, `http`, `metrics`, …). However, to not run into a multitude of circular import chains, this would need to break every single existing usage of the library. (As just one example, if the ubiquitious `prometheus.MustRegister` (with more than 2,000 uses on GitHub alone) is kept in the `prometheus` package, but the other registry concerns go into a new `registry` package, then the `prometheus` package would import the `registry` package (to call the actual register method), while at the same time the `registry` package needs to import the `prometheus` package to access `Collector`, `Metric`, `Desc` and more. If we moved `MustRegister` into the `registry` package, thousands of code lines would have to be fixed (which would be easy if the world was a mono repo, but it is not). If we moved everything else the proposed registry package needs into packages of their own, we would break thousands of other code lines.) The main problem is really the top-level functions like `MustRegister`, `Handler`, `Push`, …, which effectively pull everything into one package. Those functions are however very convenient for the easy and very frequent use-cases. This problem has to be revisited later. For now, I'm trying to keep the amount of exported names in the package as low as possible (e.g. I unexported expvarCollector in this commit because the NewExpvarCollector constructor is enough to export, and it is now consistent with other collectors, like the goCollector). Non-goals that won't be tackled anytime soon -------------------------------------------- Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection; and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion (which hasn't been done so far, without causing any trouble, but might be done in the future without breaking any interfaces). What are the breaking changes? ============================== - Signature of functions pushing to Pushgateway has changed to allow arbitrary grouping (which was planned for a long time anyway, and now that I had to work on the Push code anyway for the registry refurbishment, I finally did it, cf. #100). - The registry is doing more consistency checks by default now. Past creators of inconsistent metrics could have masked the problem by not setting `EnableCollectChecks`. Those inconsistencies will now be detected. (But note that a "best effort" metrics collection is now possible with `HandlerOpts.ErrorHandling = ContinueOnError`.) - `EnableCollectChecks` is gone. The registry is now performing some of those checks anyway (see previous item), and a registry with all of those checks can now be created with `NewPedanticRegistry` (it is only ever needed to test custom Collectors). - `PanicOnCollectError` is gone. This behavior can now be configured when creating a custom HTTP handler. - `SetMetricFamilyInjectionHook` is gone. A registry with a MetricFamily injection hook has to be created now with `NewRegistryWithInjectionHook`.

General context and approch =========================== This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? ================================ This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ too fat. This commit introduces a very lean `Registry` interface. Most of the existing functionality that is not part of that interface is provided by helper functions, not by methods (e.g. `MustRegisterWith`). The functions that act on the default registry are retained (with very few exceptions) so that most use cases won't see a change. The default registry is kept in the public variable `DefaultRegistry`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`) with the same implications. (This pattern is somewhat disputed within the Go community but I chose to go with the devil you know instead of creating something more complex or even disallowing any changes to the default registry. The current approach gives everybody the freedom to not touch DefaultRegistry or to do everything with a custom registry to play save.) Another important part in making the registry lean is the extraction of the HTTP exposition, which also allows for customization of the HTTP exposition. Note that the separation of metric collection and exposition has the side effect that managing the MetricFamily and Metric protobuf objects in a free-list or pool isn't really feasible anymore. By now (with better GC in more recent Go versions), the returns were anyway dimisishing. To be effective at all, scrapes had to happen more often than GC cycles, and even then most elements of the protobufs (everything excetp the MetricFamily and Metric structs themselves) would still cause allocation churn. In a future breaking change, the signature of the Write method in the Metric interface will be adjusted accordingly. In this commit, avoiding breakage is more important. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): #46 #100 #170 #205 Documentation including examples have been amended as required. What future changes does this commit enable? ============================================ The following items are not yet implemented, but this commit opens the possibility of implementing these independently. - The separation of the HTTP exposition allows the implementation of other exposition methods based on the Registry interface, as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. #197 - The public `Registry` interface allows the implementation of convenience tools for testing metrics collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. #58 Notable non-goals of this commit ================================ Non-goals that will be tackled later ------------------------------------ The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. This leads to weird naming inconsistencies but I have deemed it better to not break the world right now but do it in the change that provides better ways of instrumenting HTTP handlers. Cf. #200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. #47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. - While this commit adds new external dependency, the effort to vendor anything within the library that is not visible in any exported types will have to be done later. Non-goals that _might_ be tackled later --------------------------------------- There is a strong and understandable urge to divide the `prometheus` package into a number of sub-packages (like `registry`, `collectors`, `http`, `metrics`, …). However, to not run into a multitude of circular import chains, this would need to break every single existing usage of the library. (As just one example, if the ubiquitious `prometheus.MustRegister` (with more than 2,000 uses on GitHub alone) is kept in the `prometheus` package, but the other registry concerns go into a new `registry` package, then the `prometheus` package would import the `registry` package (to call the actual register method), while at the same time the `registry` package needs to import the `prometheus` package to access `Collector`, `Metric`, `Desc` and more. If we moved `MustRegister` into the `registry` package, thousands of code lines would have to be fixed (which would be easy if the world was a mono repo, but it is not). If we moved everything else the proposed registry package needs into packages of their own, we would break thousands of other code lines.) The main problem is really the top-level functions like `MustRegister`, `Handler`, `Push`, …, which effectively pull everything into one package. Those functions are however very convenient for the easy and very frequent use-cases. This problem has to be revisited later. For now, I'm trying to keep the amount of exported names in the package as low as possible (e.g. I unexported expvarCollector in this commit because the NewExpvarCollector constructor is enough to export, and it is now consistent with other collectors, like the goCollector). Non-goals that won't be tackled anytime soon -------------------------------------------- Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection; and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion (which hasn't been done so far, without causing any trouble, but might be done in the future without breaking any interfaces). What are the breaking changes? ============================== - Signatures of functions pushing to Pushgateway have changed to allow arbitrary grouping (which was planned for a long time anyway, and now that I had to work on the Push code anyway for the registry refurbishment, I finally did it, cf. #100). - The registry is doing more consistency checks by default now. Past creators of inconsistent metrics could have masked the problem by not setting `EnableCollectChecks`. Those inconsistencies will now be detected. (But note that a "best effort" metrics collection is now possible with `HandlerOpts.ErrorHandling = ContinueOnError`.) - `EnableCollectChecks` is gone. The registry is now performing some of those checks anyway (see previous item), and a registry with all of those checks can now be created with `NewPedanticRegistry` (it is only ever needed to test custom Collectors). - `PanicOnCollectError` is gone. This behavior can now be configured when creating a custom HTTP handler. - `SetMetricFamilyInjectionHook` is gone. A registry with a MetricFamily injection hook has to be created now with `NewRegistryWithInjectionHook`.

General context and approch =========================== This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? ================================ This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ too fat. This commit introduces a qutie lean `Registry` interface (compared to the previous interval implementation). The functions that act on the default registry are retained (with very few exceptions) so that most use cases won't see a change. However, several of those are deprecated now to clean up the namespace in the future. The default registry is kept in the public variable `DefaultRegistry`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`) with the same implications. (This pattern is somewhat disputed within the Go community but I chose to go with the devil you know instead of creating something more complex or even disallowing any changes to the default registry. The current approach gives everybody the freedom to not touch DefaultRegistry or to do everything with a custom registry to play save.) Another important part in making the registry lean is the extraction of the HTTP exposition, which also allows for customization of the HTTP exposition. Note that the separation of metric collection and exposition has the side effect that managing the MetricFamily and Metric protobuf objects in a free-list or pool isn't really feasible anymore. By now (with better GC in more recent Go versions), the returns were anyway dimisishing. To be effective at all, scrapes had to happen more often than GC cycles, and even then most elements of the protobufs (everything excetp the MetricFamily and Metric structs themselves) would still cause allocation churn. In a future breaking change, the signature of the Write method in the Metric interface will be adjusted accordingly. In this commit, avoiding breakage is more important. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): #46 #100 #170 #205 Documentation including examples have been amended as required. What future changes does this commit enable? ============================================ The following items are not yet implemented, but this commit opens the possibility of implementing these independently. - The separation of the HTTP exposition allows the implementation of other exposition methods based on the Registry interface, as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. #197 - The public `Registry` interface allows the implementation of convenience tools for testing metrics collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. #58 Notable non-goals of this commit ================================ Non-goals that will be tackled later ------------------------------------ The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. This leads to weird naming inconsistencies but I have deemed it better to not break the world right now but do it in the change that provides better ways of instrumenting HTTP handlers. Cf. #200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. #47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. - While this commit adds new external dependency, the effort to vendor anything within the library that is not visible in any exported types will have to be done later. Non-goals that _might_ be tackled later --------------------------------------- There is a strong and understandable urge to divide the `prometheus` package into a number of sub-packages (like `registry`, `collectors`, `http`, `metrics`, …). However, to not run into a multitude of circular import chains, this would need to break every single existing usage of the library. (As just one example, if the ubiquitious `prometheus.MustRegister` (with more than 2,000 uses on GitHub alone) is kept in the `prometheus` package, but the other registry concerns go into a new `registry` package, then the `prometheus` package would import the `registry` package (to call the actual register method), while at the same time the `registry` package needs to import the `prometheus` package to access `Collector`, `Metric`, `Desc` and more. If we moved `MustRegister` into the `registry` package, thousands of code lines would have to be fixed (which would be easy if the world was a mono repo, but it is not). If we moved everything else the proposed registry package needs into packages of their own, we would break thousands of other code lines.) The main problem is really the top-level functions like `MustRegister`, `Handler`, …, which effectively pull everything into one package. Those functions are however very convenient for the easy and very frequent use-cases. This problem has to be revisited later. For now, I'm trying to keep the amount of exported names in the package as low as possible (e.g. I unexported expvarCollector in this commit because the NewExpvarCollector constructor is enough to export, and it is now consistent with other collectors, like the goCollector). Non-goals that won't be tackled anytime soon -------------------------------------------- Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection; and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion (which hasn't been done so far, without causing any trouble, but might be done in the future without breaking any interfaces). What are the breaking changes? ============================== - Signatures of functions pushing to Pushgateway have changed to allow arbitrary grouping (which was planned for a long time anyway, and now that I had to work on the Push code anyway for the registry refurbishment, I finally did it, cf. #100). With the gained insight that pushing to the default registry is almost never the right thing, and now that we are breaking the Push call anyway, all the Push functions were moved to their own package, which cleans up the namespace and is more idiomatic (pushing Collectors is now literally done by `push.Collectors(...)`). - The registry is doing more consistency checks by default now. Past creators of inconsistent metrics could have masked the problem by not setting `EnableCollectChecks`. Those inconsistencies will now be detected. (But note that a "best effort" metrics collection is now possible with `HandlerOpts.ErrorHandling = ContinueOnError`.) - `EnableCollectChecks` is gone. The registry is now performing some of those checks anyway (see previous item), and a registry with all of those checks can now be created with `NewPedanticRegistry` (only used for testing). - `PanicOnCollectError` is gone. This behavior can now be configured when creating a custom HTTP handler.

beorn7 · 2016-08-19T22:07:31Z

This represents the first part of test tools, those that can be implemented without the more invasive changes in 0.10. For the rest, see #230

beorn7 · 2018-07-03T09:48:44Z

Plan is to adopt the kube-state-metrics testutils package. @brancz will create a PR. See #422 for details.

General context and approch =========================== This is the first part of the long awaited wider refurbishment of `client_golang/prometheus/...`. After a lot of struggling, I decided to not go for one breaking big-bang, but cut things into smaller steps after all, mostly to keep the changes manageable and easy to review. I'm aiming for having the invasive breaking changes concentrated in as few steps as possible (ideally one). Some steps will not be breaking at all, but typically there will be breaking changes that only affect quite special cases so that 95+% of users will not be affected. This first step is an example for that, see details below. What's happening in this commit? ================================ This step is about finally creating an exported registry interface. This could not be done by simply export the existing internal implementation because the interface would be _way_ too fat. This commit introduces a qutie lean `Registry` interface (compared to the previous interval implementation). The functions that act on the default registry are retained (with very few exceptions) so that most use cases won't see a change. However, several of those are deprecated now to clean up the namespace in the future. The default registry is kept in the public variable `DefaultRegistry`. This follows the example of the http package in the standard library (cf. `http.DefaultServeMux`, `http.DefaultClient`) with the same implications. (This pattern is somewhat disputed within the Go community but I chose to go with the devil you know instead of creating something more complex or even disallowing any changes to the default registry. The current approach gives everybody the freedom to not touch DefaultRegistry or to do everything with a custom registry to play save.) Another important part in making the registry lean is the extraction of the HTTP exposition, which also allows for customization of the HTTP exposition. Note that the separation of metric collection and exposition has the side effect that managing the MetricFamily and Metric protobuf objects in a free-list or pool isn't really feasible anymore. By now (with better GC in more recent Go versions), the returns were anyway dimisishing. To be effective at all, scrapes had to happen more often than GC cycles, and even then most elements of the protobufs (everything excetp the MetricFamily and Metric structs themselves) would still cause allocation churn. In a future breaking change, the signature of the Write method in the Metric interface will be adjusted accordingly. In this commit, avoiding breakage is more important. The following issues are fixed by this commit (some solved "on the fly" now that I was touching the code anyway and it would have been stupid to port the bugs): prometheus/client_golang#46 prometheus/client_golang#100 prometheus/client_golang#170 prometheus/client_golang#205 Documentation including examples have been amended as required. What future changes does this commit enable? ============================================ The following items are not yet implemented, but this commit opens the possibility of implementing these independently. - The separation of the HTTP exposition allows the implementation of other exposition methods based on the Registry interface, as known from other Prometheus client libraries, e.g. sending the metrics to Graphite. Cf. prometheus/client_golang#197 - The public `Registry` interface allows the implementation of convenience tools for testing metrics collection. Those tools can inspect the collected MetricFamily protobufs and compare them to expectation. Also, tests can use their own testing instance of a registry. Cf. prometheus/client_golang#58 Notable non-goals of this commit ================================ Non-goals that will be tackled later ------------------------------------ The following two issues are quite closely connected to the changes in this commit but the line has been drawn deliberately to address them in later steps of the refurbishment: - `InstrumentHandler` has many known problems. The plan is to create a saner way to conveniently intrument HTTP handlers and remove the old `InstrumentHandler` altogether. To keep breakage low for now, even the default handler to expose metrics is still using the old `InstrumentHandler`. This leads to weird naming inconsistencies but I have deemed it better to not break the world right now but do it in the change that provides better ways of instrumenting HTTP handlers. Cf. prometheus/client_golang#200 - There is work underway to make the whole handling of metric descriptors (`Desc`) more intuitive and transparent for the user (including an ability for less strict checking, cf. prometheus/client_golang#47). That's quite invasive from the perspective of the internal code, namely the registry. I deliberately kept those changes out of this commit. - While this commit adds new external dependency, the effort to vendor anything within the library that is not visible in any exported types will have to be done later. Non-goals that _might_ be tackled later --------------------------------------- There is a strong and understandable urge to divide the `prometheus` package into a number of sub-packages (like `registry`, `collectors`, `http`, `metrics`, …). However, to not run into a multitude of circular import chains, this would need to break every single existing usage of the library. (As just one example, if the ubiquitious `prometheus.MustRegister` (with more than 2,000 uses on GitHub alone) is kept in the `prometheus` package, but the other registry concerns go into a new `registry` package, then the `prometheus` package would import the `registry` package (to call the actual register method), while at the same time the `registry` package needs to import the `prometheus` package to access `Collector`, `Metric`, `Desc` and more. If we moved `MustRegister` into the `registry` package, thousands of code lines would have to be fixed (which would be easy if the world was a mono repo, but it is not). If we moved everything else the proposed registry package needs into packages of their own, we would break thousands of other code lines.) The main problem is really the top-level functions like `MustRegister`, `Handler`, …, which effectively pull everything into one package. Those functions are however very convenient for the easy and very frequent use-cases. This problem has to be revisited later. For now, I'm trying to keep the amount of exported names in the package as low as possible (e.g. I unexported expvarCollector in this commit because the NewExpvarCollector constructor is enough to export, and it is now consistent with other collectors, like the goCollector). Non-goals that won't be tackled anytime soon -------------------------------------------- Something that I have played with a lot is "streaming collection", i.e. allow an implementation of the `Registry` interface that collects metrics incrementally and serves them while doing so. As it has turned out, this has many many issues and makes the `Registry` interface very clunky. Eventually, I made the call that it is unlikely we will really implement streaming collection; and making the interface more clunky for something that might not even happen is really a big no-no. Note that the `Registry` interface only creates the in-memory representation of the metric family protobufs in one go. The serializaton onto the wire can still be handled in a streaming fashion (which hasn't been done so far, without causing any trouble, but might be done in the future without breaking any interfaces). What are the breaking changes? ============================== - Signatures of functions pushing to Pushgateway have changed to allow arbitrary grouping (which was planned for a long time anyway, and now that I had to work on the Push code anyway for the registry refurbishment, I finally did it, cf. prometheus/client_golang#100). With the gained insight that pushing to the default registry is almost never the right thing, and now that we are breaking the Push call anyway, all the Push functions were moved to their own package, which cleans up the namespace and is more idiomatic (pushing Collectors is now literally done by `push.Collectors(...)`). - The registry is doing more consistency checks by default now. Past creators of inconsistent metrics could have masked the problem by not setting `EnableCollectChecks`. Those inconsistencies will now be detected. (But note that a "best effort" metrics collection is now possible with `HandlerOpts.ErrorHandling = ContinueOnError`.) - `EnableCollectChecks` is gone. The registry is now performing some of those checks anyway (see previous item), and a registry with all of those checks can now be created with `NewPedanticRegistry` (only used for testing). - `PanicOnCollectError` is gone. This behavior can now be configured when creating a custom HTTP handler.

beorn7 self-assigned this Jan 27, 2015

juliusv changed the title ~~Imprvove testability~~ Improve testability Feb 10, 2015

juliusv mentioned this issue Feb 18, 2015

Handle collectd's 'derive' type prometheus/collectd_exporter#6

Merged

discordianfish mentioned this issue Mar 11, 2015

Provide cadvisor stats as prometheus metrics google/cadvisor#545

Merged

matthiasr pushed a commit to matthiasr/node_exporter that referenced this issue May 7, 2015

integration test, part 1

a37e2f6

pending prometheus/client_golang#58, just check for presence of expected metrics

matthiasr mentioned this issue May 11, 2015

Add JSON as a format to the HTTP handler #120

Closed

miekg mentioned this issue Mar 10, 2016

Add tests miekg/caddy-prometheus#7

Closed

beorn7 mentioned this issue Aug 1, 2016

Create a public registry interface and separate out HTTP exposition #214

Merged

beorn7 added this to the v0.9 milestone Aug 19, 2016

beorn7 added the feature request label Aug 19, 2016

AlekSi mentioned this issue Jul 7, 2017

Expose Metric details #323

Closed

beorn7 mentioned this issue Jul 7, 2017

Expose Desc fields #322

Closed

serathius mentioned this issue Mar 7, 2018

Show gomaxprocs on status page prometheus/prometheus#1468

Closed

beorn7 mentioned this issue May 29, 2018

Is there a way to get current counter value? #412

Closed

beorn7 mentioned this issue Jul 3, 2018

Include testing package #422

Closed

beorn7 assigned brancz Jul 3, 2018

brancz mentioned this issue Jul 3, 2018

[WIP] Add testutils package #423

Closed

beorn7 mentioned this issue Sep 4, 2018

Add a single-value extractor and general package doc #454

Merged

beorn7 closed this as completed Sep 4, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve testability #58

Improve testability #58

beorn7 commented Jan 27, 2015

brian-brazil commented Jan 27, 2015

grobie commented May 14, 2015

beorn7 commented Aug 19, 2016

beorn7 commented Jul 3, 2018

Improve testability #58

Improve testability #58

Comments

beorn7 commented Jan 27, 2015

brian-brazil commented Jan 27, 2015

grobie commented May 14, 2015

beorn7 commented Aug 19, 2016

beorn7 commented Jul 3, 2018