Skip to content

Commit bbd432e

Browse files
committed
simplify OptimizedFmt, in reality a bytes.Buffer will only error in one very specific case that should never happen
if we're actually going to OOM via logging it would panic Signed-off-by: Callum Styan <[email protected]>
1 parent 0ca41cc commit bbd432e

File tree

2 files changed

+38
-108
lines changed

2 files changed

+38
-108
lines changed

internal/entryhuman/entry.go

Lines changed: 37 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -282,57 +282,37 @@ func Fmt(
282282

283283
// OptimizedFmt returns a human readable format for ent. Assumes we have a bytes.Buffer
284284
// which we will more easily be able to assume underlying reallocation of it's size is possible
285-
// if neceesary than for an arbitrary io.Writer/io.StringWriter
285+
// if necessary than for an arbitrary io.Writer/io.StringWriter
286+
// Note that while bytes.Buffer can in theory return an error for writes, it only does so if the buffer size will
287+
// exceed our architectures max integer size. If the system is actually OOM and more memory cannot be allocated
288+
// it will panic instead.
286289
//
287290
// We never return with a trailing newline because Go's testing framework adds one
288291
// automatically and if we include one, then we'll get two newlines.
289292
// We also do not indent the fields as go's test does that automatically
290293
// for extra lines in a log so if we did it here, the fields would be indented
291294
// twice in test logs. So the Stderr logger indents all the fields itself.
292-
func OptimizedFmt(
293-
buf *bytes.Buffer,
294-
termW io.Writer,
295-
ent slog.SinkEntry,
296-
) error {
297-
writeString := func(s string) error {
298-
_, err := buf.WriteString(s)
299-
return err
300-
}
301-
295+
func OptimizedFmt(buf *bytes.Buffer, termW io.Writer, ent slog.SinkEntry) {
302296
reset(buf, termW)
303297

304298
// Timestamp + space
305-
if err := writeString(render(termW, timeStyle, ent.Time.Format(TimeFormat))); err != nil {
306-
return err
307-
}
308-
if err := writeString(" "); err != nil {
309-
return err
310-
}
299+
buf.WriteString(render(termW, timeStyle, ent.Time.Format(TimeFormat)))
300+
buf.WriteString(" ")
311301

312302
// Level label + two spaces
313303
lvl := bracketedLevel(ent.Level) // e.g. "[debu]", "[info]"
314-
if err := writeString(render(termW, levelStyle(ent.Level), lvl)); err != nil {
315-
return err
316-
}
317-
if err := writeString(" "); err != nil {
318-
return err
319-
}
304+
buf.WriteString(render(termW, levelStyle(ent.Level), lvl))
305+
buf.WriteString(" ")
320306

321307
// Logger names: name1.name2.name3: (no strings.Join allocation)
322308
if len(ent.LoggerNames) > 0 {
323309
for i, name := range ent.LoggerNames {
324310
if i > 0 {
325-
if err := writeString("."); err != nil {
326-
return err
327-
}
328-
}
329-
if err := writeString(quoteKey(name)); err != nil {
330-
return err
311+
buf.WriteString(".")
331312
}
313+
buf.WriteString(quoteKey(name))
332314
}
333-
if err := writeString(": "); err != nil {
334-
return err
335-
}
315+
buf.WriteString(": ")
336316
}
337317

338318
// Message (detect multiline)
@@ -344,40 +324,22 @@ func OptimizedFmt(
344324
multilineVal = msg
345325
msg = quote("...")
346326
}
347-
if err := writeString(msg); err != nil {
348-
return err
349-
}
327+
buf.WriteString(msg)
350328

351329
keyStyle := timeStyle
352330
equalsStyle := timeStyle
353331

354332
// Write trace/span directly (do not mutate ent.Fields)
355333
if ent.SpanContext.IsValid() {
356-
if err := writeString(tab); err != nil {
357-
return err
358-
}
359-
if err := writeString(render(termW, keyStyle, quoteKey("trace"))); err != nil {
360-
return err
361-
}
362-
if err := writeString(render(termW, equalsStyle, "=")); err != nil {
363-
return err
364-
}
365-
if err := writeString(ent.SpanContext.TraceID().String()); err != nil {
366-
return err
367-
}
334+
buf.WriteString(tab)
368335

369-
if err := writeString(tab); err != nil {
370-
return err
371-
}
372-
if err := writeString(render(termW, keyStyle, quoteKey("span"))); err != nil {
373-
return err
374-
}
375-
if err := writeString(render(termW, equalsStyle, "=")); err != nil {
376-
return err
377-
}
378-
if err := writeString(ent.SpanContext.SpanID().String()); err != nil {
379-
return err
380-
}
336+
buf.WriteString(render(termW, keyStyle, quoteKey("trace")))
337+
buf.WriteString(render(termW, equalsStyle, "="))
338+
buf.WriteString(ent.SpanContext.TraceID().String())
339+
buf.WriteString(tab)
340+
buf.WriteString(render(termW, keyStyle, quoteKey("span")))
341+
buf.WriteString(render(termW, equalsStyle, "="))
342+
buf.WriteString(ent.SpanContext.SpanID().String())
381343
}
382344

383345
// Find a multiline field without mutating ent.Fields.
@@ -409,84 +371,55 @@ func OptimizedFmt(
409371
continue
410372
}
411373
if i < len(ent.Fields) {
412-
if err := writeString(tab); err != nil {
413-
return err
414-
}
415-
}
416-
if err := writeString(render(termW, keyStyle, quoteKey(f.Name))); err != nil {
417-
return err
418-
}
419-
if err := writeString(render(termW, equalsStyle, "=")); err != nil {
420-
return err
374+
buf.WriteString(tab)
421375
}
422376

377+
buf.WriteString(render(termW, keyStyle, quoteKey(f.Name)))
378+
buf.WriteString(render(termW, equalsStyle, "="))
379+
423380
if ok, err := writeValueFast(buf, f.Value); err != nil {
424-
return err
381+
// return err
425382
} else if !ok {
426-
if err := writeString(formatValue(f.Value)); err != nil {
427-
return err
428-
}
383+
buf.WriteString(formatValue(f.Value))
429384
}
430385
}
431386

432387
// Multiline value block
433388
if multilineVal != "" {
434389
if msg != "..." {
435-
if err := writeString(" ..."); err != nil {
436-
return err
437-
}
390+
buf.WriteString(" ...")
438391
}
439392

440-
if err := writeString("\n"); err != nil {
441-
return err
442-
}
443-
if err := writeString(render(termW, keyStyle, multilineKey)); err != nil {
444-
return err
445-
}
446-
if err := writeString("= "); err != nil {
447-
return err
448-
}
393+
buf.WriteString("\n")
394+
buf.WriteString(render(termW, keyStyle, multilineKey))
395+
buf.WriteString("= ")
449396

450397
// First line up to first newline
451398
s := multilineVal
452399
if n := strings.IndexByte(s, '\n'); n >= 0 {
453-
if err := writeString(s[:n]); err != nil {
454-
return err
455-
}
400+
buf.WriteString(s[:n])
456401
s = s[n+1:]
457402
} else {
458-
if err := writeString(s); err != nil {
459-
return err
460-
}
403+
buf.WriteString(s)
461404
s = ""
462405
}
463406

464407
indent := strings.Repeat(" ", len(multilineKey)+2)
465408
for len(s) > 0 {
466-
if err := writeString("\n"); err != nil {
467-
return err
468-
}
409+
buf.WriteString("\n")
469410
// Only indent non-empty lines.
470411
if s[0] != '\n' {
471-
if err := writeString(indent); err != nil {
472-
return err
473-
}
412+
buf.WriteString(indent)
474413
}
475414
if n := strings.IndexByte(s, '\n'); n >= 0 {
476-
if err := writeString(s[:n]); err != nil {
477-
return err
478-
}
415+
buf.WriteString(s[:n])
479416
s = s[n+1:]
480417
} else {
481-
if err := writeString(s); err != nil {
482-
return err
483-
}
418+
buf.WriteString(s)
484419
break
485420
}
486421
}
487422
}
488-
489-
return nil
490423
}
491424

492425
var (

internal/entryhuman/entry_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -411,10 +411,7 @@ func TestEntry_Optimized(t *testing.T) {
411411
var optBuf bytes.Buffer
412412

413413
entryhuman.Fmt(&fmtBuf, io.Discard, tc.ent)
414-
err := entryhuman.OptimizedFmt(&optBuf, io.Discard, tc.ent)
415-
if err != nil {
416-
t.Fatal(err)
417-
}
414+
entryhuman.OptimizedFmt(&optBuf, io.Discard, tc.ent)
418415

419416
assert.Equal(t, "outputs match", fmtBuf.String(), optBuf.String())
420417
})

0 commit comments

Comments
 (0)