Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Fix prometheus exporter when tags are missing. #989

Merged
merged 4 commits into from
Nov 29, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
21 changes: 16 additions & 5 deletions exporter/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {
func (c *collector) toMetric(desc *prometheus.Desc, v *view.View, row *view.Row) (prometheus.Metric, error) {
switch data := row.Data.(type) {
case *view.CountData:
return prometheus.NewConstMetric(desc, prometheus.CounterValue, float64(data.Value), tagValues(row.Tags)...)
return prometheus.NewConstMetric(desc, prometheus.CounterValue, float64(data.Value), tagValues(row.Tags, v.TagKeys)...)

case *view.DistributionData:
points := make(map[float64]uint64)
Expand All @@ -235,13 +235,13 @@ func (c *collector) toMetric(desc *prometheus.Desc, v *view.View, row *view.Row)
cumCount += uint64(data.CountPerBucket[i])
points[b] = cumCount
}
return prometheus.NewConstHistogram(desc, uint64(data.Count), data.Sum(), points, tagValues(row.Tags)...)
return prometheus.NewConstHistogram(desc, uint64(data.Count), data.Sum(), points, tagValues(row.Tags, v.TagKeys)...)

case *view.SumData:
return prometheus.NewConstMetric(desc, prometheus.UntypedValue, data.Value, tagValues(row.Tags)...)
return prometheus.NewConstMetric(desc, prometheus.UntypedValue, data.Value, tagValues(row.Tags, v.TagKeys)...)

case *view.LastValueData:
return prometheus.NewConstMetric(desc, prometheus.GaugeValue, data.Value, tagValues(row.Tags)...)
return prometheus.NewConstMetric(desc, prometheus.GaugeValue, data.Value, tagValues(row.Tags, v.TagKeys)...)

default:
return nil, fmt.Errorf("aggregation %T is not yet supported", v.Aggregation)
Expand Down Expand Up @@ -272,10 +272,21 @@ func newCollector(opts Options, registrar *prometheus.Registry) *collector {
}
}

func tagValues(t []tag.Tag) []string {
func tagValues(t []tag.Tag, expectedKeys []tag.Key) []string {
var values []string
// Add empty string for all missing keys in the tags map.
idx := 0
for _, t := range t {
for t.Key != expectedKeys[idx] {
idx++
values = append(values, "")
}
values = append(values, t.Value)
idx++
}
for idx < len(expectedKeys) {
idx++
values = append(values, "")
}
return values
}
Expand Down
67 changes: 67 additions & 0 deletions exporter/prometheus/prometheus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -433,3 +433,70 @@ tests_foo{method="issue961",service="spanner"} 1
t.Fatal("output differed from expected")
}
}

func TestViewMeasureWithoutTag(t *testing.T) {
exporter, err := NewExporter(Options{})
if err != nil {
t.Fatalf("failed to create prometheus exporter: %v", err)
}
view.RegisterExporter(exporter)
defer view.UnregisterExporter(exporter)
m := stats.Int64("tests/foo", "foo", stats.UnitDimensionless)
k1, _ := tag.NewKey("key/1")
k2, _ := tag.NewKey("key/2")
k3, _ := tag.NewKey("key/3")
k4, _ := tag.NewKey("key/4")
k5, _ := tag.NewKey("key/5")
v := &view.View{
Name: m.Name(),
Description: m.Description(),
TagKeys: []tag.Key{k1, k2, k3, k4, k5}, // Ensure view has a tag
Measure: m,
Aggregation: view.Count(),
}
if err := view.Register(v); err != nil {
t.Fatalf("failed to create views: %v", err)
}
defer view.Unregister(v)
view.SetReportingPeriod(time.Millisecond)
// Make a measure without some tags in the view.
ctx, _ := tag.New(context.Background(), tag.Upsert(k2, "issue659"), tag.Upsert(k4, "issue659"))
Copy link
Member

Choose a reason for hiding this comment

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

For a fully defensible test, could you please add here a tag that wasn't registered with the view? e.g. k6 and k7?

stats.Record(ctx, m.M(1))
srv := httptest.NewServer(exporter)
defer srv.Close()
var i int
var output string
for {
time.Sleep(10 * time.Millisecond)
if i == 1000 {
t.Fatal("no output at /metrics (10s wait)")
}
i++
resp, err := http.Get(srv.URL)
if err != nil {
t.Fatalf("failed to get /metrics: %v", err)
}
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
t.Fatalf("failed to read body: %v", err)
}
resp.Body.Close()
output = string(body)
if output != "" {
break
}
}
if strings.Contains(output, "collected before with the same name and label values") {
t.Fatal("metric name and labels being duplicated but must be unique")
}
if strings.Contains(output, "error(s) occurred") {
t.Fatal("error reported by prometheus registry")
}
want := `# HELP tests_foo foo
# TYPE tests_foo counter
tests_foo{key_1="",key_2="issue659",key_3="",key_4="issue659",key_5=""} 1
`
if output != want {
t.Fatal("output differed from expected")
}
}