log: fix formatting of big.Int (#22679)

* log: fix formatting of big.Int

The implementation of formatLogfmtBigInt had two issues: it crashed when
the number was actually large enough to hit the big integer case, and
modified the big.Int while formatting it.

* log: don't call FormatLogfmtInt64 for int16

* log: separate from decimals back, not front

Co-authored-by: Péter Szilágyi <[email protected]>

Author: 2 people

log/format.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"math/big"
 	"reflect"
 	"strconv"
 	"strings"
@@ -350,15 +351,115 @@ func formatLogfmtValue(value interface{}, term bool) string {
 		return strconv.FormatFloat(float64(v), floatFormat, 3, 64)
 	case float64:
 		return strconv.FormatFloat(v, floatFormat, 3, 64)
-	case int, int8, int16, int32, int64, uint, uint8, uint16, uint32, uint64:
-		return fmt.Sprintf("%d", value)
+	case int8:
+		return strconv.FormatInt(int64(v), 10)
+	case uint8:
+		return strconv.FormatInt(int64(v), 10)
+	case int16:
+		return strconv.FormatInt(int64(v), 10)
+	case uint16:
+		return strconv.FormatInt(int64(v), 10)
+	// Larger integers get thousands separators.
+	case int:
+		return FormatLogfmtInt64(int64(v))
+	case int32:
+		return FormatLogfmtInt64(int64(v))
+	case int64:
+		return FormatLogfmtInt64(v)
+	case uint:
+		return FormatLogfmtUint64(uint64(v))
+	case uint32:
+		return FormatLogfmtUint64(uint64(v))
+	case uint64:
+		return FormatLogfmtUint64(v)
 	case string:
 		return escapeString(v)
 	default:
 		return escapeString(fmt.Sprintf("%+v", value))
 	}
 }
 
+// FormatLogfmtInt64 formats n with thousand separators.
+func FormatLogfmtInt64(n int64) string {
+	if n < 0 {
+		return formatLogfmtUint64(uint64(-n), true)
+	}
+	return formatLogfmtUint64(uint64(n), false)
+}
+
+// FormatLogfmtUint64 formats n with thousand separators.
+func FormatLogfmtUint64(n uint64) string {
+	return formatLogfmtUint64(n, false)
+}
+
+func formatLogfmtUint64(n uint64, neg bool) string {
+	// Small numbers are fine as is
+	if n < 100000 {
+		if neg {
+			return strconv.Itoa(-int(n))
+		} else {
+			return strconv.Itoa(int(n))
+		}
+	}
+	// Large numbers should be split
+	const maxLength = 26
+
+	var (
+		out   = make([]byte, maxLength)
+		i     = maxLength - 1
+		comma = 0
+	)
+	for ; n > 0; i-- {
+		if comma == 3 {
+			comma = 0
+			out[i] = ','
+		} else {
+			comma++
+			out[i] = '0' + byte(n%10)
+			n /= 10
+		}
+	}
+	if neg {
+		out[i] = '-'
+		i--
+	}
+	return string(out[i+1:])
+}
+
+// formatLogfmtBigInt formats n with thousand separators.
+func formatLogfmtBigInt(n *big.Int) string {
+	if n.IsUint64() {
+		return FormatLogfmtUint64(n.Uint64())
+	}
+	if n.IsInt64() {
+		return FormatLogfmtInt64(n.Int64())
+	}
+
+	var (
+		text  = n.String()
+		buf   = make([]byte, len(text)+len(text)/3)
+		comma = 0
+		i     = len(buf) - 1
+	)
+	for j := len(text) - 1; j >= 0; j, i = j-1, i-1 {
+		c := text[j]
+
+		switch {
+		case c == '-':
+			buf[i] = c
+		case comma == 3:
+			buf[i] = ','
+			i--
+			comma = 0
+			fallthrough
+		default:
+			buf[i] = c
+			comma++
+		}
+	}
+	return string(buf[i+1:])
+}
+
 // escapeString checks if the provided string needs escaping/quoting, and
 // calls strconv.Quote if needed
 func escapeString(s string) string {

log/format_test.go

@@ -0,0 +1,95 @@
+package log
+
+import (
+	"math"
+	"math/big"
+	"math/rand"
+	"testing"
+)
+
+func TestPrettyInt64(t *testing.T) {
+	tests := []struct {
+		n int64
+		s string
+	}{
+		{0, "0"},
+		{10, "10"},
+		{-10, "-10"},
+		{100, "100"},
+		{-100, "-100"},
+		{1000, "1000"},
+		{-1000, "-1000"},
+		{10000, "10000"},
+		{-10000, "-10000"},
+		{99999, "99999"},
+		{-99999, "-99999"},
+		{100000, "100,000"},
+		{-100000, "-100,000"},
+		{1000000, "1,000,000"},
+		{-1000000, "-1,000,000"},
+		{math.MaxInt64, "9,223,372,036,854,775,807"},
+		{math.MinInt64, "-9,223,372,036,854,775,808"},
+	}
+	for i, tt := range tests {
+		if have := FormatLogfmtInt64(tt.n); have != tt.s {
+			t.Errorf("test %d: format mismatch: have %s, want %s", i, have, tt.s)
+		}
+	}
+}
+
+func TestPrettyUint64(t *testing.T) {
+	tests := []struct {
+		n uint64
+		s string
+	}{
+		{0, "0"},
+		{10, "10"},
+		{100, "100"},
+		{1000, "1000"},
+		{10000, "10000"},
+		{99999, "99999"},
+		{100000, "100,000"},
+		{1000000, "1,000,000"},
+		{math.MaxUint64, "18,446,744,073,709,551,615"},
+	}
+	for i, tt := range tests {
+		if have := FormatLogfmtUint64(tt.n); have != tt.s {
+			t.Errorf("test %d: format mismatch: have %s, want %s", i, have, tt.s)
+		}
+	}
+}
+
+func TestPrettyBigInt(t *testing.T) {
+	tests := []struct {
+		int string
+		s   string
+	}{
+		{"111222333444555678999", "111,222,333,444,555,678,999"},
+		{"-111222333444555678999", "-111,222,333,444,555,678,999"},
+		{"11122233344455567899900", "11,122,233,344,455,567,899,900"},
+		{"-11122233344455567899900", "-11,122,233,344,455,567,899,900"},
+	}
+
+	for _, tt := range tests {
+		v, _ := new(big.Int).SetString(tt.int, 10)
+		if have := formatLogfmtBigInt(v); have != tt.s {
+			t.Errorf("invalid output %s, want %s", have, tt.s)
+		}
+	}
+}
+
+var sink string
+
+func BenchmarkPrettyInt64Logfmt(b *testing.B) {
+	b.ReportAllocs()
+	for i := 0; i < b.N; i++ {
+		sink = FormatLogfmtInt64(rand.Int63())
+	}
+}
+
+func BenchmarkPrettyUint64Logfmt(b *testing.B) {
+	b.ReportAllocs()
+	for i := 0; i < b.N; i++ {
+		sink = FormatLogfmtUint64(rand.Uint64())
+	}
+}