Skip to content

Commit 3da0544

Browse files
Copilotrbtr
andcommitted
Deduplicate JSON formatting code and add tests
Co-authored-by: rbtr <2940321+rbtr@users.noreply.github.com>
1 parent e902852 commit 3da0544

File tree

3 files changed

+80
-30
lines changed

3 files changed

+80
-30
lines changed

cns/logger/cnslogger.go

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package logger
22

33
import (
4-
"encoding/json"
54
"fmt"
65
"maps"
76
"os"
@@ -132,29 +131,15 @@ func (c *CNSLogger) Errorf(format string, args ...any) {
132131
c.sendTraceInternal(msg, ai.ErrorLevel)
133132
}
134133

135-
// toJSONString converts any object to a JSON string for logging purposes.
136-
// When the object contains json.RawMessage fields, they will be properly formatted
137-
// instead of being shown as byte arrays. Falls back to %+v if JSON marshaling fails.
138-
func toJSONString(obj any) string {
139-
if obj == nil {
140-
return "null"
141-
}
142-
143-
bytes, err := json.Marshal(obj)
144-
if err != nil {
145-
// Fall back to standard formatting if JSON marshaling fails
146-
return fmt.Sprintf("%+v", obj)
147-
}
148-
return string(bytes)
149-
}
134+
150135

151136
func (c *CNSLogger) Request(tag string, request any, err error) {
152137
c.logger.Request(tag, request, err)
153138
if c.th == nil || c.disableTraceLogging {
154139
return
155140
}
156141

157-
requestString := toJSONString(request)
142+
requestString := log.ToJSONString(request)
158143

159144
var msg string
160145
lvl := ai.InfoLevel
@@ -173,7 +158,7 @@ func (c *CNSLogger) Response(tag string, response any, returnCode types.Response
173158
return
174159
}
175160

176-
responseString := toJSONString(response)
161+
responseString := log.ToJSONString(response)
177162

178163
var msg string
179164
lvl := ai.InfoLevel
@@ -195,8 +180,8 @@ func (c *CNSLogger) ResponseEx(tag string, request, response any, returnCode typ
195180
return
196181
}
197182

198-
requestString := toJSONString(request)
199-
responseString := toJSONString(response)
183+
requestString := log.ToJSONString(request)
184+
responseString := log.ToJSONString(response)
200185

201186
var msg string
202187
lvl := ai.InfoLevel

log/logger.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,10 @@ func (logger *Logger) rotate() error {
200200
return nil
201201
}
202202

203-
// toJSONString converts any object to a JSON string for logging purposes.
203+
// ToJSONString converts any object to a JSON string for logging purposes.
204204
// When the object contains json.RawMessage fields, they will be properly formatted
205205
// instead of being shown as byte arrays. Falls back to %+v if JSON marshaling fails.
206-
func toJSONString(obj interface{}) string {
206+
func ToJSONString(obj interface{}) string {
207207
if obj == nil {
208208
return "null"
209209
}
@@ -219,31 +219,31 @@ func toJSONString(obj interface{}) string {
219219
// Request logs a structured request.
220220
func (logger *Logger) Request(tag string, request interface{}, err error) {
221221
if err == nil {
222-
logger.Printf("[%s] Received %T %s.", tag, request, toJSONString(request))
222+
logger.Printf("[%s] Received %T %s.", tag, request, ToJSONString(request))
223223
} else {
224-
logger.Errorf("[%s] Failed to decode %T %s %s.", tag, request, toJSONString(request), err.Error())
224+
logger.Errorf("[%s] Failed to decode %T %s %s.", tag, request, ToJSONString(request), err.Error())
225225
}
226226
}
227227

228228
// Response logs a structured response.
229229
func (logger *Logger) Response(tag string, response interface{}, returnCode int, returnStr string, err error) {
230230
if err == nil && returnCode == 0 {
231-
logger.Printf("[%s] Sent %T %s.", tag, response, toJSONString(response))
231+
logger.Printf("[%s] Sent %T %s.", tag, response, ToJSONString(response))
232232
} else if err != nil {
233-
logger.Errorf("[%s] Code:%s, %s %s.", tag, returnStr, toJSONString(response), err.Error())
233+
logger.Errorf("[%s] Code:%s, %s %s.", tag, returnStr, ToJSONString(response), err.Error())
234234
} else {
235-
logger.Errorf("[%s] Code:%s, %s.", tag, returnStr, toJSONString(response))
235+
logger.Errorf("[%s] Code:%s, %s.", tag, returnStr, ToJSONString(response))
236236
}
237237
}
238238

239239
// ResponseEx logs a structured response and the request associate with it.
240240
func (logger *Logger) ResponseEx(tag string, request interface{}, response interface{}, returnCode int, returnStr string, err error) {
241241
if err == nil && returnCode == 0 {
242-
logger.Printf("[%s] Sent %T %s %T %s.", tag, request, toJSONString(request), response, toJSONString(response))
242+
logger.Printf("[%s] Sent %T %s %T %s.", tag, request, ToJSONString(request), response, ToJSONString(response))
243243
} else if err != nil {
244-
logger.Errorf("[%s] Code:%s, %s, %s %s.", tag, returnStr, toJSONString(request), toJSONString(response), err.Error())
244+
logger.Errorf("[%s] Code:%s, %s, %s %s.", tag, returnStr, ToJSONString(request), ToJSONString(response), err.Error())
245245
} else {
246-
logger.Errorf("[%s] Code:%s, %s, %s.", tag, returnStr, toJSONString(request), toJSONString(response))
246+
logger.Errorf("[%s] Code:%s, %s, %s.", tag, returnStr, ToJSONString(request), ToJSONString(response))
247247
}
248248
}
249249

log/logger_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package log
55

66
import (
7+
"encoding/json"
78
"fmt"
89
"os"
910
"path"
@@ -18,6 +19,70 @@ const (
1819
logName = "test"
1920
)
2021

22+
func TestToJSONString(t *testing.T) {
23+
tests := []struct {
24+
name string
25+
input interface{}
26+
expected string
27+
}{
28+
{
29+
name: "nil value",
30+
input: nil,
31+
expected: "null",
32+
},
33+
{
34+
name: "simple struct",
35+
input: struct{ Name string }{"test"},
36+
expected: `{"Name":"test"}`,
37+
},
38+
{
39+
name: "struct with json.RawMessage",
40+
input: struct {
41+
ID string
42+
Data json.RawMessage
43+
Enabled bool
44+
}{
45+
ID: "123",
46+
Data: json.RawMessage(`{"foo":"bar"}`),
47+
Enabled: true,
48+
},
49+
expected: `{"ID":"123","Data":{"foo":"bar"},"Enabled":true}`,
50+
},
51+
{
52+
name: "nested json.RawMessage",
53+
input: struct {
54+
Context json.RawMessage
55+
}{
56+
Context: json.RawMessage(`{"PodName":"test-pod","PodNamespace":"default"}`),
57+
},
58+
expected: `{"Context":{"PodName":"test-pod","PodNamespace":"default"}}`,
59+
},
60+
}
61+
62+
for _, test := range tests {
63+
t.Run(test.name, func(t *testing.T) {
64+
result := ToJSONString(test.input)
65+
assert.Equal(t, test.expected, result)
66+
})
67+
}
68+
}
69+
70+
func TestToJSONStringFallback(t *testing.T) {
71+
// Create a circular reference that cannot be marshaled to JSON
72+
type Circular struct {
73+
Name string
74+
Referrer *Circular
75+
}
76+
77+
circular := &Circular{Name: "test"}
78+
circular.Referrer = circular // Create circular reference
79+
80+
// This should fall back to %+v formatting
81+
result := ToJSONString(circular)
82+
assert.Contains(t, result, "test")
83+
assert.Contains(t, result, "Referrer")
84+
}
85+
2186
func TestNewLoggerError(t *testing.T) {
2287
// we expect an error from NewLoggerE in the event that we provide an
2388
// unwriteable directory

0 commit comments

Comments
 (0)