From b428416f06ab075bd71fe870ff5318524747776a Mon Sep 17 00:00:00 2001 From: beorn7 Date: Thu, 19 Oct 2023 17:54:42 +0200 Subject: [PATCH] textparse: Update comment about timestamp_ms protobuf parsing By now, we know better what the plan is. Signed-off-by: beorn7 --- model/textparse/protobufparse.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index 94ea5e4a35..d6d87ee368 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -148,9 +148,15 @@ func (p *ProtobufParser) Series() ([]byte, *int64, float64) { if ts != 0 { return p.metricBytes.Bytes(), &ts, v } - // Nasty hack: Assume that ts==0 means no timestamp. That's not true in - // general, but proto3 has no distinction between unset and - // default. Need to avoid in the final format. + // TODO(beorn7): We assume here that ts==0 means no timestamp. That's + // not true in general, but proto3 originally has no distinction between + // unset and default. At a later stage, the `optional` keyword was + // (re-)introduced in proto3, but gogo-protobuf never got updated to + // support it. (Note that setting `[(gogoproto.nullable) = true]` for + // the `timestamp_ms` field doesn't help, either.) We plan to migrate + // away from gogo-protobuf to an actively maintained protobuf + // implementation. Once that's done, we can simply use the `optional` + // keyword and check for the unset state explicitly. return p.metricBytes.Bytes(), nil, v }