Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Prometheus] Add unit tests for parsing metric families #36669

Merged
merged 10 commits into from
Oct 3, 2023
Merged

[Prometheus] Add unit tests for parsing metric families #36669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9f57d4a
Add tests for parsing metrics.
constanca-m Sep 25, 2023
b7b76eb
Merge branch 'main' into add-prometheus-tests
constanca-m Sep 25, 2023
4a2c943
Remove _info suffix.
constanca-m Sep 25, 2023
b5153df
Add unit tests
constanca-m Sep 29, 2023
1370449
Add unit tests
constanca-m Sep 29, 2023
a0b459c
Add unit tests
constanca-m Sep 29, 2023
c140872
Add license header
constanca-m Sep 29, 2023
82050d8
Fix goimports.
constanca-m Sep 29, 2023
3d32656
Fix openmetrics tests.
constanca-m Sep 29, 2023
76cf65e
Merge branch 'main' into add-prometheus-tests
constanca-m Oct 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 8 additions & 12 deletions metricbeat/helper/openmetrics/openmetrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ metrics_one_count_total{name="john",surname="williams"} 2
metrics_one_count_total{name="jahn",surname="baldwin",age="30"} 3
`

openMetricsCounterKeyLabelWithNaNInf = `# TYPE metrics_one_count_errors counter
metrics_one_count_errors{name="jane",surname="foster"} 1
openMetricsCounterKeyLabelWithNaNInf = `# TYPE metrics_one_count_errors_total counter
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to this, counter types MUST have the suffix _total. I had to fix this to pass the test.

metrics_one_count_errors_total{name="jane",surname="foster"} 1
# TYPE metrics_one_count_total counter
metrics_one_count_total{name="jane",surname="foster"} NaN
metrics_one_count_total{name="john",surname="williams"} +Inf
Expand Down Expand Up @@ -537,22 +537,20 @@ func TestOpenMetrics(t *testing.T) {
msg: "Info metric",
mapping: &MetricsMapping{
Metrics: map[string]MetricMap{
"target_info": Metric("target_info.metric"),
"target": Metric("target"),
},
},
expected: []mapstr.M{
mapstr.M{
"target_info": mapstr.M{
"metric": int64(1),
},
"target": int64(1),
},
},
},
{
msg: "Info metric with labels",
mapping: &MetricsMapping{
Metrics: map[string]MetricMap{
"target_with_labels_info": Metric("target_with_labels_info.metric"),
"target_with_labels": Metric("target_with_labels"),
},
Labels: map[string]LabelMap{
"env": Label("labels.env"),
Expand All @@ -561,9 +559,7 @@ func TestOpenMetrics(t *testing.T) {
},
expected: []mapstr.M{
mapstr.M{
"target_with_labels_info": mapstr.M{
"metric": int64(1),
},
"target_with_labels": int64(1),
"labels": mapstr.M{
"env": "prod",
"hostname": "myhost",
Expand Down Expand Up @@ -747,8 +743,8 @@ func TestOpenMetricsKeyLabels(t *testing.T) {
openmetricsResponse: openMetricsCounterKeyLabelWithNaNInf,
mapping: &MetricsMapping{
Metrics: map[string]MetricMap{
"metrics_one_count_errors": Metric("metrics.one.count"),
"metrics_one_count_total": Metric("metrics.one.count"),
"metrics_one_count_errors_total": Metric("metrics.one.count"),
"metrics_one_count_total": Metric("metrics.one.count"),
},
Labels: map[string]LabelMap{
"name": KeyLabel("metrics.one.labels.name"),
Expand Down
88 changes: 0 additions & 88 deletions metricbeat/helper/prometheus/prometheus_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ import (
"net/http"
"sort"
"testing"
"time"

"github.com/prometheus/prometheus/pkg/labels"

"github.com/stretchr/testify/assert"

Expand Down Expand Up @@ -1005,88 +1002,3 @@ func TestPrometheusKeyLabels(t *testing.T) {
}
}
}

func TestPrometheusHistogramWithoutSuffix(t *testing.T) {
promHistogramWithAndWithoutBucket := `
# HELP http_server_requests_seconds Duration of HTTP server request handling
# TYPE http_server_requests_seconds histogram
http_server_requests_seconds{exception="None",uri="/actuator/prometheus",quantile="0.002796201",} 0.046137344
http_server_requests_seconds{exception="None",uri="/actuator/prometheus",quantile="0.003145726",} 0.046137344
http_server_requests_seconds_bucket{exception="None",uri="/actuator/prometheus",le="0.001",} 0.0
http_server_requests_seconds_bucket{exception="None",uri="/actuator/prometheus",le="0.001048576",} 0.0
http_server_requests_seconds_count{exception="None",uri="/actuator/prometheus",} 1.0
http_server_requests_seconds_sum{exception="None",uri="/actuator/prometheus",} 0.046745444
http_server_requests_seconds_impossible{exception="None",uri="/actuator/prometheus",} 0.046745444
http_server_requests_seconds_created{exception="None",uri="/actuator/prometheus",} 0.046745444
`

labelsList := []*labels.Label{
{
Name: "exception",
Value: "None",
},
{
Name: "uri",
Value: "/actuator/prometheus",
},
}

cumulativeCounts := []uint64{
uint64(0.0),
uint64(0.0),
}
upperBounds := []float64{
0.001,
0.001048576,
}
var buckets []*Bucket
for i := range cumulativeCounts {
buckets = append(buckets, &Bucket{
CumulativeCount: &cumulativeCounts[i],
UpperBound: &upperBounds[i],
})
}

name := "http_server_requests_seconds"
help := "Duration of HTTP server request handling"

var expectedMetric OpenMetric
expectedMetric.Name = &name
expectedMetric.Label = labelsList
sampleSum := 0.046745444
sampleCount := uint64(1.0)
expectedMetric.Histogram = &Histogram{
IsGaugeHistogram: false,
SampleSum: &sampleSum,
SampleCount: &sampleCount,
Bucket: buckets,
}

expectedMetrics := []*OpenMetric{
&expectedMetric,
}

type Histogram struct {
SampleCount *uint64
SampleSum *float64
Bucket []*Bucket
IsGaugeHistogram bool
}

expected := []*MetricFamily{
{
Name: &name,
Help: &help,
Type: "histogram",
Unit: nil,
Metric: expectedMetrics,
},
}

b := []byte(promHistogramWithAndWithoutBucket)
result, err := ParseMetricFamilies(b, ContentTypeTextFormat, time.Now())
if err != nil {
t.Fatal("ParseMetricFamilies returned an error.")
}
assert.Equal(t, expected, result)
}
100 changes: 65 additions & 35 deletions metricbeat/helper/prometheus/textparse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,19 +318,25 @@ func (m *MetricFamily) GetMetric() []*OpenMetric {
}

const (
suffixTotal = "_total"
suffixGCount = "_gcount"
suffixGSum = "_gsum"
suffixCount = "_count"
suffixSum = "_sum"
suffixBucket = "_bucket"
suffixTotal = "_total"
suffixGCount = "_gcount"
suffixGSum = "_gsum"
suffixCount = "_count"
suffixSum = "_sum"
suffixBucket = "_bucket"
suffixCreated = "_created"
suffixInfo = "_info"
)

// Counters have _total suffix
func isTotal(name string) bool {
return strings.HasSuffix(name, suffixTotal)
}

func isCreated(name string) bool {
return strings.HasSuffix(name, suffixCreated)
}

func isGCount(name string) bool {
return strings.HasSuffix(name, suffixGCount)
}
Expand All @@ -351,7 +357,11 @@ func isBucket(name string) bool {
return strings.HasSuffix(name, suffixBucket)
}

func summaryMetricName(name string, s float64, qv string, lbls string, t *int64, summariesByName map[string]map[string]*OpenMetric) (string, *OpenMetric) {
func isInfo(name string) bool {
return strings.HasSuffix(name, suffixInfo)
}

func summaryMetricName(name string, s float64, qv string, lbls string, summariesByName map[string]map[string]*OpenMetric) (string, *OpenMetric) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameter t wasn't being used, that is why it is removed.

var summary = &Summary{}
var quantile = []*Quantile{}
var quant = &Quantile{}
Expand All @@ -360,10 +370,10 @@ func summaryMetricName(name string, s float64, qv string, lbls string, t *int64,
case isCount(name):
u := uint64(s)
summary.SampleCount = &u
name = name[:len(name)-6]
name = strings.TrimSuffix(name, suffixCount)
case isSum(name):
summary.SampleSum = &s
name = name[:len(name)-4]
name = strings.TrimSuffix(name, suffixSum)
default:
f, err := strconv.ParseFloat(qv, 64)
if err != nil {
Expand All @@ -373,8 +383,8 @@ func summaryMetricName(name string, s float64, qv string, lbls string, t *int64,
quant.Value = &s
}

_, k := summariesByName[name]
if !k {
_, ok := summariesByName[name]
if !ok {
summariesByName[name] = make(map[string]*OpenMetric)
}
metric, ok := summariesByName[name][lbls]
Expand All @@ -392,7 +402,6 @@ func summaryMetricName(name string, s float64, qv string, lbls string, t *int64,
} else if quant.Quantile != nil {
metric.Summary.Quantile = append(metric.Summary.Quantile, quant)
}

return name, metric
}

Expand Down Expand Up @@ -499,8 +508,6 @@ func ParseMetricFamilies(b []byte, contentType string, ts time.Time) ([]*MetricF
fam = &MetricFamily{Name: &s, Type: t}
metricFamiliesByName[s] = fam
} else {
// In case the metric family already exists, we need to make sure the type is correctly defined
// instead of being `unknown` like it was initialized for the other entry types (help and unit).
fam.Type = t
}
mt = t
Expand All @@ -511,21 +518,23 @@ func ParseMetricFamilies(b []byte, contentType string, ts time.Time) ([]*MetricF
h := string(t)
_, ok = metricFamiliesByName[s]
if !ok {
fam = &MetricFamily{Name: &s, Help: &h, Type: textparse.MetricTypeUnknown}
fam = &MetricFamily{Name: &s, Help: &h}
metricFamiliesByName[s] = fam
} else {
fam.Help = &h
}
fam.Help = &h
continue
case textparse.EntryUnit:
buf, t := parser.Unit()
s := string(buf)
u := string(t)
_, ok = metricFamiliesByName[s]
if !ok {
fam = &MetricFamily{Name: &s, Unit: &u, Type: textparse.MetricTypeUnknown}
fam = &MetricFamily{Name: &s, Unit: &u}
metricFamiliesByName[string(buf)] = fam
} else {
fam.Unit = &u
}
fam.Unit = &u
continue
case textparse.EntryComment:
continue
Expand Down Expand Up @@ -577,49 +586,67 @@ func ParseMetricFamilies(b []byte, contentType string, ts time.Time) ([]*MetricF
var metric *OpenMetric

metricName := lset.Get(labels.MetricName)
var lookupMetricName string

// lookupMetricName will have the suffixes removed
lookupMetricName := metricName
var exm *exemplar.Exemplar

// Suffixes - https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#suffixes
switch mt {
case textparse.MetricTypeCounter:
if contentType == OpenMetricsType && !isTotal(metricName) && !isCreated(metricName) {
// Possible suffixes for counter in Open metrics are _created and _total.
// Otherwise, ignore.
// https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#counter-1
continue
}

var counter = &Counter{Value: &v}
mn := lset.Get(labels.MetricName)
metric = &OpenMetric{Name: &mn, Counter: counter, Label: labelPairs}
if isTotal(metricName) && contentType == OpenMetricsType { // Remove suffix _total, get lookup metricname
lookupMetricName = strings.TrimSuffix(metricName, suffixTotal)
if contentType == OpenMetricsType {
// Remove the two possible suffixes, _created and _total
if isTotal(metricName) {
lookupMetricName = strings.TrimSuffix(metricName, suffixTotal)
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} else {
} isCreated(metricName) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of this condition above if contentType == OpenMetricsType && !isTotal(metricName) && !isCreated(metricName) we already know that if we are in the else, the suffix is _created @gizas

lookupMetricName = strings.TrimSuffix(metricName, suffixCreated)
}
} else {
lookupMetricName = metricName
}
case textparse.MetricTypeGauge:
var gauge = &Gauge{Value: &v}
metric = &OpenMetric{Name: &metricName, Gauge: gauge, Label: labelPairs}
lookupMetricName = metricName
//lookupMetricName = metricName
case textparse.MetricTypeInfo:
// Info only exists for Openmetrics. It must have the suffix _info
if !isInfo(metricName) {
continue
}
lookupMetricName = strings.TrimSuffix(metricName, suffixInfo)
value := int64(v)
var info = &Info{Value: &value}
metric = &OpenMetric{Name: &metricName, Info: info, Label: labelPairs}
lookupMetricName = metricName
case textparse.MetricTypeSummary:
lookupMetricName, metric = summaryMetricName(metricName, v, qv, lbls.String(), &t, summariesByName)
lookupMetricName, metric = summaryMetricName(metricName, v, qv, lbls.String(), summariesByName)
metric.Label = labelPairs
if !isSum(metricName) {
// Avoid registering the metric multiple times.
continue
}
metricName = lookupMetricName
case textparse.MetricTypeHistogram:
if hasExemplar := parser.Exemplar(&e); hasExemplar {
exm = &e
}
lookupMetricName, metric = histogramMetricName(metricName, v, qv, lbls.String(), &t, false, exm, histogramsByName)
if metric == nil { // metric name does not have a suffix supported for the type histogram
if metric == nil {
continue
}
metric.Label = labelPairs
if !isSum(metricName) {
// Avoid registering the metric multiple times.
continue
}
metricName = lookupMetricName
case textparse.MetricTypeGaugeHistogram:
if hasExemplar := parser.Exemplar(&e); hasExemplar {
exm = &e
Expand All @@ -631,29 +658,32 @@ func ParseMetricFamilies(b []byte, contentType string, ts time.Time) ([]*MetricF
metric.Label = labelPairs
metric.Histogram.IsGaugeHistogram = true
if !isGSum(metricName) {
// Avoid registering the metric multiple times.
continue
}
metricName = lookupMetricName
case textparse.MetricTypeStateset:
value := int64(v)
var stateset = &Stateset{Value: &value}
metric = &OpenMetric{Name: &metricName, Stateset: stateset, Label: labelPairs}
lookupMetricName = metricName
case textparse.MetricTypeUnknown:
var unknown = &Unknown{Value: &v}
metric = &OpenMetric{Name: &metricName, Unknown: unknown, Label: labelPairs}
lookupMetricName = metricName
default:
lookupMetricName = metricName
}

fam, ok = metricFamiliesByName[lookupMetricName]
if !ok {
fam = &MetricFamily{Type: mt}
metricFamiliesByName[lookupMetricName] = fam
}
// If the lookupMetricName is not in metricFamiliesByName, we check for metric name, in case
// the removed suffix is part of the name.
fam, ok = metricFamiliesByName[metricName]
if !ok {
// There is not any metadata for this metric. In this case, the name of the metric
// will remain metricName instead of the possible modified lookupMetricName
fam = &MetricFamily{Name: &metricName, Type: mt}
metricFamiliesByName[metricName] = fam

fam.Name = &metricName
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line needed to be removed, because it was causing errors.

Example: if we had this metric:

# HELP cc_seconds A counter
# TYPE cc_seconds counter
# UNIT cc_seconds seconds
cc_seconds_total 1.0
cc_seconds_created 123.456

The metric would be renamed to cc_seconds_total and after to cc_seconds_created. When in reality, the metric name is cc_seconds (without the suffix). And then the metric.Metric is a list of metrics, where both are included.
This test case is in the test file.

}
}

if hasExemplar := parser.Exemplar(&e); hasExemplar && mt != textparse.MetricTypeHistogram && metric != nil {
if !e.HasTs {
Expand Down
Loading
Loading