Track Start Workflow actions correctly (temporalio#5603)

stephanos · web-flow · commit ed909df75d41 · 2024-03-26T11:00:50.000-07:00
## What changed?  Adjust the Action metric for Start Workflow. This metric is used by OSS users to estimate their Cloud bill. ## Why?  After temporalio/api#359 was merged, a successful gRPC response from Start Workflow doesn't always result in an Action. ## How did you test it?  Added tests. ## Potential risks  Low. It's only an internal metric. ## Documentation  ## Is hotfix candidate?
diff --git a/common/rpc/interceptor/telemetry.go b/common/rpc/interceptor/telemetry.go
@@ -66,17 +66,18 @@ var (
 var (
 	respondWorkflowTaskCompleted = "RespondWorkflowTaskCompleted"
 	pollActivityTaskQueue        = "PollActivityTaskQueue"
+	startWorkflowExecution       = "StartWorkflowExecution"
 
 	grpcActions = map[string]struct{}{
+		startWorkflowExecution:             {},
+		respondWorkflowTaskCompleted:       {},
+		pollActivityTaskQueue:              {},
 		"QueryWorkflow":                    {},
 		"RecordActivityTaskHeartbeat":      {},
 		"RecordActivityTaskHeartbeatById":  {},
 		"ResetWorkflowExecution":           {},
-		"StartWorkflowExecution":           {},
 		"SignalWorkflowExecution":          {},
 		"SignalWithStartWorkflowExecution": {},
-		"RespondWorkflowTaskCompleted":     {},
-		"PollActivityTaskQueue":            {},
 		"CreateSchedule":                   {},
 		"UpdateSchedule":                   {},
 		"DeleteSchedule":                   {},
@@ -214,6 +215,15 @@ func (ti *TelemetryInterceptor) emitActionMetric(
 	}
 
 	switch methodName {
+	case startWorkflowExecution:
+		resp, ok := result.(*workflowservice.StartWorkflowExecutionResponse)
+		if !ok {
+			return
+		}
+		if resp.Started {
+			metrics.ActionCounter.With(metricsHandler).Record(1, metrics.ActionType("grpc_"+methodName))
+		}
+
 	case respondWorkflowTaskCompleted:
 		// handle commands
 		completedRequest, ok := req.(*workflowservice.RespondWorkflowTaskCompletedRequest)
diff --git a/common/rpc/interceptor/telemetry_test.go b/common/rpc/interceptor/telemetry_test.go
@@ -29,6 +29,7 @@ import (
 
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/assert"
+	"go.temporal.io/api/workflowservice/v1"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 
@@ -55,21 +56,31 @@ func TestEmitActionMetric(t *testing.T) {
 		methodName        string
 		fullName          string
 		expectEmitMetrics bool
+		resp              any
 	}{
 		{
-			queryWorkflow,
-			api.WorkflowServicePrefix + queryWorkflow,
-			true,
+			methodName: startWorkflow,
+			fullName:   api.WorkflowServicePrefix + startWorkflow,
+			resp:       &workflowservice.StartWorkflowExecutionResponse{Started: false},
 		},
 		{
-			queryWorkflow,
-			api.AdminServicePrefix + queryWorkflow,
-			false,
+			methodName:        startWorkflow,
+			fullName:          api.WorkflowServicePrefix + startWorkflow,
+			resp:              &workflowservice.StartWorkflowExecutionResponse{Started: true},
+			expectEmitMetrics: true,
 		},
 		{
-			metrics.MatchingClientAddWorkflowTaskScope,
-			api.WorkflowServicePrefix + queryWorkflow,
-			false,
+			methodName:        queryWorkflow,
+			fullName:          api.WorkflowServicePrefix + queryWorkflow,
+			expectEmitMetrics: true,
+		},
+		{
+			methodName: queryWorkflow,
+			fullName:   api.AdminServicePrefix + queryWorkflow,
+		},
+		{
+			methodName: metrics.MatchingClientAddWorkflowTaskScope,
+			fullName:   api.WorkflowServicePrefix + queryWorkflow,
 		},
 	}
 
@@ -80,7 +91,7 @@ func TestEmitActionMetric(t *testing.T) {
 			} else {
 				metricsHandler.EXPECT().Counter(gomock.Any()).Return(metrics.NoopCounterMetricFunc).Times(0)
 			}
-			telemetry.emitActionMetric(tt.methodName, tt.fullName, nil, metricsHandler, nil)
+			telemetry.emitActionMetric(tt.methodName, tt.fullName, nil, metricsHandler, tt.resp)
 		})
 	}
 }
