Fix prometheus exporter when tags are missing. (#989)

* Fix prometheus exporter when tags are missing.

* Change the TestViewMeasureWithoutTag to force sanitization.

* Revert the dead code removing.

* Add more tag combinations and in different orders.

exporter/prometheus/prometheus.go

@@ -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)
@@ -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)
@@ -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
 }

exporter/prometheus/prometheus_test.go

@@ -433,3 +433,74 @@ 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")
+	randomKey, _ := tag.NewKey("issue659")
+	v := &view.View{
+		Name:        m.Name(),
+		Description: m.Description(),
+		TagKeys:     []tag.Key{k2, k5, k3, k1, k4}, // 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.
+	ctx1, _ := tag.New(context.Background(), tag.Upsert(k4, "issue659"), tag.Upsert(randomKey, "value"), tag.Upsert(k2, "issue659"))
+	stats.Record(ctx1, m.M(1))
+	ctx2, _ := tag.New(context.Background(), tag.Upsert(k5, "issue659"), tag.Upsert(k3, "issue659"), tag.Upsert(k1, "issue659"))
+	stats.Record(ctx2, m.M(2))
+	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
+tests_foo{key_1="issue659",key_2="",key_3="issue659",key_4="",key_5="issue659"} 1
+`
+	if output != want {
+		t.Fatalf("output differed from expected output: %s want: %s", output, want)
+	}
+}