Skip to content

Add proper categorization for client connection closing error #6844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Apr 30, 2025
Merged

Add proper categorization for client connection closing error #6844

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
158f1d9
Categorize grpc client connection closing error
timl3136 Apr 21, 2025
d608866
respond to comments
timl3136 Apr 21, 2025
3ace4b1
respond to comments
timl3136 Apr 22, 2025
92855a9
lint
timl3136 Apr 22, 2025
523c5fa
revert to string-based gRPC connection error detection since exact er…
timl3136 Apr 24, 2025
506e6cd
Merge remote-tracking branch 'upstream/master' into grpc-error-cath
timl3136 Apr 25, 2025
b539bc5
Respond to comments
timl3136 Apr 29, 2025
4c1de59
lint
timl3136 Apr 29, 2025
54d171c
Merge remote-tracking branch 'upstream/master' into grpc-error-cath
timl3136 Apr 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions common/constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,3 +316,6 @@ func GetTaskPriority(
) int {
return class | subClass
}

// GRPCConnectionClosingError is the error message returned when a gRPC client connection is closing
const GRPCConnectionClosingError = "grpc: the client connection is closing"
2 changes: 2 additions & 0 deletions common/metrics/defs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2085,6 +2085,7 @@ const (
CadenceErrQueryFailedCounter
CadenceErrLimitExceededCounter
CadenceErrContextTimeoutCounter
CadenceErrGRPCConnectionClosingCounter
CadenceErrRetryTaskCounter
CadenceErrBadBinaryCounter
CadenceErrClientVersionNotSupportedCounter
Expand Down Expand Up @@ -2814,6 +2815,7 @@ var MetricDefs = map[ServiceIdx]map[int]metricDefinition{
CadenceErrQueryFailedCounter: {metricName: "cadence_errors_query_failed", metricType: Counter},
CadenceErrLimitExceededCounter: {metricName: "cadence_errors_limit_exceeded", metricType: Counter},
CadenceErrContextTimeoutCounter: {metricName: "cadence_errors_context_timeout", metricType: Counter},
CadenceErrGRPCConnectionClosingCounter: {metricName: "cadence_errors_grpc_connection_closing", metricType: Counter},
CadenceErrRetryTaskCounter: {metricName: "cadence_errors_retry_task", metricType: Counter},
CadenceErrBadBinaryCounter: {metricName: "cadence_errors_bad_binary", metricType: Counter},
CadenceErrClientVersionNotSupportedCounter: {metricName: "cadence_errors_client_version_not_supported", metricType: Counter},
Expand Down
7 changes: 7 additions & 0 deletions service/frontend/wrappers/metered/metered.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

"go.uber.org/yarpc/yarpcerrors"

"github.com/uber/cadence/common/constants"
"github.com/uber/cadence/common/log"
"github.com/uber/cadence/common/log/tag"
"github.com/uber/cadence/common/metrics"
Expand Down Expand Up @@ -80,12 +81,18 @@ func (h *apiHandler) handleErr(err error, scope metrics.Scope, logger log.Logger
scope.IncCounter(metrics.CadenceErrContextTimeoutCounter)
return err
}
if err.Code() == yarpcerrors.CodeCancelled {
scope.IncCounter(metrics.CadenceErrGRPCConnectionClosingCounter)
logger.Warn(constants.GRPCConnectionClosingError, tag.Error(err))
return err
}
}
if errors.Is(err, context.DeadlineExceeded) {
logger.Error("Frontend request timedout", tag.Error(err))
scope.IncCounter(metrics.CadenceErrContextTimeoutCounter)
return err
}

logger.Error("Uncategorized error", tag.Error(err))
scope.IncCounter(metrics.CadenceFailures)
return frontendInternalServiceError("cadence internal uncategorized error, msg: %v", err.Error())
Expand Down
13 changes: 13 additions & 0 deletions service/frontend/wrappers/metered/metered_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
"go.uber.org/yarpc/yarpcerrors"

"github.com/uber/cadence/common/cache"
"github.com/uber/cadence/common/constants"
"github.com/uber/cadence/common/dynamicconfig/dynamicproperties"
"github.com/uber/cadence/common/log/tag"
"github.com/uber/cadence/common/log/testlogger"
Expand Down Expand Up @@ -111,6 +112,18 @@ func TestHandleErr_InternalServiceError(t *testing.T) {
assert.Contains(t, err.Error(), "cadence internal error")
}

func TestHandleErr_ClientConnectionClosingError(t *testing.T) {
logger := testlogger.New(t)
testScope := tally.NewTestScope("test", nil)
metricsClient := metrics.NewClient(testScope, metrics.Frontend)
handler := &apiHandler{}

err := handler.handleErr(errors.New(constants.GRPCConnectionClosingError), metricsClient.Scope(0), logger)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not covering the yarpc canceled error branch.


assert.NotNil(t, err)
assert.Contains(t, err.Error(), constants.GRPCConnectionClosingError)
}

func TestHandleErr_UncategorizedError(t *testing.T) {
logger := testlogger.New(t)
testScope := tally.NewTestScope("test", nil)
Expand Down