Details
-
Task
-
Status: Closed
-
Major
-
Resolution: Fixed
-
None
-
None
Description
To follow up on THRIFT-5744, implement slog.LogValuer on compiler generated exceptions, and maybe also structs.
The current problem is that when logging an exception, the Stringer implementation will be used, but that usually gives you quite unhelpful string, especially when optional fields are used.
For example we have this exception defined:
exception Error {
1: optional i32 code
2: optional string message
3: optional map<string, string> details
4: optional bool retryable
}
When you create an instance of it:
return &baseplate.Error{ Code: thrift.Int32Ptr(404), Message: thrift.Pointer("no such id"), }
And then log that error, instead of getting something useful with the code (404) and message ("no such id") in it, you get something like this instead because this is how we implemented fmt.Stringer from the compiler:
Error({Code:0xc000426648 Message:0xc00041ca10 Details:map[] Retryable:<nil>})
The compiler generates implementation for json.Marshaler for all exceptions, which will be much more helpful. The marshaled json for such error would be:
{"code":400,"message":"no such id"}
They are also much more expensive, so probably not suitable to replace the current Stringer implementation, but when logging them, it's probably worth it to implement slog.LogValuer so we can log something more useful, probably like this:
func (p *Error) LogValue() slog.Value{ var sb strings.Builder sb.WriteString("baseplate.Error") bytes, err := json.Marshal(p) if err != nil { // should not happen return slog.StringValue(fmt.Sprintf("baseplate.Error.LogValue: failed to marshal json: %v", err)) } sb.Write(bytes) return slog.StringValue(sb.String()) }
Which will make the error logged show as:
baseplate.Error{"code":400,"message":"no such id"}
Some open questions:
- Is this also useful for structs?
- Do we want to make this optional (e.g. only generate LogValue function when a compiler flag is passed in)?
Attachments
Issue Links
- links to
