Fix submetric matching bug when nonexistent keys are specified

grafana · Mar 8, 2022 · a3138b1 · a3138b1
1 parent e5d8c32
commit a3138b1
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 3 deletions.
diff --git a/core/engine_test.go b/core/engine_test.go
@@ -401,6 +401,15 @@ func TestEngine_processThresholds(t *testing.T) {
 		"unused,failing":      {false, map[string][]string{"unused_counter": {"count>1"}}, false},
 		"unused,subm,passing": {true, map[string][]string{"unused_counter{a:2}": {"count<1"}}, false},
 		"unused,subm,failing": {false, map[string][]string{"unused_counter{a:2}": {"count>1"}}, false},
+
+		"used,passing":               {true, map[string][]string{"used_counter": {"count==2"}}, false},
+		"used,failing":               {false, map[string][]string{"used_counter": {"count<1"}}, false},
+		"used,subm,passing":          {true, map[string][]string{"used_counter{b:1}": {"count==2"}}, false},
+		"used,not-subm,passing":      {true, map[string][]string{"used_counter{b:2}": {"count==0"}}, false},
+		"used,invalid-subm,passing1": {true, map[string][]string{"used_counter{c:''}": {"count==0"}}, false},
+		"used,invalid-subm,failing1": {false, map[string][]string{"used_counter{c:''}": {"count>0"}}, false},
+		"used,invalid-subm,passing2": {true, map[string][]string{"used_counter{c:}": {"count==0"}}, false},
+		"used,invalid-subm,failing2": {false, map[string][]string{"used_counter{c:}": {"count>0"}}, false},
 	}
 
 	for name, data := range testdata {
@@ -409,7 +418,9 @@ func TestEngine_processThresholds(t *testing.T) {
 			t.Parallel()
 
 			registry := metrics.NewRegistry()
-			metric, err := registry.NewMetric("my_metric", stats.Gauge)
+			gaugeMetric, err := registry.NewMetric("my_metric", stats.Gauge)
+			require.NoError(t, err)
+			counterMetric, err := registry.NewMetric("used_counter", stats.Counter)
 			require.NoError(t, err)
 			_, err = registry.NewMetric("unused_counter", stats.Counter)
 			require.NoError(t, err)
@@ -427,7 +438,10 @@ func TestEngine_processThresholds(t *testing.T) {
 			defer wait()
 
 			e.processSamples(
-				[]stats.SampleContainer{stats.Sample{Metric: metric, Value: 1.25, Tags: stats.IntoSampleTags(&map[string]string{"a": "1"})}},
+				[]stats.SampleContainer{
+					stats.Sample{Metric: gaugeMetric, Value: 1.25, Tags: stats.IntoSampleTags(&map[string]string{"a": "1"})},
+					stats.Sample{Metric: counterMetric, Value: 2, Tags: stats.IntoSampleTags(&map[string]string{"b": "1"})},
+				},
 			)
 
 			assert.Equal(t, data.abort, e.processThresholds())

diff --git a/stats/stats.go b/stats/stats.go
@@ -233,7 +233,7 @@ func (st *SampleTags) Contains(other *SampleTags) bool {
 	}
 
 	for k, v := range other.tags {
-		if st.tags[k] != v {
+		if myv, ok := st.tags[k]; !ok || myv != v {
 			return false
 		}
 	}

diff --git a/stats/stats_test.go b/stats/stats_test.go
@@ -132,6 +132,7 @@ func TestSampleTags(t *testing.T) {
 	assert.False(t, tags.IsEqual(IntoSampleTags(&map[string]string{"key1": "val1", "key2": "val3"})))
 	assert.True(t, tags.Contains(IntoSampleTags(&map[string]string{"key1": "val1"})))
 	assert.False(t, tags.Contains(IntoSampleTags(&map[string]string{"key3": "val1"})))
+	assert.False(t, tags.Contains(IntoSampleTags(&map[string]string{"nonexistent_key": ""})))
 	assert.Equal(t, tagMap, tags.CloneTags())
 
 	assert.Nil(t, tags.json) // No cache