Ensure consistent labels for rpc metrics (#1444)

temporalio · Apr 24, 2024 · 93c08b0 · 93c08b0
1 parent 69bc6c3
commit 93c08b0
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 7 deletions.
diff --git a/internal/common/metrics/grpc.go b/internal/common/metrics/grpc.go
@@ -27,11 +27,11 @@ import (
 	"strings"
 	"time"
 
-	"google.golang.org/grpc/status"
 	enumspb "go.temporal.io/api/enums/v1"
 	"go.temporal.io/api/serviceerror"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 // HandlerContextKey is the context key for a MetricHandler value.
@@ -63,16 +63,17 @@ func NewGRPCInterceptor(defaultHandler Handler, suffix string) grpc.UnaryClientI
 
 		// Only take method name after the last slash
 		operation := method[strings.LastIndex(method, "/")+1:]
-		tags := map[string]string{OperationTagName: operation}
 
 		// Since this interceptor can be used for clients of different name, we
 		// attempt to extract the namespace out of the request. All namespace-based
 		// requests have been confirmed to have a top-level namespace field.
+		namespace := "_unknown_"
 		if nsReq, _ := req.(interface{ GetNamespace() string }); nsReq != nil {
-			tags[NamespaceTagName] = nsReq.GetNamespace()
+			namespace = nsReq.GetNamespace()
 		}
 
 		// Capture time, record start, run, and record end
+		tags := map[string]string{OperationTagName: operation, NamespaceTagName: namespace}
 		handler = handler.WithTags(tags)
 		start := time.Now()
 		recordRequestStart(handler, longPoll, suffix)

diff --git a/internal/common/metrics/grpc_test.go b/internal/common/metrics/grpc_test.go
@@ -65,12 +65,12 @@ func TestGRPCInterceptor(t *testing.T) {
 	counters := handler.Counters()
 	require.Len(t, counters, 1)
 	require.Equal(t, metrics.TemporalRequest+"_my_suffix", counters[0].Name)
-	require.Equal(t, map[string]string{metrics.OperationTagName: "Check"}, counters[0].Tags)
+	require.Equal(t, map[string]string{metrics.OperationTagName: "Check", metrics.NamespaceTagName: "_unknown_"}, counters[0].Tags)
 	require.Equal(t, int64(1), counters[0].Value())
 	timers := handler.Timers()
 	require.Len(t, timers, 1)
 	require.Equal(t, metrics.TemporalRequestLatency+"_my_suffix", timers[0].Name)
-	require.Equal(t, map[string]string{metrics.OperationTagName: "Check"}, timers[0].Tags)
+	require.Equal(t, map[string]string{metrics.OperationTagName: "Check", metrics.NamespaceTagName: "_unknown_"}, timers[0].Tags)
 
 	// Now clear the metrics and set a handler with tags and long poll on the
 	// context and make a known failing call
@@ -85,9 +85,9 @@ func TestGRPCInterceptor(t *testing.T) {
 	counters = handler.Counters()
 	require.Len(t, counters, 2)
 	require.Equal(t, metrics.TemporalLongRequest+"_my_suffix", counters[0].Name)
-	require.Equal(t, map[string]string{metrics.OperationTagName: "Check", "roottag": "roottagval"}, counters[0].Tags)
+	require.Equal(t, map[string]string{metrics.OperationTagName: "Check", "roottag": "roottagval", metrics.NamespaceTagName: "_unknown_"}, counters[0].Tags)
 	require.Equal(t, int64(1), counters[0].Value())
 	require.Equal(t, metrics.TemporalLongRequestFailure+"_my_suffix", counters[1].Name)
-	require.Equal(t, map[string]string{metrics.OperationTagName: "Check", "roottag": "roottagval"}, counters[1].Tags)
+	require.Equal(t, map[string]string{metrics.OperationTagName: "Check", "roottag": "roottagval", metrics.NamespaceTagName: "_unknown_"}, counters[1].Tags)
 	require.Equal(t, int64(1), counters[1].Value())
 }