From e9d9bb1b083aa8b199a945e722a11807b84bcf78 Mon Sep 17 00:00:00 2001 From: beorn7 Date: Thu, 5 Jan 2023 15:21:18 +0100 Subject: [PATCH 1/2] textparse: Handle unknown metric types in protobuf gracefully So far, the parser hasn't validated that the type is valid in the `Next()` call. Later, in the `Series()` call, however, it assumes that we will only see valid types and therefore panics with `encountered unexpected metric type, this is a bug`. This commit fixes said bug by adding validation to the `Next()` call. Signed-off-by: beorn7 --- model/textparse/protobufparse.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index 37c6f0ebb..d18afa04d 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -331,7 +331,7 @@ func (p *ProtobufParser) Next() (Entry, error) { } // We are at the beginning of a metric family. Put only the name - // into metricBytes and validate only name and help for now. + // into metricBytes and validate only name, help, and type for now. name := p.mf.GetName() if !model.IsValidMetricName(model.LabelValue(name)) { return EntryInvalid, errors.Errorf("invalid metric name: %s", name) @@ -339,6 +339,16 @@ func (p *ProtobufParser) Next() (Entry, error) { if help := p.mf.GetHelp(); !utf8.ValidString(help) { return EntryInvalid, errors.Errorf("invalid help for metric %q: %s", name, help) } + switch p.mf.GetType() { + case dto.MetricType_COUNTER, + dto.MetricType_GAUGE, + dto.MetricType_HISTOGRAM, + dto.MetricType_SUMMARY, + dto.MetricType_UNTYPED: + // All good. + default: + return EntryInvalid, errors.Errorf("unknown metric type for metric %q: %s", name, p.mf.GetType()) + } p.metricBytes.Reset() p.metricBytes.WriteString(name) From b5d4a94e9d686c1c208106f481c33087ee94e94d Mon Sep 17 00:00:00 2001 From: beorn7 Date: Thu, 5 Jan 2023 15:39:10 +0100 Subject: [PATCH 2/2] textparse: Add gauge histogram support to protobuf parsing With this commit, the parser stops to see a gauge histogram (whether native or conventional) as an unexpected metric type. It ingests it normally, it even sets the `GaugeHistogram` type in the metadata (as it has already done for a conventional gauge histogram scraped using OpenMetrics), but it otherwise treats it as a normal counter-like histogram. Once #11783 is merged, though, it should be very easy to utilize the type information. Signed-off-by: beorn7 --- model/textparse/protobufparse.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index d18afa04d..003f88456 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -105,7 +105,7 @@ func (p *ProtobufParser) Series() ([]byte, *int64, float64) { default: v = s.GetQuantile()[p.fieldPos].GetValue() } - case dto.MetricType_HISTOGRAM: + case dto.MetricType_HISTOGRAM, dto.MetricType_GAUGE_HISTOGRAM: // This should only happen for a legacy histogram. h := m.GetHistogram() switch p.fieldPos { @@ -225,6 +225,8 @@ func (p *ProtobufParser) Type() ([]byte, MetricType) { return n, MetricTypeGauge case dto.MetricType_HISTOGRAM: return n, MetricTypeHistogram + case dto.MetricType_GAUGE_HISTOGRAM: + return n, MetricTypeGaugeHistogram case dto.MetricType_SUMMARY: return n, MetricTypeSummary } @@ -273,7 +275,7 @@ func (p *ProtobufParser) Exemplar(ex *exemplar.Exemplar) bool { switch p.mf.GetType() { case dto.MetricType_COUNTER: exProto = m.GetCounter().GetExemplar() - case dto.MetricType_HISTOGRAM: + case dto.MetricType_HISTOGRAM, dto.MetricType_GAUGE_HISTOGRAM: bb := m.GetHistogram().GetBucket() if p.fieldPos < 0 { if p.state == EntrySeries { @@ -343,6 +345,7 @@ func (p *ProtobufParser) Next() (Entry, error) { case dto.MetricType_COUNTER, dto.MetricType_GAUGE, dto.MetricType_HISTOGRAM, + dto.MetricType_GAUGE_HISTOGRAM, dto.MetricType_SUMMARY, dto.MetricType_UNTYPED: // All good. @@ -356,7 +359,8 @@ func (p *ProtobufParser) Next() (Entry, error) { case EntryHelp: p.state = EntryType case EntryType: - if p.mf.GetType() == dto.MetricType_HISTOGRAM && + t := p.mf.GetType() + if (t == dto.MetricType_HISTOGRAM || t == dto.MetricType_GAUGE_HISTOGRAM) && isNativeHistogram(p.mf.GetMetric()[0].GetHistogram()) { p.state = EntryHistogram } else { @@ -366,8 +370,11 @@ func (p *ProtobufParser) Next() (Entry, error) { return EntryInvalid, err } case EntryHistogram, EntrySeries: + t := p.mf.GetType() if p.state == EntrySeries && !p.fieldsDone && - (p.mf.GetType() == dto.MetricType_SUMMARY || p.mf.GetType() == dto.MetricType_HISTOGRAM) { + (t == dto.MetricType_SUMMARY || + t == dto.MetricType_HISTOGRAM || + t == dto.MetricType_GAUGE_HISTOGRAM) { p.fieldPos++ } else { p.metricPos++ @@ -428,7 +435,7 @@ func (p *ProtobufParser) getMagicName() string { if p.fieldPos == -1 { return p.mf.GetName() + "_sum" } - if t == dto.MetricType_HISTOGRAM { + if t == dto.MetricType_HISTOGRAM || t == dto.MetricType_GAUGE_HISTOGRAM { return p.mf.GetName() + "_bucket" } return p.mf.GetName() @@ -446,7 +453,7 @@ func (p *ProtobufParser) getMagicLabel() (bool, string, string) { q := qq[p.fieldPos] p.fieldsDone = p.fieldPos == len(qq)-1 return true, model.QuantileLabel, formatOpenMetricsFloat(q.GetQuantile()) - case dto.MetricType_HISTOGRAM: + case dto.MetricType_HISTOGRAM, dto.MetricType_GAUGE_HISTOGRAM: bb := p.mf.GetMetric()[p.metricPos].GetHistogram().GetBucket() if p.fieldPos >= len(bb) { p.fieldsDone = true