Merge branch 'feat/always-log-cgroup-path' into 'master'

Log cgroup_path on successful requests

See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/5193

Merged-by: Justin Tobler <jtobler@gitlab.com>
Approved-by: Justin Tobler <jtobler@gitlab.com>
Reviewed-by: Steve Azzopardi <sazzopardi@gitlab.com>
Reviewed-by: Will Chandler <wchandler@gitlab.com>
Reviewed-by: Justin Tobler <jtobler@gitlab.com>
Co-authored-by: Steve Azzopardi <sazzopardi@gitlab.com>

3 files changed, 51 insertions, 13 deletions

diff --git a/internal/command/command.go b/internal/command/command.go
index 0ada77777..6228b1c07 100644
--- a/internal/command/command.go
+++ b/internal/command/command.go
@@ -449,6 +449,10 @@ func (c *Command) logProcessComplete() {
 			stats.RecordSum("command.minflt", int(rusage.Minflt))
 			stats.RecordSum("command.majflt", int(rusage.Majflt))
 		}
+
+		if c.cgroupPath != "" {
+			stats.RecordMetadata("command.cgroup_path", c.cgroupPath)
+		}
 	}
 
 	service, method := methodFromContext(ctx)
@@ -483,6 +487,11 @@ func (c *Command) logProcessComplete() {
 			"majflt", rusage.Majflt,
 		)
 	}
+	if c.cgroupPath != "" {
+		c.span.LogKV(
+			"cgroup_path", c.cgroupPath,
+		)
+	}
 	c.span.Finish()
 }
 
diff --git a/internal/command/stats.go b/internal/command/stats.go
index 4ae7a582e..03b8f6025 100644
--- a/internal/command/stats.go
+++ b/internal/command/stats.go
@@ -9,59 +9,72 @@ import (
 
 type requestStatsKey struct{}
 
-//nolint:revive // This is unintentionally missing documentation.
+// Stats records statistics about a command that was spawned.
 type Stats struct {
-	registry map[string]int
+	resource map[string]int
+	metadata map[string]string
 	sync.Mutex
 }
 
-//nolint:revive // This is unintentionally missing documentation.
+// RecordSum sums up all the values for a given key.
 func (stats *Stats) RecordSum(key string, value int) {
 	stats.Lock()
 	defer stats.Unlock()
 
-	if prevValue, ok := stats.registry[key]; ok {
+	if prevValue, ok := stats.resource[key]; ok {
 		value += prevValue
 	}
 
-	stats.registry[key] = value
+	stats.resource[key] = value
 }
 
-//nolint:revive // This is unintentionally missing documentation.
+// RecordMax will store the max value for a given key.
 func (stats *Stats) RecordMax(key string, value int) {
 	stats.Lock()
 	defer stats.Unlock()
 
-	if prevValue, ok := stats.registry[key]; ok {
+	if prevValue, ok := stats.resource[key]; ok {
 		if prevValue > value {
 			return
 		}
 	}
 
-	stats.registry[key] = value
+	stats.resource[key] = value
 }
 
-//nolint:revive // This is unintentionally missing documentation.
+// RecordMetadata records metadata for the given key.
+func (stats *Stats) RecordMetadata(key string, value string) {
+	stats.Lock()
+	defer stats.Unlock()
+
+	stats.metadata[key] = value
+}
+
+// Fields returns all the stats as logrus.Fields
 func (stats *Stats) Fields() logrus.Fields {
 	stats.Lock()
 	defer stats.Unlock()
 
 	f := logrus.Fields{}
-	for k, v := range stats.registry {
+	for k, v := range stats.resource {
+		f[k] = v
+	}
+	for k, v := range stats.metadata {
 		f[k] = v
 	}
 	return f
 }
 
-//nolint:revive // This is unintentionally missing documentation.
+// StatsFromContext gets the `Stats` from the given context.
 func StatsFromContext(ctx context.Context) *Stats {
 	stats, _ := ctx.Value(requestStatsKey{}).(*Stats)
 	return stats
 }
 
-//nolint:revive // This is unintentionally missing documentation.
+// InitContextStats returns a new context with `Stats` added to the given context.
 func InitContextStats(ctx context.Context) context.Context {
 	return context.WithValue(ctx, requestStatsKey{}, &Stats{
-		registry: make(map[string]int),
+		resource: make(map[string]int),
+		metadata: make(map[string]string),
 	})
 }
diff --git a/internal/command/stats_test.go b/internal/command/stats_test.go
index 71cb35b72..b4211c11b 100644
--- a/internal/command/stats_test.go
+++ b/internal/command/stats_test.go
@@ -70,3 +70,19 @@ func TestStatsFromContext_RecordMax(t *testing.T) {
 	require.NotNil(t, stats)
 	require.Equal(t, stats.Fields(), logrus.Fields{"foo": 1024})
 }
+
+func TestStatsFromContext_RecordMetadata(t *testing.T) {
+	ctx := testhelper.Context(t)
+
+	ctx = InitContextStats(ctx)
+
+	stats := StatsFromContext(ctx)
+
+	stats.RecordMetadata("foo", "bar")
+	require.NotNil(t, stats)
+	require.Equal(t, stats.Fields(), logrus.Fields{"foo": "bar"})
+
+	stats.RecordMetadata("foo", "baz") // override the existing value
+	require.NotNil(t, stats)
+	require.Equal(t, stats.Fields(), logrus.Fields{"foo": "baz"})
+}