Hi,

I am aware that there is work being done to approve a plain text format for 
native histograms 
(see https://github.com/prometheus/client_python/issues/918 for instance). 
But it is also my understanding that it's not approved yet, and that it 
would then need to be implemented in the whole stack before it becomes 
usable.

As a result, I started working on adding protobuf support to the python 
client which would open the door to adding native histograms.

I wanted to see what I was fighting against so I already started an 
implementation, and have a proof of concept for a client that will ship 
data in protobuf delimited format, similar to what the golang client does.

I'm at the point where it makes sense to gather feedback from this group, 
to see if this is something that we would be interested to work on and 
actually merge, in one form or another, into the actual client, or if this 
is something that will stay a side project for me, and potentially be used 
at my work while we wait for the plain text implementation.

I can quickly explain what I've done:
- imported the metrics.proto 
<https://github.com/prometheus/prometheus/blob/30967330ca038ebae04f9c23c1ed1905b171894e/prompb/io/prometheus/client/metrics.proto>
 
file found in the prometheus repo. I removed references to gogoproto, and 
added a bunch of `optional` qualifiers to make some edge cases work better 
(mainly passing existing tests if we're honest)
- the new proto classes are then used extensively in metrics.py 
<https://github.com/prometheus/client_python/blob/master/prometheus_client/metrics.py>
 
and metrics_core.py 
<https://github.com/prometheus/client_python/blob/master/prometheus_client/metrics_core.py>
- since I was going for minimal changes, I kept the underlying 
Sample/Timestamp/... classes found in samples.py 
<https://github.com/prometheus/client_python/blob/master/prometheus_client/samples.py>
 The 
obvious benefit for me is that it meant not having to rewrite all the plain 
text generators, and just having to make sure that I could translate 
protobuf classes into `Sample`.
- from there, marshalling the new samples into binary strings to send over 
the wire is fairly trivial

The BIG hurdle in my current implementation is that we currently offer the 
ability (mainly, hopefully exclusively?, used in tests) to manually 
override the content of `Metric.samples`. This is of course a problem for 
me as I'm storing everything as a protobuf `MetricFamily`, and only 
generate samples on the fly if I need to.
Because I wasn't sure of the implications, I decided (again, as a first 
POC), to keep a `Metric._samples` list available. You can still manually 
set it (it's tied to the `.sample` property). Which has the side effect of 
also clearing all the protobuf metrics currently stored in the object, and 
if you ask for `Metric.samples` I return both.

There are a couple other side effects. The main one being that 
`Metric.timestamp_ms` in protobuf is, as the name indicates, in ms (int). 
Other timestamps (exemplars, created) are stored as ns. It does mean that 
in my version, the timestamp of a metric is no longer in nanosecond 
resolution. Which translates to the plain text outputs. There are other 
small changes in the output for similar reasons (underlying protobuf types) 
but we can talk about these later.

I feel like that enough text for a first message to this group.
Looking forward to hearing from y'all. I need to clean up my work a bit, 
but if that's helpful I'm of course happy to push this to a public branch 
and share it here if requested.

Thanks!

-- 
You received this message because you are subscribed to the Google Groups 
"Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to prometheus-developers+unsubscr...@googlegroups.com.
To view this discussion visit 
https://groups.google.com/d/msgid/prometheus-developers/48079c9b-d696-43e8-b54f-3c084d824a99n%40googlegroups.com.

Reply via email to