Add default Go runtime metrics for /gc/gogc:percent, /gc/gomemlimit:b…

…ytes, /sched/gomaxprocs:threads (#1559) * Add go_gomaxprocs, go_gogc_percent and go_gomemlimit to the default Go runtime metrics Signed-off-by: Arianna Vespri <[email protected]> * Move newly added metrics out of base metrics and into goCollector Signed-off-by: Arianna Vespri <[email protected]> * Rethink struct for newly added metrics, adapt and add tests Signed-off-by: Arianna Vespri <[email protected]> * Simplify new metrics reading Signed-off-by: Arianna Vespri <[email protected]> * Correct loop, add debugging lines Signed-off-by: Arianna Vespri <[email protected]> * Make goRuntimeEnvVarsMetrics function Go version dependent Signed-off-by: Arianna Vespri <[email protected]> * Fix go mod Signed-off-by: Arianna Vespri <[email protected]> * Remove debuggin line Signed-off-by: Arianna Vespri <[email protected]> * Move default runtime metrics into the runtime metrics flow, change tests accordingly Signed-off-by: Arianna Vespri <[email protected]> * Go version expected default runtime metrics map for tests Signed-off-by: Arianna Vespri <[email protected]> * 1.21 update. Signed-off-by: bwplotka <[email protected]> * Addressed comments on Arianna's PR. Signed-off-by: bwplotka <[email protected]> * Use default GoCollector func in test Signed-off-by: Arianna Vespri <[email protected]> --------- Signed-off-by: Arianna Vespri <[email protected]> Signed-off-by: bwplotka <[email protected]> Co-authored-by: bwplotka <[email protected]>
prometheus · Aug 9, 2024 · 3ad2722 · 3ad2722
1 parent 0715727
commit 3ad2722
Show file tree

Hide file tree

Showing 13 changed files with 389 additions and 227 deletions.
diff --git a/prometheus/collectors/go_collector_go117_test.go b/prometheus/collectors/go_collector_go117_test.go
@@ -102,3 +102,9 @@ func withSchedulerMetrics() []string {
 func withDebugMetrics() []string {
 	return withBaseMetrics([]string{})
 }
+
+var defaultRuntimeMetrics = []string{}
+
+func withDefaultRuntimeMetrics(metricNames []string, withoutGC, withoutSched bool) []string {
+	return metricNames
+}
diff --git a/prometheus/collectors/go_collector_go119_test.go b/prometheus/collectors/go_collector_go119_test.go
@@ -16,6 +16,8 @@
 
 package collectors
 
+import "sort"
+
 func withAllMetrics() []string {
 	return withBaseMetrics([]string{
 		"go_cgo_go_to_c_calls_calls_total",
@@ -109,3 +111,18 @@ func withSchedulerMetrics() []string {
 func withDebugMetrics() []string {
 	return withBaseMetrics([]string{})
 }
+
+var defaultRuntimeMetrics = []string{
+	"go_sched_gomaxprocs_threads",
+}
+
+func withDefaultRuntimeMetrics(metricNames []string, withoutGC, withoutSched bool) []string {
+	// If withoutSched is true, exclude "go_sched_gomaxprocs_threads".
+	if withoutSched {
+		return metricNames
+	}
+	metricNames = append(metricNames, defaultRuntimeMetrics...)
+	// sorting is required
+	sort.Strings(metricNames)
+	return metricNames
+}
diff --git a/prometheus/collectors/go_collector_go120_test.go b/prometheus/collectors/go_collector_go120_test.go
@@ -16,6 +16,8 @@
 
 package collectors
 
+import "sort"
+
 func withAllMetrics() []string {
 	return withBaseMetrics([]string{
 		"go_cgo_go_to_c_calls_calls_total",
@@ -116,3 +118,18 @@ func withSchedulerMetrics() []string {
 func withDebugMetrics() []string {
 	return withBaseMetrics([]string{})
 }
+
+var defaultRuntimeMetrics = []string{
+	"go_sched_gomaxprocs_threads",
+}
+
+func withDefaultRuntimeMetrics(metricNames []string, withoutGC, withoutSched bool) []string {
+	// If withoutSched is true, exclude "go_sched_gomaxprocs_threads".
+	if withoutSched {
+		return metricNames
+	}
+	metricNames = append(metricNames, defaultRuntimeMetrics...)
+	// sorting is required
+	sort.Strings(metricNames)
+	return metricNames
+}
diff --git a/prometheus/collectors/go_collector_go121_test.go b/prometheus/collectors/go_collector_go121_test.go
@@ -16,6 +16,8 @@
 
 package collectors
 
+import "sort"
+
 func withAllMetrics() []string {
 	return withBaseMetrics([]string{
 		"go_cgo_go_to_c_calls_calls_total",
@@ -169,3 +171,28 @@ func withDebugMetrics() []string {
 		"go_godebug_non_default_behavior_zipinsecurepath_events_total",
 	})
 }
+
+var defaultRuntimeMetrics = []string{
+	"go_gc_gogc_percent",
+	"go_gc_gomemlimit_bytes",
+	"go_sched_gomaxprocs_threads",
+}
+
+func withDefaultRuntimeMetrics(metricNames []string, withoutGC, withoutSched bool) []string {
+	if withoutGC && withoutSched {
+		// If both flags are true, return the metricNames as is.
+		return metricNames
+	} else if withoutGC && !withoutSched {
+		// If only withoutGC is true, include "go_sched_gomaxprocs_threads" only.
+		metricNames = append(metricNames, []string{"go_sched_gomaxprocs_threads"}...)
+	} else if withoutSched && !withoutGC {
+		// If only withoutSched is true, exclude "go_sched_gomaxprocs_threads".
+		metricNames = append(metricNames, []string{"go_gc_gogc_percent", "go_gc_gomemlimit_bytes"}...)
+	} else {
+		// If neither flag is true, use the default metrics.
+		metricNames = append(metricNames, defaultRuntimeMetrics...)
+	}
+	// sorting is required
+	sort.Strings(metricNames)
+	return metricNames
+}
diff --git a/prometheus/collectors/go_collector_go122_test.go b/prometheus/collectors/go_collector_go122_test.go
@@ -16,6 +16,8 @@
 
 package collectors
 
+import "sort"
+
 func withAllMetrics() []string {
 	return withBaseMetrics([]string{
 		"go_cgo_go_to_c_calls_calls_total",
@@ -191,3 +193,28 @@ func withDebugMetrics() []string {
 		"go_godebug_non_default_behavior_zipinsecurepath_events_total",
 	})
 }
+
+var defaultRuntimeMetrics = []string{
+	"go_gc_gogc_percent",
+	"go_gc_gomemlimit_bytes",
+	"go_sched_gomaxprocs_threads",
+}
+
+func withDefaultRuntimeMetrics(metricNames []string, withoutGC, withoutSched bool) []string {
+	if withoutGC && withoutSched {
+		// If both flags are true, return the metricNames as is.
+		return metricNames
+	} else if withoutGC && !withoutSched {
+		// If only withoutGC is true, include "go_sched_gomaxprocs_threads" only.
+		metricNames = append(metricNames, []string{"go_sched_gomaxprocs_threads"}...)
+	} else if withoutSched && !withoutGC {
+		// If only withoutSched is true, exclude "go_sched_gomaxprocs_threads".
+		metricNames = append(metricNames, []string{"go_gc_gogc_percent", "go_gc_gomemlimit_bytes"}...)
+	} else {
+		// If neither flag is true, use the default metrics.
+		metricNames = append(metricNames, defaultRuntimeMetrics...)
+	}
+	// sorting is required
+	sort.Strings(metricNames)
+	return metricNames
+}
diff --git a/prometheus/collectors/go_collector_latest_test.go b/prometheus/collectors/go_collector_latest_test.go
@@ -93,7 +93,9 @@ func TestWithGoCollectorDefault(t *testing.T) {
 		got = append(got, r.GetName())
 	}
 
-	if diff := cmp.Diff(got, withBaseMetrics(memstatMetrics)); diff != "" {
+	expected := append(withBaseMetrics(memstatMetrics), defaultRuntimeMetrics...)
+	sort.Strings(expected)
+	if diff := cmp.Diff(got, expected); diff != "" {
 		t.Errorf("[IMPORTANT, those are default metrics, can't change in 1.x] missmatch (-want +got):\n%s", diff)
 	}
 }
@@ -113,7 +115,7 @@ func TestWithGoCollectorMemStatsMetricsDisabled(t *testing.T) {
 		got = append(got, r.GetName())
 	}
 
-	if diff := cmp.Diff(got, baseMetrics); diff != "" {
+	if diff := cmp.Diff(got, withBaseMetrics(defaultRuntimeMetrics)); diff != "" {
 		t.Errorf("missmatch (-want +got):\n%s", diff)
 	}
 }
@@ -127,7 +129,7 @@ func TestGoCollectorAllowList(t *testing.T) {
 		{
 			name:     "Without any rules",
 			rules:    nil,
-			expected: baseMetrics,
+			expected: withBaseMetrics(defaultRuntimeMetrics),
 		},
 		{
 			name:     "allow all",
@@ -137,22 +139,22 @@ func TestGoCollectorAllowList(t *testing.T) {
 		{
 			name:     "allow GC",
 			rules:    []GoRuntimeMetricsRule{MetricsGC},
-			expected: withGCMetrics(),
+			expected: withDefaultRuntimeMetrics(withGCMetrics(), true, false),
 		},
 		{
 			name:     "allow Memory",
 			rules:    []GoRuntimeMetricsRule{MetricsMemory},
-			expected: withMemoryMetrics(),
+			expected: withDefaultRuntimeMetrics(withMemoryMetrics(), false, false),
 		},
 		{
 			name:     "allow Scheduler",
 			rules:    []GoRuntimeMetricsRule{MetricsScheduler},
-			expected: withSchedulerMetrics(),
+			expected: withDefaultRuntimeMetrics(withSchedulerMetrics(), false, true),
 		},
 		{
 			name:     "allow debug",
 			rules:    []GoRuntimeMetricsRule{MetricsDebug},
-			expected: withDebugMetrics(),
+			expected: withDefaultRuntimeMetrics(withDebugMetrics(), false, false),
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
@@ -193,7 +195,7 @@ func TestGoCollectorDenyList(t *testing.T) {
 		{
 			name:     "Without any matchers",
 			matchers: nil,
-			expected: baseMetrics,
+			expected: withBaseMetrics(defaultRuntimeMetrics),
 		},
 		{
 			name:     "deny all",
@@ -206,6 +208,14 @@ func TestGoCollectorDenyList(t *testing.T) {
 				regexp.MustCompile("^/gc/.*"),
 				regexp.MustCompile("^/sched/latencies:.*"),
 			},
+			expected: withDefaultRuntimeMetrics(baseMetrics, true, false),
+		},
+		{
+			name: "deny gc and scheduler",
+			matchers: []*regexp.Regexp{
+				regexp.MustCompile("^/gc/.*"),
+				regexp.MustCompile("^/sched/.*"),
+			},
 			expected: baseMetrics,
 		},
 	} {
@@ -235,7 +245,7 @@ func TestGoCollectorDenyList(t *testing.T) {
 func ExampleGoCollector() {
 	reg := prometheus.NewRegistry()
 
-	// Register the GoCollector with the default options. Only the base metrics and memstats are enabled.
+	// Register the GoCollector with the default options. Only the base metrics, default runtime metrics and memstats are enabled.
 	reg.MustRegister(NewGoCollector())
 
 	http.Handle("/metrics", promhttp.HandlerFor(reg, promhttp.HandlerOpts{}))

diff --git a/prometheus/gen_go_collector_metrics_set.go b/prometheus/gen_go_collector_metrics_set.go
@@ -63,16 +63,29 @@ func main() {
 	v := goVersion(gv.Segments()[1])
 	log.Printf("generating metrics for Go version %q", v)
 
+	allDesc := metrics.All()
+
+	// Find default metrics.
+	var defaultDesc []metrics.Description
+	for _, d := range allDesc {
+		if !internal.GoCollectorDefaultRuntimeMetrics.MatchString(d.Name) {
+			continue
+		}
+		defaultDesc = append(defaultDesc, d)
+	}
+
 	// Generate code.
 	var buf bytes.Buffer
 	err = testFile.Execute(&buf, struct {
-		Descriptions []metrics.Description
-		GoVersion    goVersion
-		Cardinality  int
+		AllDescriptions     []metrics.Description
+		DefaultDescriptions []metrics.Description
+		GoVersion           goVersion
+		Cardinality         int
 	}{
-		Descriptions: metrics.All(),
-		GoVersion:    v,
-		Cardinality:  rmCardinality(),
+		AllDescriptions:     allDesc,
+		DefaultDescriptions: defaultDesc,
+		GoVersion:           v,
+		Cardinality:         rmCardinality(),
 	})
 	if err != nil {
 		log.Fatalf("executing template: %v", err)
@@ -167,14 +180,25 @@ var testFile = template.Must(template.New("testFile").Funcs(map[string]interface
 
 package prometheus
 
-var expectedRuntimeMetrics = map[string]string{
-{{- range .Descriptions -}}
+var (
+	expectedRuntimeMetrics = map[string]string{
+{{- range .AllDescriptions -}}
 	{{- $trans := rm2prom . -}}
 	{{- if ne $trans "" }}
 	{{.Name | printf "%q"}}: {{$trans | printf "%q"}},
 	{{- end -}}
 {{end}}
-}
+	}
+
+	expMetrics = map[string]string{
+{{- range .DefaultDescriptions -}}
+	{{- $trans := rm2prom . -}}
+	{{- if ne $trans "" }}
+	{{.Name | printf "%q"}}: {{$trans | printf "%q"}},
+	{{- end -}}
+{{end}}
+	}
+)
 
 const expectedRuntimeMetricsCardinality = {{.Cardinality}}
 `))
diff --git a/prometheus/go_collector_latest.go b/prometheus/go_collector_latest.go
@@ -17,6 +17,7 @@
 package prometheus
 
 import (
+	"fmt"
 	"math"
 	"runtime"
 	"runtime/metrics"
@@ -153,7 +154,8 @@ func defaultGoCollectorOptions() internal.GoCollectorOptions {
 			"/gc/heap/frees-by-size:bytes":  goGCHeapFreesBytes,
 		},
 		RuntimeMetricRules: []internal.GoCollectorRule{
-			//{Matcher: regexp.MustCompile("")},
+			// Recommended metrics we want by default from runtime/metrics.
+			{Matcher: internal.GoCollectorDefaultRuntimeMetrics},
 		},
 	}
 }
@@ -376,13 +378,13 @@ func unwrapScalarRMValue(v metrics.Value) float64 {
 		//
 		// This should never happen because we always populate our metric
 		// set from the runtime/metrics package.
-		panic("unexpected unsupported metric")
+		panic("unexpected bad kind metric")
 	default:
 		// Unsupported metric kind.
 		//
 		// This should never happen because we check for this during initialization
 		// and flag and filter metrics whose kinds we don't understand.
-		panic("unexpected unsupported metric kind")
+		panic(fmt.Sprintf("unexpected unsupported metric: %v", v.Kind()))
 	}
 }
 

diff --git a/prometheus/go_collector_latest_test.go b/prometheus/go_collector_latest_test.go
@@ -74,6 +74,13 @@ func addExpectedRuntimeMetrics(metrics map[string]struct{}) map[string]struct{}
 	return metrics
 }
 
+func addExpectedDefaultRuntimeMetrics(metrics map[string]struct{}) map[string]struct{} {
+	for _, e := range expMetrics {
+		metrics[e] = struct{}{}
+	}
+	return metrics
+}
+
 func TestGoCollector_ExposedMetrics(t *testing.T) {
 	for _, tcase := range []struct {
 		opts              internal.GoCollectorOptions
@@ -86,8 +93,9 @@ func TestGoCollector_ExposedMetrics(t *testing.T) {
 			expectedFQNameSet: expectedBaseMetrics(),
 		},
 		{
-			// Default, only MemStats.
-			expectedFQNameSet: addExpectedRuntimeMemStats(expectedBaseMetrics()),
+			// Default, only MemStats and default Runtime metrics.
+			opts:              defaultGoCollectorOptions(),
+			expectedFQNameSet: addExpectedDefaultRuntimeMetrics(addExpectedRuntimeMemStats(expectedBaseMetrics())),
 		},
 		{
 			// Get all runtime/metrics without MemStats.