Add detecting wrong metric type in prom to sd

GoogleCloudPlatform · Oct 13, 2020 · 189762f · 189762f
1 parent f90ac08
commit 189762f
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 8 deletions.
diff --git a/prometheus-to-sd/main.go b/prometheus-to-sd/main.go
@@ -223,6 +223,9 @@ func readAndPushDataToStackdriver(stackdriverService *v3.Service, gceConf *confi
 		select {
 		case <-exportTicker:
 			ts, scrapeTimestamp, err := timeSeriesBuilder.Build()
+			// Mark cache as stale at the first export attempt after each refresh. Cache is considered refreshed only if after
+			// previous export there was successful call to Refresh function.
+			metricDescriptorCache.MarkStale()
 			if err != nil {
 				glog.Errorf("Could not build time series for component %v: %v", sourceConfig.Component, err)
 			} else {
@@ -231,9 +234,6 @@ func readAndPushDataToStackdriver(stackdriverService *v3.Service, gceConf *confi
 		default:
 		}
 
-		// Mark cache at the beginning of each iteration as stale. Cache is considered refreshed only if during
-		// current iteration there was successful call to Refresh function.
-		metricDescriptorCache.MarkStale()
 		glog.V(4).Infof("Scraping metrics of component %v", sourceConfig.Component)
 		select {
 		case <-signal:

diff --git a/prometheus-to-sd/translator/metric_descriptor_cache.go b/prometheus-to-sd/translator/metric_descriptor_cache.go
@@ -82,7 +82,7 @@ func (cache *MetricDescriptorCache) ValidateMetricDescriptors(metrics map[string
 			continue
 		}
 		updatedMetricDescriptor := MetricFamilyToMetricDescriptor(cache.config, metricFamily, metricDescriptor)
-		if descriptorLabelSetChanged(metricDescriptor, updatedMetricDescriptor) {
+		if descriptorLabelSetChanged(metricDescriptor, updatedMetricDescriptor) || descriptorMetricKindChanged(metricDescriptor, updatedMetricDescriptor) {
 			cache.broken[metricFamily.GetName()] = true
 			metricFamilyDropped.WithLabelValues(cache.config.SourceConfig.Component, metricFamily.GetName()).Set(1.0)
 			glog.Warningf("Definition of the metric %s was changed and metric is not going to be pushed", metricFamily.GetName())
@@ -142,7 +142,7 @@ func (cache *MetricDescriptorCache) getMetricDescriptor(metric string) *v3.Metri
 }
 
 func descriptorChanged(original *v3.MetricDescriptor, checked *v3.MetricDescriptor) bool {
-	return descriptorDescriptionChanged(original, checked) || descriptorLabelSetChanged(original, checked)
+	return descriptorDescriptionChanged(original, checked) || descriptorLabelSetChanged(original, checked) || descriptorMetricKindChanged(original, checked)
 }
 
 func descriptorDescriptionChanged(original *v3.MetricDescriptor, checked *v3.MetricDescriptor) bool {
@@ -170,6 +170,14 @@ func descriptorLabelSetChanged(original *v3.MetricDescriptor, checked *v3.Metric
 	return false
 }
 
+func descriptorMetricKindChanged(original *v3.MetricDescriptor, checked *v3.MetricDescriptor) bool {
+	if original.MetricKind != checked.MetricKind {
+		glog.V(4).Infof("Metric kind is different, %v != %v", original.MetricKind, checked.MetricKind)
+		return true
+	}
+	return false
+}
+
 // Refresh function fetches all metric descriptors of all metrics defined for given component with a defined prefix
 // and puts them into cache.
 func (cache *MetricDescriptorCache) Refresh() {

diff --git a/prometheus-to-sd/translator/metric_descriptor_cache_test.go b/prometheus-to-sd/translator/metric_descriptor_cache_test.go
@@ -27,6 +27,8 @@ import (
 	"github.com/GoogleCloudPlatform/k8s-stackdriver/prometheus-to-sd/config"
 )
 
+var counterType = dto.MetricType_COUNTER
+
 var equalDescriptor = "equal"
 var differentDescription = "differentDescription"
 var differentLabels = "differentLabels"
@@ -41,25 +43,35 @@ var label3 = &v3.LabelDescriptor{Key: "label3"}
 var originalDescriptor = v3.MetricDescriptor{
 	Name:        equalDescriptor,
 	Description: description1,
+	MetricKind:  "GAUGE",
 	Labels:      []*v3.LabelDescriptor{label1, label2},
 }
 
 var otherDescriptors = map[*v3.MetricDescriptor]bool{
 	{
 		Name:        equalDescriptor,
 		Description: description1,
+		MetricKind:  "GAUGE",
 		Labels:      []*v3.LabelDescriptor{label1, label2},
 	}: false,
 	{
 		Name:        differentDescription,
 		Description: description2,
+		MetricKind:  "GAUGE",
 		Labels:      []*v3.LabelDescriptor{label1, label2},
 	}: true,
 	{
 		Name:        differentLabels,
 		Description: description1,
+		MetricKind:  "GAUGE",
 		Labels:      []*v3.LabelDescriptor{label3},
 	}: true,
+	{
+		Name:        equalDescriptor,
+		Description: description1,
+		MetricKind:  "CUMULATIVE",
+		Labels:      []*v3.LabelDescriptor{label1, label2},
+	}: true,
 }
 
 func TestDescriptorChanged(t *testing.T) {
@@ -84,11 +96,13 @@ func TestValidateMetricDescriptors(t *testing.T) {
 			description: "Metric is broken if new label was added",
 			descriptors: []*v3.MetricDescriptor{
 				{
-					Name: "descriptor1",
+					Name:       "descriptor1",
+					MetricKind: "CUMULATIVE",
 				},
 			},
 			metricFamily: &dto.MetricFamily{
 				Name: stringPtr("descriptor1"),
+				Type: &counterType,
 				Metric: []*dto.Metric{
 					{
 						Counter: &dto.Counter{
@@ -106,15 +120,39 @@ func TestValidateMetricDescriptors(t *testing.T) {
 			missing: false,
 			broken:  true,
 		},
+		{
+			description: "Metric is broken if metric kind was changed",
+			descriptors: []*v3.MetricDescriptor{
+				{
+					Name:       "descriptor1",
+					MetricKind: "GAUGE",
+				},
+			},
+			metricFamily: &dto.MetricFamily{
+				Name: stringPtr("descriptor1"),
+				Type: &counterType,
+				Metric: []*dto.Metric{
+					{
+						Counter: &dto.Counter{
+							Value: floatPtr(100.0),
+						},
+					},
+				},
+			},
+			missing: false,
+			broken:  true,
+		},
 		{
 			description: "Metric family hasn't changed",
 			descriptors: []*v3.MetricDescriptor{
 				{
-					Name: "descriptor1",
+					Name:       "descriptor1",
+					MetricKind: "CUMULATIVE",
 				},
 			},
 			metricFamily: &dto.MetricFamily{
 				Name: stringPtr("descriptor1"),
+				Type: &counterType,
 				Metric: []*dto.Metric{
 					{
 						Counter: &dto.Counter{
@@ -130,11 +168,13 @@ func TestValidateMetricDescriptors(t *testing.T) {
 			description: "Metric is not in the cache",
 			descriptors: []*v3.MetricDescriptor{
 				{
-					Name: "descriptor1",
+					Name:       "descriptor1",
+					MetricKind: "CUMULATIVE",
 				},
 			},
 			metricFamily: &dto.MetricFamily{
 				Name: stringPtr("descriptor2"),
+				Type: &counterType,
 				Metric: []*dto.Metric{
 					{
 						Counter: &dto.Counter{
@@ -152,10 +192,12 @@ func TestValidateMetricDescriptors(t *testing.T) {
 				{
 					Name:        "descriptor1",
 					Description: "original description",
+					MetricKind:  "CUMULATIVE",
 				},
 			},
 			metricFamily: &dto.MetricFamily{
 				Name: stringPtr("descriptor1"),
+				Type: &counterType,
 				Help: stringPtr("changed description"),
 				Metric: []*dto.Metric{
 					{