Add completely new push syntax #381
Conversation
|
See #341 in particular to see the idea behind this. |
| @@ -67,18 +71,16 @@ func ExampleAddFromGatherer() { | |||
| if err != nil { | |||
| fmt.Println("DB backup failed:", err) | |||
| } else { | |||
| // Only now register successTime. | |||
| registry.MustRegister(successTime) | |||
| // Add succesTime to pusher only in case of success. | |||
| // We could as well register it with the registry. | ||
| // This example, however, demonstrates that you can | ||
| // mix Gatherers and Collectors when handling a Pusher. | ||
| pusher.Collector(successTime) |
There was a problem hiding this comment.
Its not clear to me at first glance exactly what this call is doing, is it pushing or is it registering?
AddCollector may be clearer
There was a problem hiding this comment.
I had considered that name, too, but in the end I decided to go for the approach to use a verb only for the methods that are actually performing the push to the PGW (Push/Add) and use nouns for the methods that configure the push. This also makes the methods shorter. Furthermore, using the verb "add" specifically will easily get confused with the Add method.
Hence, I do thing the current naming is the clearest and easiest to handle. Does somebody else have an opinion on this?
prometheus/push/push.go
Outdated
| return p | ||
| } | ||
|
|
||
| // HostnameGrouping adds a label pair with “instance” as the name and the |
There was a problem hiding this comment.
The reason this existed was to allow for past mis-uses of the pgw when the pgw api changed quite some time ago. Now might be the time to remove it.
There was a problem hiding this comment.
I don't remember anything, but I'm for removing it. As the author of https://prometheus.io/docs/practices/pushing/, I really want to discourage this use :)
There was a problem hiding this comment.
OK, that's quorum. I have removed HostnameGrouping, adjusted the tests and examples, and edited the deprecation notice on the old HostnameGroupingKey.
This allows adding more options in elegant ways, showcased here by
HTTP basic auth and by injecting a custom http.Client.
Fixes #341 and #372.
@grobie @alitari @shlimp