{"push_id":1227600877,"size":1,"distinct_size":1,"ref":"refs/heads/beorn7/registry","head":"189c78a6b109fe3b92f82080f4cc6fe3af8a3677","before":"bca5e4376138a401b425dea44e778f623c11b8fe","commits":[{"sha":"189c78a6b109fe3b92f82080f4cc6fe3af8a3677","author":{"name":"beorn7","email":"f01376397657f6d911db00af04170b21f71dc340@soundcloud.com"},"message":"Create a public registry interface and separate out HTTP exposition\n\nGeneral context and approch\n===========================\n\nThis is the first part of the long awaited wider refurbishment of\n`client_golang/prometheus/...`. After a lot of struggling, I decided\nto not go for one breaking big-bang, but cut things into smaller steps\nafter all, mostly to keep the changes manageable and easy to\nreview. I'm aiming for having the invasive breaking changes\nconcentrated in as few steps as possible (ideally one). Some steps\nwill not be breaking at all, but typically there will be breaking\nchanges that only affect quite special cases so that 95+% of users\nwill not be affected. This first step is an example for that, see\ndetails below.\n\nWhat's happening in this commit?\n================================\n\nThis step is about finally creating an exported registry\ninterface. This could not be done by simply export the existing\ninternal implementation because the interface would be _way_ too\nfat. This commit introduces a very lean `Registry` interface. Most of\nthe existing functionality that is not part of that interface is\nprovided by helper functions, not by methods\n(e.g. `MustRegisterWith`). The functions that act on the default\nregistry are retained (with very few exceptions) so that most use\ncases won't see a change.\n\nThe default registry is kept in the public variable\n`DefaultRegistry`. This follows the example of the http package in the\nstandard library (cf. `http.DefaultServeMux`, `http.DefaultClient`)\nwith the same implications. (This pattern is somewhat disputed within\nthe Go community but I chose to go with the devil you know instead of\ncreating something more complex or even disallowing any changes to the\ndefault registry. The current approach gives everybody the freedom to\nnot touch DefaultRegistry or to do everything with a custom registry\nto play save.)\n\nAnother important part in making the registry lean is the extraction\nof the HTTP exposition, which also allows for customization of the\nHTTP exposition.\n\nThe following issues are fixed by this commit (some solved \"on the\nfly\" now that I was touching the code anyway and it would have been\nstupid to port the bugs):\n\nhttps://github.com/prometheus/client_golang/issues/46\nhttps://github.com/prometheus/client_golang/issues/100\nhttps://github.com/prometheus/client_golang/issues/170\nhttps://github.com/prometheus/client_golang/issues/205\n\nDocumentation including examples have been amended as required.\n\nWhat future changes does this commit enable?\n============================================\n\nThe following items are not yet implemented, but this commit opens the\npossibility of implementing these independently.\n\n- The separation of the HTTP exposition allows the implementation of\n  other exposition methods based on the Registry interface, as known\n  from other Prometheus client libraries, e.g. sending the metrics to\n  Graphite.\n  Cf. https://github.com/prometheus/client_golang/issues/197\n\n- The public `Registry` interface allows the implementation of\n  convenience tools for testing metrics collection. Those tools can\n  inspect the collected MetricFamily protobufs and compare them to\n  expectation. Also, tests can use their own testing instance of a\n  registry.\n  Cf. https://github.com/prometheus/client_golang/issues/58\n\nNotable non-goals of this commit\n================================\n\nNon-goals that will be tackled later\n------------------------------------\n\nThe following two issues are quite closely connected to the changes in\nthis commit but the line has been drawn deliberately to address them\nin later steps of the refurbishment:\n\n- `InstrumentHandler` has many known problems. The plan is to create a\n  saner way to conveniently intrument HTTP handlers and remove the old\n  `InstrumentHandler` altogether. To keep breakage low for now, even\n  the default handler to expose metrics is still using the old\n  `InstrumentHandler`. This leads to weird naming inconsistencies but\n  I have deemed it better to not break the world right now but do it\n  in the change that provides better ways of instrumenting HTTP\n  handlers.\n  Cf. https://github.com/prometheus/client_golang/issues/200\n\n- There is work underway to make the whole handling of metric\n  descriptors (`Desc`) more intuitive and transparent for the user\n  (including an ability for less strict checking,\n  cf. https://github.com/prometheus/client_golang/issues/47). That's\n  quite invasive from the perspective of the internal code, namely the\n  registry. I deliberately kept those changes out of this commit.\n\n- While this commit adds new external dependency, the effort to vendor\n  anything within the library that is not visible in any exported\n  types will have to be done later.\n\nNon-goals that _might_ be tackled later\n---------------------------------------\n\nThere is a strong and understandable urge to divide the `prometheus`\npackage into a number of sub-packages (like `registry`, `collectors`,\n`http`, `metrics`, …). However, to not run into a multitude of\ncircular import chains, this would need to break every single existing\nusage of the library. (As just one example, if the ubiquitious\n`prometheus.MustRegister` (with more than 2,000 uses on GitHub alone)\nis kept in the `prometheus` package, but the other registry concerns\ngo into a new `registry` package, then the `prometheus` package would\nimport the `registry` package (to call the actual register method),\nwhile at the same time the `registry` package needs to import the\n`prometheus` package to access `Collector`, `Metric`, `Desc` and\nmore. If we moved `MustRegister` into the `registry` package,\nthousands of code lines would have to be fixed (which would be easy if\nthe world was a mono repo, but it is not). If we moved everything else\nthe proposed registry package needs into packages of their own, we\nwould break thousands of other code lines.)\n\nThe main problem is really the top-level functions like\n`MustRegister`, `Handler`, `Push`, …, which effectively pull\neverything into one package. Those functions are however very\nconvenient for the easy and very frequent use-cases.\n\nThis problem has to be revisited later.\n\nFor now, I'm trying to keep the amount of exported names in the\npackage as low as possible (e.g. I unexported expvarCollector in this\ncommit because the NewExpvarCollector constructor is enough to export,\nand it is now consistent with other collectors, like the goCollector).\n\nNon-goals that won't be tackled anytime soon\n--------------------------------------------\n\nSomething that I have played with a lot is \"streaming collection\",\ni.e. allow an implementation of the `Registry` interface that collects\nmetrics incrementally and serves them while doing so. As it has turned\nout, this has many many issues and makes the `Registry` interface very\nclunky. Eventually, I made the call that it is unlikely we will really\nimplement streaming collection; and making the interface more clunky\nfor something that might not even happen is really a big no-no. Note\nthat the `Registry` interface only creates the in-memory\nrepresentation of the metric family protobufs in one go. The\nserializaton onto the wire can still be handled in a streaming fashion\n(which hasn't been done so far, without causing any trouble, but might\nbe done in the future without breaking any interfaces).\n\nWhat are the breaking changes?\n==============================\n\n- Signature of functions pushing to Pushgateway has changed to allow\n  arbitrary grouping (which was planned for a long time anyway, and\n  now that I had to work on the Push code anyway for the registry\n  refurbishment, I finally did it,\n  cf. https://github.com/prometheus/client_golang/issues/100).\n\n- The registry is doing more consistency checks by default now. Past\n  creators of inconsistent metrics could have masked the problem by\n  not setting `EnableCollectChecks`. Those inconsistencies will now be\n  detected. (But note that a \"best effort\" metrics collection is now\n  possible with `HandlerOpts.ErrorHandling = ContinueOnError`.)\n\n- `EnableCollectChecks` is gone. The registry is now performing some\n  of those checks anyway (see previous item), and a registry with all\n  of those checks can now be created with `NewPedanticRegistry` (it is\n  only ever needed to test custom Collectors).\n\n- `PanicOnCollectError` is gone. This behavior can now be configured\n  when creating a custom HTTP handler.\n\n- `SetMetricFamilyInjectionHook` is gone. A registry with a\n  MetricFamily injection hook has to be created now with\n  `NewRegistryWithInjectionHook`.","distinct":true,"url":"https://api.github.com/repos/prometheus/client_golang/commits/189c78a6b109fe3b92f82080f4cc6fe3af8a3677"}]}
{"actor":{"display_login":"beorn7"}}