agent/metrics: allow preinitializing supporting metrics to zero

pkg/agent/protocol/grpc/proxy.go

@@ -80,7 +80,11 @@ func NewProxy(c ProxyConfig, d Disruption) (protocol.Proxy, error) {
 	return &proxy{
 		disruption: d,
 		config:     c,
-		metrics:    &protocol.MetricMap{},
+		metrics: protocol.NewMetricMap(
+			protocol.MetricRequests,
+			protocol.MetricRequestsExcluded,
+			protocol.MetricRequestsDisrupted,
+		),
 	}, nil
 }
 

pkg/agent/protocol/grpc/proxy_test.go

@@ -341,11 +341,28 @@ func Test_ProxyMetrics(t *testing.T) {
 	type TestCase struct {
 		title           string
 		disruption      Disruption
+		skipRequest     bool
 		expectedMetrics map[string]uint
 	}
 
 	// TODO: Add test for excluded endpoints
 	testCases := []TestCase{
+		{
+			title: "no requests",
+			disruption: Disruption{
+				AverageDelay:   0,
+				DelayVariation: 0,
+				ErrorRate:      0.0,
+				StatusCode:     0,
+				StatusMessage:  "",
+			},
+			skipRequest: true,
+			expectedMetrics: map[string]uint{
+				protocol.MetricRequests:          0,
+				protocol.MetricRequestsDisrupted: 0,
+				protocol.MetricRequestsExcluded:  0,
+			},
+		},
 		{
 			title: "passthrough",
 			disruption: Disruption{
@@ -356,7 +373,9 @@ func Test_ProxyMetrics(t *testing.T) {
 				StatusMessage:  "",
 			},
 			expectedMetrics: map[string]uint{
-				protocol.MetricRequests: 1,
+				protocol.MetricRequests:          1,
+				protocol.MetricRequestsDisrupted: 0,
+				protocol.MetricRequestsExcluded:  0,
 			},
 		},
 		{
@@ -371,6 +390,7 @@ func Test_ProxyMetrics(t *testing.T) {
 			expectedMetrics: map[string]uint{
 				protocol.MetricRequests:          1,
 				protocol.MetricRequestsDisrupted: 1,
+				protocol.MetricRequestsExcluded:  0,
 			},
 		},
 	}
@@ -435,18 +455,20 @@ func Test_ProxyMetrics(t *testing.T) {
 				_ = conn.Close()
 			}()
 
-			client := ping.NewPingServiceClient(conn)
-
-			var headers metadata.MD
-			_, _ = client.Ping(
-				context.TODO(),
-				&ping.PingRequest{
-					Error:   0,
-					Message: "ping",
-				},
-				grpc.Header(&headers),
-				grpc.WaitForReady(true),
-			)
+			if !tc.skipRequest {
+				client := ping.NewPingServiceClient(conn)
+
+				var headers metadata.MD
+				_, _ = client.Ping(
+					context.TODO(),
+					&ping.PingRequest{
+						Error:   0,
+						Message: "ping",
+					},
+					grpc.Header(&headers),
+					grpc.WaitForReady(true),
+				)
+			}
 
 			metrics := proxy.Metrics()
 

pkg/agent/protocol/http/proxy.go

@@ -44,7 +44,7 @@ type proxy struct {
 	config     ProxyConfig
 	disruption Disruption
 	srv        *http.Server
-	metrics    protocol.MetricMap
+	metrics    *protocol.MetricMap
 }
 
 // NewProxy return a new Proxy for HTTP requests
@@ -72,6 +72,7 @@ func NewProxy(c ProxyConfig, d Disruption) (protocol.Proxy, error) {
 	return &proxy{
 		disruption: d,
 		config:     c,
+		metrics:    protocol.NewMetricMap(supportedMetrics()...),
 	}, nil
 }
 
@@ -176,7 +177,7 @@ func (p *proxy) Start() error {
 	handler := &httpHandler{
 		upstreamURL: *upstreamURL,
 		disruption:  p.disruption,
-		metrics:     &p.metrics,
+		metrics:     p.metrics,
 	}
 
 	p.srv = &http.Server{
@@ -211,3 +212,14 @@ func (p *proxy) Force() error {
 	}
 	return nil
 }
+
+// supportedMetrics is a helper function that returns the metrics that the http proxy supports and thus should be
+// pre-initialized to zero. This function is defined due to the testing limitations mentioned in
+// https://github.com/grafana/xk6-disruptor/issues/314, as httpHandler tests currently need this information.
+func supportedMetrics() []string {
+	return []string{
+		protocol.MetricRequests,
+		protocol.MetricRequestsExcluded,
+		protocol.MetricRequestsDisrupted,
+	}
+}

pkg/agent/protocol/http/proxy_test.go

@@ -335,7 +335,7 @@ func Test_ProxyHandler(t *testing.T) {
 			handler := &httpHandler{
 				upstreamURL: *upstreamURL,
 				disruption:  tc.disruption,
-				metrics:     &protocol.MetricMap{},
+				metrics:     protocol.NewMetricMap(supportedMetrics()...),
 			}
 
 			proxyServer := httptest.NewServer(handler)
@@ -388,8 +388,12 @@ func Test_Metrics(t *testing.T) {
 		expectedMetrics map[string]uint
 	}{
 		{
-			name:            "no requests",
-			expectedMetrics: map[string]uint{},
+			name: "no requests",
+			expectedMetrics: map[string]uint{
+				protocol.MetricRequests:          0,
+				protocol.MetricRequestsExcluded:  0,
+				protocol.MetricRequestsDisrupted: 0,
+			},
 		},
 		{
 			name: "requests",
@@ -419,12 +423,12 @@ func Test_Metrics(t *testing.T) {
 				t.Fatalf("error parsing httptest url")
 			}
 
-			metrics := protocol.MetricMap{}
+			metrics := protocol.NewMetricMap(supportedMetrics()...)
 
 			handler := &httpHandler{
 				upstreamURL: *upstreamURL,
 				disruption:  tc.config,
-				metrics:     &metrics,
+				metrics:     metrics,
 			}
 
 			proxyServer := httptest.NewServer(handler)

pkg/agent/protocol/metricmap.go

@@ -8,15 +8,25 @@ type MetricMap struct {
 	mutex   sync.RWMutex
 }
 
-// Inc increases the value of the specified counter by one.
+// NewMetricMap returns a MetricMap with the specified metrics initialized to zero.
+func NewMetricMap(metrics ...string) *MetricMap {
+	mm := &MetricMap{
+		metrics: map[string]uint{},
+	}
+
+	for _, metric := range metrics {
+		mm.metrics[metric] = 0
+	}
+
+	return mm
+}
+
+// Inc increases the value of the specified counter by one. If the metric hasn't been initialized or incremented before,
+// it is set to 1.
 func (m *MetricMap) Inc(name string) {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
-	if m.metrics == nil {
-		m.metrics = make(map[string]uint)
-	}
-
 	m.metrics[name]++
 }
 

pkg/agent/protocol/metricmap_test.go

@@ -9,18 +9,34 @@ import (
 func TestMetricMap(t *testing.T) {
 	t.Parallel()
 
+	t.Run("initializes metrics", func(t *testing.T) {
+		t.Parallel()
+
+		const foo = "foo_metric"
+
+		mm := protocol.NewMetricMap(foo)
+		fooMetric, hasFoo := mm.Map()[foo]
+		if !hasFoo {
+			t.Fatalf("foo should exist in the output map")
+		}
+
+		if fooMetric != 0 {
+			t.Fatalf("foo should be zero")
+		}
+	})
+
 	t.Run("increases counters", func(t *testing.T) {
 		t.Parallel()
 
-		const name = "foo_metric"
+		const foo = "foo_metric"
 
-		mm := protocol.MetricMap{}
-		if current := mm.Map(); current[name] != 0 {
-			t.Fatalf("map should start containing zero")
+		mm := protocol.NewMetricMap()
+		if current := mm.Map(); current[foo] != 0 {
+			t.Fatalf("uninitialized foo should be zero")
 		}
 
-		mm.Inc(name)
-		if updated := mm.Map(); updated[name] != 1 {
+		mm.Inc(foo)
+		if updated := mm.Map(); updated[foo] != 1 {
 			t.Fatalf("metric was not incremented")
 		}
 	})

pkg/agent/protocol/protocol.go

@@ -30,7 +30,8 @@ type Proxy interface {
 	Start() error
 	Stop() error
 	// Metrics returns a map of counter-type metrics. Implementations may return zero or more of the metrics defined
-	// below, as well as any number of implementation-defined metrics.
+	// below, as well as any number of implementation-defined metrics. Callers can check if a metric exists in the map
+	// returned by Metrics to distinguish a counter with a value of zero from an unsupported metric.
 	Metrics() map[string]uint
 	Force() error
 }