Add Prometheus Pushgateway support #33
Conversation
This allows support for additional reporters to share prometheus settings.
This will allow support for Prometheus Pushgateway
gjsduarte
force-pushed
the
feature/pushgateway
branch
from
b860e0c to
b1f6f27
Compare
|
pushgateway? really? why that antipattern? |
|
An anti-pattern indeed. Nevertheless still necessary if you want to use your efemeral applications with the Prometheus godness. |
|
Hey @peavers, thanks a lot for bringing this up! I was asked about pushgateway a few times in the last couple weeks and boom, here you are! 🎉 I'm in the process of releasing all modules for Kamon 2.0 and even though I already merged #32, I'll still need a couple days to give this PR a serious look. Just hang tight a bit and thanks for the patience! |
| this("kamon.prometheus", "pushgateway", httpClientFactory) | ||
|
|
||
| override def reportPeriodSnapshot(snapshot: PeriodSnapshot): Unit = { | ||
| val scrapeDataBuilder = new ScrapeDataBuilder(settings, PrometheusSettings.environmentTags(settings)) |
There was a problem hiding this comment.
Hey @peavers, I was taking a look at the documentation on the pushgateway and it seemed like the data being pushed to the PG has to be the same as what would have been exposed in the scrape endpoint but with this implementation we would be overwriting the data every minute. For example, if a job runs for 5 minutes, each minute we would be pushing what happened in the last minute and that would overwrite the data from the previous minute (at least for the metrics that have already been reported).. is that the intended flow? maybe I misunderstood the docs!
There was a problem hiding this comment.
Hi @ivantopo
We've taken a look at a few other implementations and they seem to be doing the same thing when it comes to collecting and pushing the metrics:
So seems to be the normal way of handling it? Would love to know what you think about this, or if you've got an alternative solution/suggestion we could try out?
Cheers
There was a problem hiding this comment.
Hey @peavers! I think that the pushing part is fine, the only part that doesn't seem to fit correctly is the "when". Let me explain:
- From the Java example (see this section it looks like the intended way to use it is to push the metrics only once after the entire batch job has finished. If this is the desired behavior then
reportPeriodSnapshotshould accumulate all the data it receives and push it out only whenstopis called. - From the .Net example, it seems like all data is being pushed periodically, which looks more similar to what is happening on this PR, just with the exception here we are not accumulating the data.
The part that I find missing is this one, which is the one that ensures that data from the entire run is going to be sent instead of only the data from the last snapshot (in Kamon core we only report the data that was recorded within the last interval instead of the cumulative values from the start of the app, that's why we need to accumulate it for Prometheus). Hope this explains it better! We could also jump on a quick chat to hash this out and finally get this merged!
|
Any progress on this PR? |
Depends on #32