Conversation
metrics.proto
Outdated
| message Exemplar { | ||
| repeated LabelPair label = 1; | ||
| optional double value = 2; | ||
| optional double timestamp = 3; // OpenMetrics-style: seconds as float. |
There was a problem hiding this comment.
OM is using the timestamp from google/protobuf/timestamp.proto
There was a problem hiding this comment.
(Not that you need to follow that, Prometheus wouldn't presumably support more than millisecond anyway)
There was a problem hiding this comment.
Good point. That might even simplify things. I'll do a bit of research before merging this.
There was a problem hiding this comment.
If you just want to get something working for now, this is fine. If you want to fully support OpenMetrics longer term then nanoseconds should be supported even if we never end up using it in Prometheus itself.
There was a problem hiding this comment.
I don't think that this proto file will ever be considered as a real OpenMetrics representation. (The proto definition in the OM repo look quite different.) Since here, the timestamp is a float, nanosecend precision can be provided (and it is in my PoC implementation in expfmt). Nevertheless, consistency is good. I'll play with the timestamp.proto.
There was a problem hiding this comment.
Nanosecond precision can't be done with a float64, you can only get to microsecond.
There was a problem hiding this comment.
Yes, right. We live too far away from 1970-01-01…
This makes metrics.proto a bit more OpenMetrics-like. Note that this is not an attempt to make metrics.proto a full and valid representation of OpenMetrics. It's merely added here so that v1 of prometheus/client_golang (which uses metrics.proto as the internal data model) can expose OpenMetrics text format. Signed-off-by: beorn7 <[email protected]>
beorn7
force-pushed
the
beorn7/exemplars
branch
from
3921c38 to
a2bc88f
Compare
|
Changed timestamp to google/protobuf/timestamp.proto. PHAL. |
3 participants
We need it here because client_golang uses this as the data model.
(There are also changes necessary in prometheus/common. This is just the start. I need this merged and tagged for proper Go Module support in the upcoming PR in prometheus/common, and then the same for the upcoming PR in prometheus/client_golang.)