Skip to content

Commit 507d637

Browse files
authored
Create dynamic config for max service error message length (#10151)
## What changed? Create dynamic config for max service error message length ## Why? Replacing constant max length with dynamic config for added flexibility. ## How did you test it? - [x] built - [ ] run locally and tested manually - [x] covered by existing tests - [ ] added new unit test(s) - [ ] added new functional test(s) ## Potential risks
1 parent 76d568a commit 507d637

8 files changed

Lines changed: 93 additions & 28 deletions

File tree

common/dynamicconfig/constants.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3225,6 +3225,12 @@ WorkerActivitiesPerSecond, MaxConcurrentActivityTaskPollers.
32253225
`MaxUserMetadataDetailsSize is the maximum size of user metadata details payloads in bytes.`,
32263226
)
32273227

3228+
MaxServiceErrorMessageLength = NewGlobalIntSetting(
3229+
"system.maxServiceErrorMessageLength",
3230+
4000,
3231+
"MaxServiceErrorMessageLength is the max length of service error message. If it's longer, it will be truncated.",
3232+
)
3233+
32283234
LogAllReqErrors = NewNamespaceBoolSetting(
32293235
"system.logAllReqErrors",
32303236
false,

common/rpc/interceptor/service_error_interceptor.go

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,24 +5,33 @@ import (
55
"errors"
66

77
"go.temporal.io/api/serviceerror"
8+
"go.temporal.io/server/common/dynamicconfig"
89
"go.temporal.io/server/common/persistence/serialization"
910
"go.temporal.io/server/common/util"
1011
"google.golang.org/grpc"
1112
"google.golang.org/grpc/status"
1213
)
1314

14-
const (
15-
maxMessageLength = 4000
16-
truncatedSuffix = "... <truncated>"
17-
)
15+
const truncatedSuffix = "... <truncated>"
16+
17+
type ServiceErrorInterceptor struct {
18+
maxMessageLength dynamicconfig.IntPropertyFn
19+
}
1820

19-
func ServiceErrorInterceptor(
21+
func NewServiceErrorInterceptor(
22+
maxMessageLength dynamicconfig.IntPropertyFn,
23+
) *ServiceErrorInterceptor {
24+
return &ServiceErrorInterceptor{
25+
maxMessageLength: maxMessageLength,
26+
}
27+
}
28+
29+
func (i *ServiceErrorInterceptor) Intercept(
2030
ctx context.Context,
2131
req any,
2232
_ *grpc.UnaryServerInfo,
2333
handler grpc.UnaryHandler,
2434
) (any, error) {
25-
2635
resp, err := handler(ctx, req)
2736

2837
var deserializationError *serialization.DeserializationError
@@ -33,10 +42,11 @@ func ServiceErrorInterceptor(
3342
}
3443

3544
// truncate message length if needed
45+
maxLength := i.maxMessageLength()
3646
st := serviceerror.ToStatus(err)
37-
if len(st.Message()) > maxMessageLength {
47+
if len(st.Message()) > maxLength {
3848
p := st.Proto()
39-
p.Message = util.TruncateUTF8(p.Message, maxMessageLength-len(truncatedSuffix)) + truncatedSuffix
49+
p.Message = util.TruncateUTF8(p.Message, maxLength-len(truncatedSuffix)) + truncatedSuffix
4050
st = status.FromProto(p)
4151
}
4252

common/rpc/interceptor/service_error_interceptor_test.go

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,14 @@ import (
88
"github.com/stretchr/testify/require"
99
enumspb "go.temporal.io/api/enums/v1"
1010
"go.temporal.io/api/serviceerror"
11+
"go.temporal.io/server/common/dynamicconfig"
1112
"go.temporal.io/server/common/persistence/serialization"
1213
"google.golang.org/grpc/codes"
1314
"google.golang.org/grpc/status"
1415
)
1516

17+
const testMaxMessageLength = 4000
18+
1619
type UnaryHandler func(ctx context.Context, req any) (any, error)
1720

1821
type (
@@ -28,16 +31,17 @@ func (e *ErrorWithoutStatus) Error() string {
2831

2932
// Error returns string message.
3033
func TestServiceErrorInterceptorUnknown(t *testing.T) {
34+
interceptor := NewServiceErrorInterceptor(dynamicconfig.GetIntPropertyFn(testMaxMessageLength))
3135

32-
_, err := ServiceErrorInterceptor(t.Context(), nil, nil,
36+
_, err := interceptor.Intercept(t.Context(), nil, nil,
3337
func(ctx context.Context, req any) (any, error) {
3438
return nil, status.Error(codes.InvalidArgument, "invalid argument")
3539
})
3640

3741
require.Error(t, err)
3842
require.Equal(t, codes.InvalidArgument, status.Code(err))
3943

40-
_, err = ServiceErrorInterceptor(t.Context(), nil, nil,
44+
_, err = interceptor.Intercept(t.Context(), nil, nil,
4145
func(ctx context.Context, req any) (any, error) {
4246
errWithoutStatus := &ErrorWithoutStatus{
4347
Message: "unknown error without status",
@@ -50,12 +54,13 @@ func TestServiceErrorInterceptorUnknown(t *testing.T) {
5054
}
5155

5256
func TestServiceErrorInterceptorSer(t *testing.T) {
57+
interceptor := NewServiceErrorInterceptor(dynamicconfig.GetIntPropertyFn(testMaxMessageLength))
5358
serErrors := []error{
5459
serialization.NewDeserializationError(enumspb.ENCODING_TYPE_PROTO3, nil),
5560
serialization.NewSerializationError(enumspb.ENCODING_TYPE_PROTO3, nil),
5661
}
5762
for _, inErr := range serErrors {
58-
_, err := ServiceErrorInterceptor(t.Context(), nil, nil,
63+
_, err := interceptor.Intercept(t.Context(), nil, nil,
5964
func(_ context.Context, _ any) (any, error) {
6065
return nil, inErr
6166
})
@@ -64,8 +69,10 @@ func TestServiceErrorInterceptorSer(t *testing.T) {
6469
}
6570

6671
func TestServiceErrorInterceptorTruncation(t *testing.T) {
72+
interceptor := NewServiceErrorInterceptor(dynamicconfig.GetIntPropertyFn(testMaxMessageLength))
73+
6774
t.Run("nil error is not affected", func(t *testing.T) {
68-
_, err := ServiceErrorInterceptor(t.Context(), nil, nil,
75+
_, err := interceptor.Intercept(t.Context(), nil, nil,
6976
func(_ context.Context, _ any) (any, error) {
7077
return "ok", nil
7178
})
@@ -74,7 +81,7 @@ func TestServiceErrorInterceptorTruncation(t *testing.T) {
7481

7582
t.Run("short message is not truncated", func(t *testing.T) {
7683
msg := "short error"
77-
_, err := ServiceErrorInterceptor(t.Context(), nil, nil,
84+
_, err := interceptor.Intercept(t.Context(), nil, nil,
7885
func(_ context.Context, _ any) (any, error) {
7986
return nil, serviceerror.NewInternal(msg)
8087
})
@@ -84,8 +91,8 @@ func TestServiceErrorInterceptorTruncation(t *testing.T) {
8491
})
8592

8693
t.Run("message at exact limit is not truncated", func(t *testing.T) {
87-
msg := strings.Repeat("a", maxMessageLength)
88-
_, err := ServiceErrorInterceptor(t.Context(), nil, nil,
94+
msg := strings.Repeat("a", testMaxMessageLength)
95+
_, err := interceptor.Intercept(t.Context(), nil, nil,
8996
func(_ context.Context, _ any) (any, error) {
9097
return nil, serviceerror.NewInternal(msg)
9198
})
@@ -95,20 +102,20 @@ func TestServiceErrorInterceptorTruncation(t *testing.T) {
95102
})
96103

97104
t.Run("message over limit is truncated", func(t *testing.T) {
98-
msg := strings.Repeat("a", maxMessageLength+100)
99-
_, err := ServiceErrorInterceptor(t.Context(), nil, nil,
105+
msg := strings.Repeat("a", testMaxMessageLength+100)
106+
_, err := interceptor.Intercept(t.Context(), nil, nil,
100107
func(_ context.Context, _ any) (any, error) {
101108
return nil, serviceerror.NewInternal(msg)
102109
})
103110
require.Error(t, err)
104111
st := status.Convert(err)
105-
require.LessOrEqual(t, len(st.Message()), maxMessageLength)
112+
require.LessOrEqual(t, len(st.Message()), testMaxMessageLength)
106113
require.True(t, strings.HasSuffix(st.Message(), truncatedSuffix))
107114
})
108115

109116
t.Run("truncation preserves error code", func(t *testing.T) {
110-
msg := strings.Repeat("x", maxMessageLength+500)
111-
_, err := ServiceErrorInterceptor(t.Context(), nil, nil,
117+
msg := strings.Repeat("x", testMaxMessageLength+500)
118+
_, err := interceptor.Intercept(t.Context(), nil, nil,
112119
func(_ context.Context, _ any) (any, error) {
113120
return nil, serviceerror.NewNotFound(msg)
114121
})
@@ -119,15 +126,15 @@ func TestServiceErrorInterceptorTruncation(t *testing.T) {
119126
t.Run("truncation respects multi-byte UTF-8 boundary", func(t *testing.T) {
120127
// Fill up to near the limit with multi-byte characters (3 bytes each for '€')
121128
// then push over the limit so truncation must split within the repeated chars.
122-
euroCount := maxMessageLength / len("€") // each '€' is 3 bytes
129+
euroCount := testMaxMessageLength / len("€") // each '€' is 3 bytes
123130
msg := strings.Repeat("€", euroCount+100)
124-
_, err := ServiceErrorInterceptor(t.Context(), nil, nil,
131+
_, err := interceptor.Intercept(t.Context(), nil, nil,
125132
func(_ context.Context, _ any) (any, error) {
126133
return nil, serviceerror.NewInternal(msg)
127134
})
128135
require.Error(t, err)
129136
st := status.Convert(err)
130-
require.LessOrEqual(t, len(st.Message()), maxMessageLength)
137+
require.LessOrEqual(t, len(st.Message()), testMaxMessageLength)
131138
require.True(t, strings.HasSuffix(st.Message(), truncatedSuffix))
132139
// Verify the truncated body (without suffix) is valid UTF-8 by checking
133140
// that no partial rune was left behind — the full message should be valid.

service/frontend/fx.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ var Module = fx.Options(
7777
// A more robust approach would require using fx groups but we shouldn't overcomplicate until this becomes an issue.
7878
fx.Provide(MuxRouterProvider),
7979
fx.Provide(ConfigProvider),
80+
fx.Provide(ServiceErrorInterceptorProvider),
8081
fx.Provide(NamespaceLogInterceptorProvider),
8182
fx.Provide(NamespaceHandoverInterceptorProvider),
8283
fx.Provide(interceptor.NewRoutingKeyExtractor),
@@ -218,6 +219,7 @@ func GrpcServerOptionsProvider(
218219
serviceConfig *Config,
219220
serviceName primitives.ServiceName,
220221
rpcFactory common.RPCFactory,
222+
serviceErrorInterceptor *interceptor.ServiceErrorInterceptor,
221223
namespaceLogInterceptor *interceptor.NamespaceLogInterceptor,
222224
namespaceRateLimiterInterceptor interceptor.NamespaceRateLimitInterceptor,
223225
namespaceCountLimiterInterceptor *interceptor.ConcurrentRequestLimitInterceptor,
@@ -271,7 +273,7 @@ func GrpcServerOptionsProvider(
271273
// Mask error interceptor should be the most outer interceptor since it handle the errors format
272274
// Service Error Interceptor should be the next most outer interceptor on error handling
273275
maskInternalErrorDetailsInterceptor.Intercept,
274-
interceptor.ServiceErrorInterceptor,
276+
serviceErrorInterceptor.Intercept,
275277
interceptor.NewFrontendServiceErrorInterceptor(logger),
276278
// BusinessID interceptor extracts business ID and adds it to context for use, must be before any interceptor that touches namespaces (namespaceValidator, handoverInterceptor)
277279
businessIDInterceptor.Intercept,
@@ -342,6 +344,14 @@ func ConfigProvider(
342344
)
343345
}
344346

347+
func ServiceErrorInterceptorProvider(
348+
dc *dynamicconfig.Collection,
349+
) *interceptor.ServiceErrorInterceptor {
350+
return interceptor.NewServiceErrorInterceptor(
351+
dynamicconfig.MaxServiceErrorMessageLength.Get(dc),
352+
)
353+
}
354+
345355
func ThrottledLoggerRpsFnProvider(serviceConfig *Config) resource.ThrottledLoggerRpsFn {
346356
return func() float64 { return float64(serviceConfig.ThrottledLogRPS()) }
347357
}

service/frontend/fx_test.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
enumspb "go.temporal.io/api/enums/v1"
1313
"go.temporal.io/api/serviceerror"
1414
"go.temporal.io/api/workflowservice/v1"
15+
"go.temporal.io/server/common/dynamicconfig"
1516
"go.temporal.io/server/common/log"
1617
"go.temporal.io/server/common/membership"
1718
"go.temporal.io/server/common/metrics"
@@ -205,6 +206,10 @@ func TestRateLimitInterceptorProvider(t *testing.T) {
205206
}
206207
tc.configure(&tc)
207208

209+
serviceErrorInterceptor := interceptor.NewServiceErrorInterceptor(
210+
dynamicconfig.GetIntPropertyFn(4000),
211+
)
212+
208213
// Create a rate limit interceptor which uses the per-instance and global RPS limits from the test case.
209214
rateLimitInterceptor := RateLimitInterceptorProvider(&Config{
210215
RPS: func() int {
@@ -225,7 +230,7 @@ func TestRateLimitInterceptorProvider(t *testing.T) {
225230
// Create a gRPC server for the fake workflow service.
226231
svc := &testSvc{}
227232
server := grpc.NewServer(grpc.ChainUnaryInterceptor(
228-
interceptor.ServiceErrorInterceptor,
233+
serviceErrorInterceptor.Intercept,
229234
interceptor.NewFrontendServiceErrorInterceptor(log.NewTestLogger()),
230235
rateLimitInterceptor.Intercept,
231236
))
@@ -569,6 +574,10 @@ func TestNamespaceRateLimitInterceptorProvider(t *testing.T) {
569574

570575
config := getTestConfig(tc)
571576

577+
serviceErrorInterceptor := interceptor.NewServiceErrorInterceptor(
578+
dynamicconfig.GetIntPropertyFn(4000),
579+
)
580+
572581
// Create a rate limit interceptor.
573582
rateLimitInterceptor := NamespaceRateLimitInterceptorProvider(
574583
primitives.FrontendService,
@@ -582,7 +591,7 @@ func TestNamespaceRateLimitInterceptorProvider(t *testing.T) {
582591
// Create a gRPC server for the fake workflow service.
583592
svc := &testSvc{}
584593
server := grpc.NewServer(grpc.ChainUnaryInterceptor(
585-
interceptor.ServiceErrorInterceptor,
594+
serviceErrorInterceptor.Intercept,
586595
interceptor.NewFrontendServiceErrorInterceptor(log.NewTestLogger()),
587596
rateLimitInterceptor.Intercept,
588597
))
@@ -764,13 +773,17 @@ func TestNamespaceRateLimitMetrics(t *testing.T) {
764773
},
765774
}
766775

776+
serviceErrorInterceptor := interceptor.NewServiceErrorInterceptor(
777+
dynamicconfig.GetIntPropertyFn(4000),
778+
)
779+
767780
// Create a rate limit interceptor which uses the per-instance and global RPS limits from the test case.
768781
rateLimitInterceptor := RateLimitInterceptorProvider(config, serviceResolver, metricsHandler, log.NewTestLogger())
769782

770783
// Create a gRPC server for the fake workflow service.
771784
svc := &testSvc{}
772785
server := grpc.NewServer(grpc.ChainUnaryInterceptor(
773-
interceptor.ServiceErrorInterceptor,
786+
serviceErrorInterceptor.Intercept,
774787
interceptor.NewFrontendServiceErrorInterceptor(log.NewTestLogger()),
775788
rateLimitInterceptor.Intercept,
776789
))

service/fx.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ type (
4040

4141
Logger log.Logger
4242
RPCFactory common.RPCFactory
43+
ServiceErrorInterceptor *interceptor.ServiceErrorInterceptor
4344
RetryableInterceptor *interceptor.RetryableInterceptor
4445
TelemetryInterceptor *interceptor.TelemetryInterceptor
4546
NamespaceRateLimitInterceptor interceptor.NamespaceRateLimitInterceptor `optional:"true"`
@@ -157,7 +158,7 @@ func GrpcServerOptionsProvider(
157158

158159
func getUnaryInterceptors(params GrpcServerOptionsParams) []grpc.UnaryServerInterceptor {
159160
interceptors := []grpc.UnaryServerInterceptor{
160-
interceptor.ServiceErrorInterceptor,
161+
params.ServiceErrorInterceptor.Intercept,
161162
metrics.NewServerMetricsContextInjectorInterceptor(),
162163
metrics.NewServerMetricsTrailerPropagatorInterceptor(params.Logger),
163164
params.TelemetryInterceptor.UnaryIntercept,

service/history/fx.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ var Module = fx.Options(
6464
ChasmEngineModule,
6565
fx.Provide(ConfigProvider), // might be worth just using provider for configs.Config directly
6666
fx.Provide(workflow.NewCommandHandlerRegistry),
67+
fx.Provide(ServiceErrorInterceptorProvider),
6768
fx.Provide(RetryableInterceptorProvider),
6869
fx.Provide(ErrorHandlerProvider),
6970
fx.Provide(TelemetryInterceptorProvider),
@@ -196,6 +197,14 @@ func ConfigProvider(
196197
)
197198
}
198199

200+
func ServiceErrorInterceptorProvider(
201+
dc *dynamicconfig.Collection,
202+
) *interceptor.ServiceErrorInterceptor {
203+
return interceptor.NewServiceErrorInterceptor(
204+
dynamicconfig.MaxServiceErrorMessageLength.Get(dc),
205+
)
206+
}
207+
199208
func ThrottledLoggerRpsFnProvider(serviceConfig *configs.Config) resource.ThrottledLoggerRpsFn {
200209
return func() float64 { return float64(serviceConfig.ThrottledLogRPS()) }
201210
}

service/matching/fx.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ var Module = fx.Options(
3535
fx.Provide(PersistenceRateLimitingParamsProvider),
3636
service.PersistenceLazyLoadedServiceResolverModule,
3737
fx.Provide(ThrottledLoggerRpsFnProvider),
38+
fx.Provide(ServiceErrorInterceptorProvider),
3839
fx.Provide(ContextMetadataInterceptorProvider),
3940
fx.Provide(RetryableInterceptorProvider),
4041
fx.Provide(ErrorHandlerProvider),
@@ -62,6 +63,14 @@ func ConfigProvider(
6263
return NewConfig(dc)
6364
}
6465

66+
func ServiceErrorInterceptorProvider(
67+
dc *dynamicconfig.Collection,
68+
) *interceptor.ServiceErrorInterceptor {
69+
return interceptor.NewServiceErrorInterceptor(
70+
dynamicconfig.MaxServiceErrorMessageLength.Get(dc),
71+
)
72+
}
73+
6574
func RetryableInterceptorProvider() *interceptor.RetryableInterceptor {
6675
return interceptor.NewRetryableInterceptor(
6776
common.CreateMatchingHandlerRetryPolicy(),

0 commit comments

Comments
 (0)