Fix nexus_task_execution_failed to include OperationError outcome in …

…start requests (#1664)
temporalio · Oct 14, 2024 · e503995 · e503995
1 parent 3671c99
commit e503995
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 1 deletion.
diff --git a/internal/internal_nexus_task_poller.go b/internal/internal_nexus_task_poller.go
@@ -152,7 +152,12 @@ func (ntp *nexusTaskPoller) ProcessTask(task interface{}) error {
 
 	// Execution latency (in-SDK processing time).
 	metricsHandler.Timer(metrics.NexusTaskExecutionLatency).Record(time.Since(executionStartTime))
-	if err != nil || failure != nil {
+
+	// Increment failure in all forms of errors:
+	// Internal error processing the task.
+	// Failure from user handler.
+	// Special case for the start response with operation error.
+	if err != nil || failure != nil || res.Response.GetStartOperation().GetOperationError() != nil {
 		metricsHandler.Counter(metrics.NexusTaskExecutionFailedCounter).Inc(1)
 	}
 

diff --git a/test/nexus_test.go b/test/nexus_test.go
@@ -251,6 +251,13 @@ func TestNexusSyncOperation(t *testing.T) {
 		require.ErrorAs(t, err, &unsuccessfulOperationErr)
 		require.Equal(t, nexus.OperationStateFailed, unsuccessfulOperationErr.State)
 		require.Equal(t, "fail", unsuccessfulOperationErr.Failure.Message)
+
+		require.EventuallyWithT(t, func(t *assert.CollectT) {
+			tc.requireTimer(t, metrics.NexusTaskEndToEndLatency, service.Name, syncOp.Name())
+			tc.requireTimer(t, metrics.NexusTaskScheduleToStartLatency, service.Name, syncOp.Name())
+			tc.requireTimer(t, metrics.NexusTaskExecutionLatency, service.Name, syncOp.Name())
+			tc.requireCounter(t, metrics.NexusTaskExecutionFailedCounter, service.Name, syncOp.Name())
+		}, time.Second*3, time.Millisecond*100)
 	})
 
 	t.Run("fmt-errorf", func(t *testing.T) {
@@ -259,6 +266,13 @@ func TestNexusSyncOperation(t *testing.T) {
 		var unexpectedResponseErr *nexus.UnexpectedResponseError
 		require.ErrorAs(t, err, &unexpectedResponseErr)
 		require.Contains(t, unexpectedResponseErr.Message, `"500 Internal Server Error": arbitrary error message`)
+
+		require.EventuallyWithT(t, func(t *assert.CollectT) {
+			tc.requireTimer(t, metrics.NexusTaskEndToEndLatency, service.Name, syncOp.Name())
+			tc.requireTimer(t, metrics.NexusTaskScheduleToStartLatency, service.Name, syncOp.Name())
+			tc.requireTimer(t, metrics.NexusTaskExecutionLatency, service.Name, syncOp.Name())
+			tc.requireCounter(t, metrics.NexusTaskExecutionFailedCounter, service.Name, syncOp.Name())
+		}, time.Second*3, time.Millisecond*100)
 	})
 
 	t.Run("handlererror", func(t *testing.T) {
@@ -327,6 +341,13 @@ func TestNexusSyncOperation(t *testing.T) {
 		require.ErrorAs(t, err, &unexpectedResponseErr)
 		require.Equal(t, 500, unexpectedResponseErr.Response.StatusCode)
 		require.Contains(t, unexpectedResponseErr.Message, "panic: panic requested")
+
+		require.EventuallyWithT(t, func(t *assert.CollectT) {
+			tc.requireTimer(t, metrics.NexusTaskEndToEndLatency, service.Name, syncOp.Name())
+			tc.requireTimer(t, metrics.NexusTaskScheduleToStartLatency, service.Name, syncOp.Name())
+			tc.requireTimer(t, metrics.NexusTaskExecutionLatency, service.Name, syncOp.Name())
+			tc.requireCounter(t, metrics.NexusTaskExecutionFailedCounter, service.Name, syncOp.Name())
+		}, time.Second*3, time.Millisecond*100)
 	})
 }