refactor(metric): move common metrics functions out of 'internal' dir…

…ectory (#2536)

internal/cmd/base/internal/metric/build_info.go

@@ -27,7 +27,7 @@ var buildInfoVec = prometheus.NewGaugeVec(
 	[]string{labelGoVersion, labelGitRevision, labelBoundaryVersion},
 )
 
-func retrieveBuildInfoLabels() map[string]string {
+func getBuildInfoLabels() map[string]string {
 	verInfo := version.Get()
 
 	return map[string]string{
@@ -45,6 +45,6 @@ func InitializeBuildInfo(r prometheus.Registerer) {
 	}
 
 	r.MustRegister(buildInfoVec)
-	l := prometheus.Labels(retrieveBuildInfoLabels())
+	l := prometheus.Labels(getBuildInfoLabels())
 	buildInfoVec.With(l).Set(float64(1))
 }

internal/daemon/controller/internal/metric/api.go

@@ -10,7 +10,7 @@ import (
 	"strings"
 
 	"github.com/hashicorp/boundary/globals"
-	"github.com/hashicorp/boundary/internal/daemon/internal/metric"
+	"github.com/hashicorp/boundary/internal/daemon/metric"
 	"github.com/hashicorp/boundary/internal/gen/controller/api/services"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"

internal/daemon/controller/internal/metric/cluster.go

@@ -4,7 +4,7 @@ import (
 	"context"
 
 	"github.com/hashicorp/boundary/globals"
-	metric "github.com/hashicorp/boundary/internal/daemon/internal/metric"
+	"github.com/hashicorp/boundary/internal/daemon/metric"
 	"github.com/prometheus/client_golang/prometheus"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"

internal/daemon/internal/metric/initialize_metrics.go → internal/daemon/metric/initialize_metrics.go

@@ -53,20 +53,29 @@ func rangeProtoFiles(m map[string][]string, fd protoreflect.FileDescriptor) bool
 	return true
 }
 
+// appendServicesAndMethods ranges through all registered files in a specified proto package
+// and appends service and method names to the provided map m.
+// This method is exported for testing purposes.
+func appendServicesAndMethods(m map[string][]string, pkg protoreflect.FileDescriptor) {
+	protoregistry.GlobalFiles.RangeFilesByPackage(
+		pkg.Package(),
+		func(fd protoreflect.FileDescriptor) bool { return rangeProtoFiles(m, fd) },
+	)
+}
+
 // InitializeGrpcCollectorsFromPackage registers and zeroes a Prometheus
-// histogram, populating all service and method labels by ranging through a
-// given protobuf package.
+// histogram, populating all service and method labels by ranging through
+// the package containing the provided FileDescriptor.
+// Note: inputting a protoreflect.FileDescriptor will populate all services and methods
+// found in its package, not just methods associated with that specific FileDescriptor.
 func InitializeGrpcCollectorsFromPackage(r prometheus.Registerer, v prometheus.ObserverVec, pkg protoreflect.FileDescriptor, codes []codes.Code) {
 	if r == nil {
 		return
 	}
 	r.MustRegister(v)
 
 	serviceNamesToMethodNames := make(map[string][]string, 0)
-	protoregistry.GlobalFiles.RangeFilesByPackage(
-		pkg.Package(),
-		func(fd protoreflect.FileDescriptor) bool { return rangeProtoFiles(serviceNamesToMethodNames, fd) },
-	)
+	appendServicesAndMethods(serviceNamesToMethodNames, pkg)
 
 	for serviceName, serviceMethods := range serviceNamesToMethodNames {
 		for _, sm := range serviceMethods {

internal/daemon/metric/initialize_metrics_test.go

@@ -0,0 +1,32 @@
+package metric
+
+import (
+	"testing"
+
+	"github.com/hashicorp/boundary/internal/gen/testing/protooptions"
+	"github.com/stretchr/testify/assert"
+	"google.golang.org/protobuf/reflect/protoreflect"
+)
+
+// As is, this test will currently fail if the services and methods in the protooptions
+// package are changed. Please use temporary testcases to manually debug.
+func Test_AppendServicesAndMethods(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name     string
+		pkg      protoreflect.FileDescriptor
+		expected map[string][]string
+	}{
+		{
+			name:     "basic",
+			pkg:      protooptions.File_testing_options_v1_service_proto,
+			expected: map[string][]string{"testing.options.v1.TestService": {"TestMethod"}},
+		},
+	}
+	for _, tc := range cases {
+		m := make(map[string][]string, 0)
+		appendServicesAndMethods(m, tc.pkg)
+		assert.Equal(t, tc.expected, m)
+	}
+}

internal/daemon/internal/metric/instrument_handlers.go → internal/daemon/metric/instrument_handlers.go

@@ -5,6 +5,7 @@ package metric
 import (
 	"context"
 	"strings"
+	"time"
 
 	"github.com/hashicorp/boundary/internal/errors"
 	"github.com/hashicorp/boundary/internal/util"
@@ -66,6 +67,34 @@ func (sh *statsHandler) HandleRPC(ctx context.Context, s stats.RPCStats) {
 	}
 }
 
+type requestRecorder struct {
+	reqLatency prometheus.ObserverVec
+	labels     prometheus.Labels
+
+	// measurements
+	start time.Time
+}
+
+// NewGrpcRequestRecorder creates a requestRecorder struct which is used to measure gRPC client request latencies.
+func NewGrpcRequestRecorder(fullMethodName string, reqLatency prometheus.ObserverVec) requestRecorder {
+	service, method := SplitMethodName(fullMethodName)
+	r := requestRecorder{
+		reqLatency: reqLatency,
+		labels: prometheus.Labels{
+			LabelGrpcMethod:  method,
+			LabelGrpcService: service,
+		},
+		start: time.Now(),
+	}
+
+	return r
+}
+
+func (r requestRecorder) Record(err error) {
+	r.labels[LabelGrpcCode] = StatusFromError(err).Code().String()
+	r.reqLatency.With(r.labels).Observe(time.Since(r.start).Seconds())
+}
+
 // StatusFromError retrieves the *status.Status from the provided error.  It'll
 // attempt to unwrap the *status.Error, which is something status.FromError
 // does not do.

internal/daemon/internal/metric/instrument_handlers_test.go → internal/daemon/metric/instrument_handlers_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/boundary/globals"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -14,6 +15,17 @@ import (
 	"google.golang.org/grpc/status"
 )
 
+var grpcRequestLatency prometheus.ObserverVec = prometheus.NewHistogramVec(
+	prometheus.HistogramOpts{
+		Namespace: globals.MetricNamespace,
+		Subsystem: "test_metric",
+		Name:      "grpc_request_duration_seconds",
+		Help:      "Test histogram.",
+		Buckets:   prometheus.DefBuckets,
+	},
+	ListGrpcLabels,
+)
+
 func TestNewStatsHandler(t *testing.T) {
 	cases := []struct {
 		name           string
@@ -140,3 +152,69 @@ func TestNewStatsHandler(t *testing.T) {
 		})
 	}
 }
+
+func TestRecorder(t *testing.T) {
+	cases := []struct {
+		name         string
+		methodName   string
+		err          error
+		wantedLabels prometheus.Labels
+	}{
+		{
+			name:       "basic",
+			methodName: "/some.service.path/method",
+			err:        nil,
+			wantedLabels: map[string]string{
+				LabelGrpcCode:    "OK",
+				LabelGrpcMethod:  "method",
+				LabelGrpcService: "some.service.path",
+			},
+		},
+		{
+			name:       "unrecognized method path format",
+			methodName: "unrecognized",
+			err:        nil,
+			wantedLabels: map[string]string{
+				LabelGrpcCode:    "OK",
+				LabelGrpcMethod:  "unknown",
+				LabelGrpcService: "unknown",
+			},
+		},
+		{
+			name:       "cancel error",
+			methodName: "/some.service.path/method",
+			err:        status.Error(codes.Canceled, ""),
+			wantedLabels: map[string]string{
+				LabelGrpcCode:    "Canceled",
+				LabelGrpcMethod:  "method",
+				LabelGrpcService: "some.service.path",
+			},
+		},
+		{
+			name:       "permission error",
+			methodName: "/some.service.path/method",
+			err:        status.Error(codes.PermissionDenied, ""),
+			wantedLabels: map[string]string{
+				LabelGrpcCode:    "PermissionDenied",
+				LabelGrpcMethod:  "method",
+				LabelGrpcService: "some.service.path",
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			ogReqLatency := grpcRequestLatency
+			defer func() { grpcRequestLatency = ogReqLatency }()
+			testableLatency := &TestableObserverVec{}
+			start := time.Now()
+			tested := NewGrpcRequestRecorder(tc.methodName, testableLatency)
+			tested.Record(tc.err)
+
+			require.Len(t, testableLatency.Observations, 1)
+			assert.Greater(t, testableLatency.Observations[0].Observation, float64(0))
+			assert.LessOrEqual(t, testableLatency.Observations[0].Observation, time.Since(start).Seconds())
+			assert.Equal(t, testableLatency.Observations[0].Labels, tc.wantedLabels)
+		})
+	}
+}

internal/daemon/worker/internal/metric/cluster_client.go

@@ -2,10 +2,9 @@ package metric
 
 import (
 	"context"
-	"time"
 
 	"github.com/hashicorp/boundary/globals"
-	metric "github.com/hashicorp/boundary/internal/daemon/internal/metric"
+	"github.com/hashicorp/boundary/internal/daemon/metric"
 	"github.com/hashicorp/boundary/internal/gen/controller/servers/services"
 	"github.com/prometheus/client_golang/prometheus"
 	"google.golang.org/grpc"
@@ -29,35 +28,6 @@ var grpcRequestLatency prometheus.ObserverVec = prometheus.NewHistogramVec(
 	metric.ListGrpcLabels,
 )
 
-type requestRecorder struct {
-	reqLatency prometheus.ObserverVec
-	labels     prometheus.Labels
-
-	// measurements
-	start time.Time
-}
-
-// NewRequestRecorder creates a requestRecorder struct which is used to measure gRPC client request latencies.
-// For testing purposes, this method is exported.
-func newRequestRecorder(fullMethodName string, reqLatency prometheus.ObserverVec) requestRecorder {
-	service, method := metric.SplitMethodName(fullMethodName)
-	r := requestRecorder{
-		reqLatency: reqLatency,
-		labels: prometheus.Labels{
-			metric.LabelGrpcMethod:  method,
-			metric.LabelGrpcService: service,
-		},
-		start: time.Now(),
-	}
-
-	return r
-}
-
-func (r requestRecorder) Record(err error) {
-	r.labels[metric.LabelGrpcCode] = metric.StatusFromError(err).Code().String()
-	r.reqLatency.With(r.labels).Observe(time.Since(r.start).Seconds())
-}
-
 // The expected codes returned by the grpc client calls to cluster services.
 var expectedGrpcClientCodes = []codes.Code{
 	codes.OK, codes.Canceled, codes.Unknown, codes.InvalidArgument, codes.DeadlineExceeded, codes.NotFound,
@@ -71,7 +41,7 @@ var expectedGrpcClientCodes = []codes.Code{
 // between the cluster and its clients.
 func InstrumentClusterClient() grpc.UnaryClientInterceptor {
 	return func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
-		r := newRequestRecorder(method, grpcRequestLatency)
+		r := metric.NewGrpcRequestRecorder(method, grpcRequestLatency)
 		err := invoker(ctx, method, req, reply, cc, opts...)
 		r.Record(err)
 		return err

internal/daemon/worker/internal/metric/cluster_client_test.go

@@ -6,13 +6,11 @@ import (
 	"testing"
 	"time"
 
-	metric "github.com/hashicorp/boundary/internal/daemon/internal/metric"
+	"github.com/hashicorp/boundary/internal/daemon/metric"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc"
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/types/known/wrapperspb"
 )
 
@@ -21,72 +19,6 @@ func TestInitializeClusterClientCollectors(t *testing.T) {
 	require.NotPanics(t, func() { InitializeClusterClientCollectors(prometheus.NewRegistry()) })
 }
 
-func TestRecorder(t *testing.T) {
-	cases := []struct {
-		name         string
-		methodName   string
-		err          error
-		wantedLabels prometheus.Labels
-	}{
-		{
-			name:       "basic",
-			methodName: "/some.service.path/method",
-			err:        nil,
-			wantedLabels: map[string]string{
-				metric.LabelGrpcCode:    "OK",
-				metric.LabelGrpcMethod:  "method",
-				metric.LabelGrpcService: "some.service.path",
-			},
-		},
-		{
-			name:       "unrecognized method path format",
-			methodName: "unrecognized",
-			err:        nil,
-			wantedLabels: map[string]string{
-				metric.LabelGrpcCode:    "OK",
-				metric.LabelGrpcMethod:  "unknown",
-				metric.LabelGrpcService: "unknown",
-			},
-		},
-		{
-			name:       "cancel error",
-			methodName: "/some.service.path/method",
-			err:        status.Error(codes.Canceled, ""),
-			wantedLabels: map[string]string{
-				metric.LabelGrpcCode:    "Canceled",
-				metric.LabelGrpcMethod:  "method",
-				metric.LabelGrpcService: "some.service.path",
-			},
-		},
-		{
-			name:       "permission error",
-			methodName: "/some.service.path/method",
-			err:        status.Error(codes.PermissionDenied, ""),
-			wantedLabels: map[string]string{
-				metric.LabelGrpcCode:    "PermissionDenied",
-				metric.LabelGrpcMethod:  "method",
-				metric.LabelGrpcService: "some.service.path",
-			},
-		},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			ogReqLatency := grpcRequestLatency
-			defer func() { grpcRequestLatency = ogReqLatency }()
-			testableLatency := &metric.TestableObserverVec{}
-			start := time.Now()
-			tested := newRequestRecorder(tc.methodName, testableLatency)
-			tested.Record(tc.err)
-
-			require.Len(t, testableLatency.Observations, 1)
-			assert.Greater(t, testableLatency.Observations[0].Observation, float64(0))
-			assert.LessOrEqual(t, testableLatency.Observations[0].Observation, time.Since(start).Seconds())
-			assert.Equal(t, testableLatency.Observations[0].Labels, tc.wantedLabels)
-		})
-	}
-}
-
 func TestInstrumentClusterClient(t *testing.T) {
 	ogReqLatency := grpcRequestLatency
 	defer func() { grpcRequestLatency = ogReqLatency }()

internal/daemon/worker/internal/metric/cluster_server.go

@@ -4,7 +4,7 @@ import (
 	"context"
 
 	"github.com/hashicorp/boundary/globals"
-	metric "github.com/hashicorp/boundary/internal/daemon/internal/metric"
+	"github.com/hashicorp/boundary/internal/daemon/metric"
 	"github.com/prometheus/client_golang/prometheus"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"