From 5d5acf57446c23f42630c179bb5cac10ee2b6d17 Mon Sep 17 00:00:00 2001 From: Arianna Vespri Date: Tue, 12 Dec 2023 12:40:08 +0100 Subject: [PATCH 1/6] Add unit protobuf parser Signed-off-by: Arianna Vespri --- model/textparse/openmetricsparse.go | 1 - model/textparse/protobufparse.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/model/textparse/openmetricsparse.go b/model/textparse/openmetricsparse.go index bb5075544..72d4e8a4d 100644 --- a/model/textparse/openmetricsparse.go +++ b/model/textparse/openmetricsparse.go @@ -136,7 +136,6 @@ func (p *OpenMetricsParser) Type() ([]byte, MetricType) { // Must only be called after Next returned a unit entry. // The returned byte slices become invalid after the next call to Next. func (p *OpenMetricsParser) Unit() ([]byte, []byte) { - // The Prometheus format does not have units. return p.l.b[p.offsets[0]:p.offsets[1]], p.text } diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index 23afb5c59..9dea097a2 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -272,7 +272,7 @@ func (p *ProtobufParser) Type() ([]byte, MetricType) { // Unit always returns (nil, nil) because units aren't supported by the protobuf // format. func (p *ProtobufParser) Unit() ([]byte, []byte) { - return nil, nil + return p.metricBytes.Bytes(), []byte(p.mf.GetUnit()) } // Comment always returns nil because comments aren't supported by the protobuf From 9fb1e9715ce9871ab95a0c0ed806e9e41a9ac7c9 Mon Sep 17 00:00:00 2001 From: Arianna Vespri Date: Thu, 14 Dec 2023 17:00:52 +0100 Subject: [PATCH 2/6] Go on adding protobuf parsing for unit Signed-off-by: Arianna Vespri --- model/textparse/protobufparse.go | 14 ++++++++++++-- model/textparse/protobufparse_test.go | 2 ++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index 9dea097a2..21329e2a0 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -269,8 +269,9 @@ func (p *ProtobufParser) Type() ([]byte, MetricType) { return n, MetricTypeUnknown } -// Unit always returns (nil, nil) because units aren't supported by the protobuf -// format. +// Unit returns the metric unit in the current entry. +// Must only be called after Next returned a unit entry. +// The returned byte slices become invalid after the next call to Next. func (p *ProtobufParser) Unit() ([]byte, []byte) { return p.metricBytes.Bytes(), []byte(p.mf.GetUnit()) } @@ -418,6 +419,13 @@ func (p *ProtobufParser) Next() (Entry, error) { default: return EntryInvalid, fmt.Errorf("unknown metric type for metric %q: %s", name, p.mf.GetType()) } + unit := p.mf.GetUnit() + fmt.Println("This is the unit:", unit) + if len(unit) > 0 { + if !strings.HasSuffix(name, fmt.Sprintf("_%s_total", unit)) || !strings.HasSuffix(name, fmt.Sprintf("_%s", unit)) || len(name) < len(unit)+1 { + return EntryInvalid, fmt.Errorf("invalid unit for metric %q: %s", name, unit) + } + } p.metricBytes.Reset() p.metricBytes.WriteString(name) @@ -435,6 +443,8 @@ func (p *ProtobufParser) Next() (Entry, error) { if err := p.updateMetricBytes(); err != nil { return EntryInvalid, err } + // case EntryUnit: + // p.state = EntryType // what is the right state and place for this? case EntryHistogram, EntrySeries: if p.redoClassic { p.redoClassic = false diff --git a/model/textparse/protobufparse_test.go b/model/textparse/protobufparse_test.go index e062e64dd..a5b22e360 100644 --- a/model/textparse/protobufparse_test.go +++ b/model/textparse/protobufparse_test.go @@ -58,6 +58,7 @@ metric: < `name: "go_memstats_alloc_bytes_total" help: "Total number of bytes allocated, even if freed." type: COUNTER +unit: "bytes" metric: < counter: < value: 1.546544e+06 @@ -665,6 +666,7 @@ func TestProtobufParse(t *testing.T) { { m: "go_memstats_alloc_bytes_total", help: "Total number of bytes allocated, even if freed.", + unit: "bytes", }, { m: "go_memstats_alloc_bytes_total", From b65021d7a590810164d99afd99a13915e6ea38ec Mon Sep 17 00:00:00 2001 From: Arianna Vespri Date: Sun, 17 Dec 2023 14:09:14 +0100 Subject: [PATCH 3/6] Get conditional right Signed-off-by: Arianna Vespri --- model/textparse/protobufparse.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index 21329e2a0..685ad08f1 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -421,8 +421,13 @@ func (p *ProtobufParser) Next() (Entry, error) { } unit := p.mf.GetUnit() fmt.Println("This is the unit:", unit) + fmt.Println("This is the name:", name) if len(unit) > 0 { - if !strings.HasSuffix(name, fmt.Sprintf("_%s_total", unit)) || !strings.HasSuffix(name, fmt.Sprintf("_%s", unit)) || len(name) < len(unit)+1 { + sfx := fmt.Sprintf("_%s", unit) + if p.mf.GetType() == dto.MetricType_COUNTER { + sfx = fmt.Sprintf("_%s_total", unit) + } + if !strings.HasSuffix(name, sfx) { return EntryInvalid, fmt.Errorf("invalid unit for metric %q: %s", name, unit) } } @@ -443,8 +448,6 @@ func (p *ProtobufParser) Next() (Entry, error) { if err := p.updateMetricBytes(); err != nil { return EntryInvalid, err } - // case EntryUnit: - // p.state = EntryType // what is the right state and place for this? case EntryHistogram, EntrySeries: if p.redoClassic { p.redoClassic = false From 51e78d9a32aefc80eccdd2360a6045cbd734bdd9 Mon Sep 17 00:00:00 2001 From: Arianna Vespri Date: Wed, 20 Dec 2023 09:31:58 +0100 Subject: [PATCH 4/6] Delete debugging lines, amend error message for unit Signed-off-by: Arianna Vespri --- model/textparse/protobufparse.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index cb622cf84..c40176bcc 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -424,15 +424,13 @@ func (p *ProtobufParser) Next() (Entry, error) { return EntryInvalid, fmt.Errorf("unknown metric type for metric %q: %s", name, p.mf.GetType()) } unit := p.mf.GetUnit() - fmt.Println("This is the unit:", unit) - fmt.Println("This is the name:", name) if len(unit) > 0 { sfx := fmt.Sprintf("_%s", unit) if p.mf.GetType() == dto.MetricType_COUNTER { sfx = fmt.Sprintf("_%s_total", unit) } if !strings.HasSuffix(name, sfx) { - return EntryInvalid, fmt.Errorf("invalid unit for metric %q: %s", name, unit) + return EntryInvalid, fmt.Errorf("unit %q not a suffix of metric %q", name, unit) } } p.metricBytes.Reset() From 9a664b515a2f86a313bd70af58de9210c7fcb04b Mon Sep 17 00:00:00 2001 From: Arianna Vespri Date: Wed, 20 Dec 2023 09:41:37 +0100 Subject: [PATCH 5/6] Correct order in error message Signed-off-by: Arianna Vespri --- model/textparse/protobufparse.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index c40176bcc..5ecdbf6ef 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -430,7 +430,7 @@ func (p *ProtobufParser) Next() (Entry, error) { sfx = fmt.Sprintf("_%s_total", unit) } if !strings.HasSuffix(name, sfx) { - return EntryInvalid, fmt.Errorf("unit %q not a suffix of metric %q", name, unit) + return EntryInvalid, fmt.Errorf("unit %q not a suffix of metric %q", unit, name) } } p.metricBytes.Reset() From 8f07f9dd90bd2694acb1ca5476e9d596a1449fd9 Mon Sep 17 00:00:00 2001 From: Arianna Vespri Date: Thu, 28 Dec 2023 15:41:38 +0100 Subject: [PATCH 6/6] Avoid creating string for suffix, consider counters without _total suffix Signed-off-by: Arianna Vespri --- model/textparse/protobufparse.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index 5ecdbf6ef..9ccd00f6e 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -425,11 +425,11 @@ func (p *ProtobufParser) Next() (Entry, error) { } unit := p.mf.GetUnit() if len(unit) > 0 { - sfx := fmt.Sprintf("_%s", unit) - if p.mf.GetType() == dto.MetricType_COUNTER { - sfx = fmt.Sprintf("_%s_total", unit) - } - if !strings.HasSuffix(name, sfx) { + if p.mf.GetType() == dto.MetricType_COUNTER && strings.HasSuffix(name, "_total") { + if !strings.HasSuffix(name[:len(name)-6], unit) || len(name)-6 < len(unit)+1 || name[len(name)-6-len(unit)-1] != '_' { + return EntryInvalid, fmt.Errorf("unit %q not a suffix of counter %q", unit, name) + } + } else if !strings.HasSuffix(name, unit) || len(name) < len(unit)+1 || name[len(name)-len(unit)-1] != '_' { return EntryInvalid, fmt.Errorf("unit %q not a suffix of metric %q", unit, name) } }