Merge pull request #1733 from OffchainLabs/rpc-client-optimizations

Don't unnecessarily marshal RPC arguments for logging
OffchainLabs · Jul 8, 2023 · c041e98 · c041e98
2 parents ae00aa6 + 85cebec
commit c041e98
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 16 deletions.
diff --git a/util/rpcclient/rpcclient.go b/util/rpcclient/rpcclient.go
@@ -85,27 +85,37 @@ func (c *RpcClient) Close() {
 	}
 }
 
-func limitedMarshal(limit int, arg interface{}) string {
-	marshalled, err := json.Marshal(arg)
+type limitedMarshal struct {
+	limit int
+	value any
+}
+
+func (m limitedMarshal) String() string {
+	marshalled, err := json.Marshal(m.value)
 	var str string
 	if err != nil {
-		str = "\"CANNOT MARSHALL:" + err.Error() + "\""
+		str = "\"CANNOT MARSHALL: " + err.Error() + "\""
 	} else {
 		str = string(marshalled)
 	}
-	if limit == 0 || len(str) <= limit {
+	if m.limit == 0 || len(str) <= m.limit {
 		return str
 	}
-	prefix := str[:limit/2-1]
-	postfix := str[len(str)-limit/2+1:]
+	prefix := str[:m.limit/2-1]
+	postfix := str[len(str)-m.limit/2+1:]
 	return fmt.Sprintf("%v..%v", prefix, postfix)
 }
 
-func logArgs(limit int, args ...interface{}) string {
+type limitedArgumentsMarshal struct {
+	limit int
+	args  []any
+}
+
+func (m limitedArgumentsMarshal) String() string {
 	res := "["
-	for i, arg := range args {
-		res += limitedMarshal(limit, arg)
-		if i < len(args)-1 {
+	for i, arg := range m.args {
+		res += limitedMarshal{m.limit, arg}.String()
+		if i < len(m.args)-1 {
 			res += ", "
 		}
 	}
@@ -118,7 +128,7 @@ func (c *RpcClient) CallContext(ctx_in context.Context, result interface{}, meth
 		return errors.New("not connected")
 	}
 	logId := atomic.AddUint64(&c.logId, 1)
-	log.Trace("sending RPC request", "method", method, "logId", logId, "args", logArgs(int(c.config().ArgLogLimit), args...))
+	log.Trace("sending RPC request", "method", method, "logId", logId, "args", limitedArgumentsMarshal{int(c.config().ArgLogLimit), args})
 	var err error
 	for i := 0; i < int(c.config().Retries)+1; i++ {
 		if ctx_in.Err() != nil {
@@ -138,9 +148,8 @@ func (c *RpcClient) CallContext(ctx_in context.Context, result interface{}, meth
 		limit := int(c.config().ArgLogLimit)
 		if err != nil && err.Error() != "already known" {
 			logger = log.Info
-			limit = 0
 		}
-		logger("rpc response", "method", method, "logId", logId, "err", err, "result", limitedMarshal(limit, result), "attempt", i, "args", logArgs(limit, args...))
+		logger("rpc response", "method", method, "logId", logId, "err", err, "result", limitedMarshal{limit, result}, "attempt", i, "args", limitedArgumentsMarshal{limit, args})
 		if err == nil {
 			return nil
 		}

diff --git a/util/rpcclient/rpcclient_test.go b/util/rpcclient/rpcclient_test.go
@@ -15,17 +15,18 @@ import (
 func TestLogArgs(t *testing.T) {
 	t.Parallel()
 
-	str := logArgs(0, 1, 2, 3, "hello, world")
+	args := []any{1, 2, 3, "hello, world"}
+	str := limitedArgumentsMarshal{0, args}.String()
 	if str != "[1, 2, 3, \"hello, world\"]" {
 		Fail(t, "unexpected logs limit 0 got:", str)
 	}
 
-	str = logArgs(100, 1, 2, 3, "hello, world")
+	str = limitedArgumentsMarshal{100, args}.String()
 	if str != "[1, 2, 3, \"hello, world\"]" {
 		Fail(t, "unexpected logs limit 100 got:", str)
 	}
 
-	str = logArgs(6, 1, 2, 3, "hello, world")
+	str = limitedArgumentsMarshal{6, args}.String()
 	if str != "[1, 2, 3, \"h..d\"]" {
 		Fail(t, "unexpected logs limit 6 got:", str)
 	}