Skip to content

Commit

Permalink
change logging code to avoid panics
Browse files Browse the repository at this point in the history
It makes no sense to risk panicking from within logging routine
just because the caller botched their x.Marshall() method. This
might impact the production app, if they don't exhaustively test
marshaling. Correctness of logging is not worth THAT much.

Also, move testing code to _test.go. This means it won't be compiled
for live code.
  • Loading branch information
jabielecki committed Feb 26, 2022
1 parent e3a860c commit ab40882
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 29 deletions.
35 changes: 6 additions & 29 deletions glog.go
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,9 @@ func logj(s severity, l *Logger, msg string, j interface{}) {
// }
buf, err := json.Marshal(j)
if err != nil {
panic(err)
// Ignore. This is just logging. Do not panic the entire app
// just because the caller had botched their x.Marshal(), etc.
return
}

logRawJSON(s, l, msg, buf)
Expand All @@ -700,22 +702,22 @@ func logRawJSON(s severity, l *Logger, msg string, buf []byte) {
if msg != "" {
msgj, err = json.Marshal(msg)
if err != nil {
panic(err)
return
}
}

sev := s.String()
if sev != "" {
sevj, err = json.Marshal(sev)
if err != nil {
panic(err)
return
}
}

if l.trace != "" {
tracej, err = json.Marshal(l.trace)
if err != nil {
panic(err)
return
}
}

Expand Down Expand Up @@ -804,28 +806,3 @@ func logRawJSON(s severity, l *Logger, msg string, buf []byte) {
}
}

// logjStdlib is only here to benchmark the stdlib "encoding/json"
// approach.
func logjStdlib(s severity, l *Logger, msg string, j interface{}) {
entry := make(map[string]json.RawMessage)

if buf, err := json.Marshal(j); err != nil {
panic(err)
} else if err := json.Unmarshal(buf, &entry); err != nil {
panic(err)
}

if v := msg; v != "" {
entry["message"], _ = json.Marshal(v)
}
if v := s.String(); v != "" {
entry["severity"], _ = json.Marshal(v)
}
if v := l.trace; v != "" {
entry["logging.googleapis.com/trace"], _ = json.Marshal(v)
}

l.mu.Lock()
defer l.mu.Unlock()
_ = json.NewEncoder(l.writer(s)).Encode(entry)
}
26 changes: 26 additions & 0 deletions glog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,3 +405,29 @@ func BenchmarkStdlibTen(b *testing.B) {
buf.Reset()
}
}

// logjStdlib is only here to benchmark the stdlib "encoding/json"
// approach. Hopefully our method is much faster than stdlib.
func logjStdlib(s severity, l *Logger, msg string, j interface{}) {
entry := make(map[string]json.RawMessage)

if buf, err := json.Marshal(j); err != nil {
panic(err)
} else if err := json.Unmarshal(buf, &entry); err != nil {
panic(err)
}

if v := msg; v != "" {
entry["message"], _ = json.Marshal(v)
}
if v := s.String(); v != "" {
entry["severity"], _ = json.Marshal(v)
}
if v := l.trace; v != "" {
entry["logging.googleapis.com/trace"], _ = json.Marshal(v)
}

l.mu.Lock()
defer l.mu.Unlock()
_ = json.NewEncoder(l.writer(s)).Encode(entry)
}

0 comments on commit ab40882

Please sign in to comment.