From a911f8f9ee5403100ebca0a2a51ccc291f44973e Mon Sep 17 00:00:00 2001 From: Jesse <1430482733@qq.com> Date: Wed, 19 Oct 2022 15:07:58 +0800 Subject: [PATCH] fix(log): toString float32 precision loss and convert uint use `FormatUint` (#2461) * fix(log): toString float32 precision loss * convert uint to string --- contrib/log/aliyun/aliyun.go | 36 +++++++++--------- contrib/log/aliyun/aliyun_test.go | 59 +++++++++++++++++------------ contrib/log/tencent/tencent.go | 36 +++++++++--------- contrib/log/tencent/tencent_test.go | 44 +++++++++++++++++++++ log/level.go | 4 ++ log/level_test.go | 6 +++ 6 files changed, 123 insertions(+), 62 deletions(-) diff --git a/contrib/log/aliyun/aliyun.go b/contrib/log/aliyun/aliyun.go index 351de69d9..836960e06 100644 --- a/contrib/log/aliyun/aliyun.go +++ b/contrib/log/aliyun/aliyun.go @@ -16,6 +16,7 @@ import ( // Logger see more detail https://github.com/aliyun/aliyun-log-go-sdk type Logger interface { log.Logger + GetProducer() *producer.Producer Close() error } @@ -81,22 +82,16 @@ func (a *aliyunLog) Close() error { } func (a *aliyunLog) Log(level log.Level, keyvals ...interface{}) error { - buf := level.String() - levelTitle := "level" - - contents := make([]*sls.LogContent, 0) + contents := make([]*sls.LogContent, 0, len(keyvals)/2+1) contents = append(contents, &sls.LogContent{ - Key: &levelTitle, - Value: &buf, + Key: newString(level.Key()), + Value: newString(level.String()), }) - for i := 0; i < len(keyvals); i += 2 { - key := toString(keyvals[i]) - value := toString(keyvals[i+1]) contents = append(contents, &sls.LogContent{ - Key: &key, - Value: &value, + Key: newString(toString(keyvals[i])), + Value: newString(toString(keyvals[i+1])), }) } @@ -104,9 +99,7 @@ func (a *aliyunLog) Log(level log.Level, keyvals ...interface{}) error { Time: proto.Uint32(uint32(time.Now().Unix())), Contents: contents, } - - err := a.producer.SendLog(a.opts.project, a.opts.logstore, "", "", logInst) - return err + return a.producer.SendLog(a.opts.project, a.opts.logstore, "", "", logInst) } // NewAliyunLog new a aliyun logger with options. @@ -128,6 +121,11 @@ func NewAliyunLog(options ...Option) Logger { } } +// newString string convert to *string +func newString(s string) *string { + return &s +} + // toString convert any type to string func toString(v interface{}) string { var key string @@ -138,23 +136,23 @@ func toString(v interface{}) string { case float64: key = strconv.FormatFloat(v, 'f', -1, 64) case float32: - key = strconv.FormatFloat(float64(v), 'f', -1, 64) + key = strconv.FormatFloat(float64(v), 'f', -1, 32) case int: key = strconv.Itoa(v) case uint: - key = strconv.Itoa(int(v)) + key = strconv.FormatUint(uint64(v), 10) case int8: key = strconv.Itoa(int(v)) case uint8: - key = strconv.Itoa(int(v)) + key = strconv.FormatUint(uint64(v), 10) case int16: key = strconv.Itoa(int(v)) case uint16: - key = strconv.Itoa(int(v)) + key = strconv.FormatUint(uint64(v), 10) case int32: key = strconv.Itoa(int(v)) case uint32: - key = strconv.Itoa(int(v)) + key = strconv.FormatUint(uint64(v), 10) case int64: key = strconv.FormatInt(v, 10) case uint64: diff --git a/contrib/log/aliyun/aliyun_test.go b/contrib/log/aliyun/aliyun_test.go index e8e2624f0..c4bfde480 100644 --- a/contrib/log/aliyun/aliyun_test.go +++ b/contrib/log/aliyun/aliyun_test.go @@ -2,6 +2,7 @@ package aliyun import ( "math" + "reflect" "testing" "github.com/go-kratos/kratos/v2/log" @@ -99,34 +100,44 @@ func TestLog(t *testing.T) { } } +func TestNewString(t *testing.T) { + ptr := newString("") + if kind := reflect.TypeOf(ptr).Kind(); kind != reflect.Ptr { + t.Errorf("want type: %v, got type: %v", reflect.Ptr, kind) + } +} + func TestToString(t *testing.T) { tests := []struct { - in interface{} - out string + name string + in interface{} + out string }{ - {math.MaxFloat64, "17976931348623157000000000000000000000000000000000000" + - "000000000000000000000000000000000000000000000000000000000000000000000000000" + - "000000000000000000000000000000000000000000000000000000000000000000000000000" + - "000000000000000000000000000000000000000000000000000000000000000000000000000" + - "0000000000000000000000000000000"}, - {math.MaxFloat32, "340282346638528860000000000000000000000"}, - {1<<((32<<(^uint(0)>>63))-1) - 1, "9223372036854775807"}, - {uint(1<<(32<<(^uint(0)>>63)) - 1), "-1"}, - {math.MaxInt8, "127"}, - {math.MaxUint8, "255"}, - {math.MaxInt16, "32767"}, - {math.MaxUint16, "65535"}, - {math.MaxInt32, "2147483647"}, - {math.MaxUint32, "4294967295"}, - {math.MaxInt64, "9223372036854775807"}, - {uint64(math.MaxUint64), "18446744073709551615"}, - {"abc", "abc"}, - {false, "false"}, - {[]byte("abc"), "abc"}, + {"float64", 6.66, "6.66"}, + {"max float64", math.MaxFloat64, "179769313486231570000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"}, //nolint:lll + {"float32", float32(6.66), "6.66"}, + {"max float32", math.MaxFloat32, "340282346638528860000000000000000000000"}, + {"int", int(math.MaxInt64), "9223372036854775807"}, + {"uint", uint(math.MaxUint64), "18446744073709551615"}, + {"int8", math.MaxInt8, "127"}, + {"uint8", math.MaxUint8, "255"}, + {"int16", math.MaxInt16, "32767"}, + {"uint16", math.MaxUint16, "65535"}, + {"int32", math.MaxInt32, "2147483647"}, + {"uint32", math.MaxUint32, "4294967295"}, + {"int64", math.MaxInt64, "9223372036854775807"}, + {"uint64", uint64(math.MaxUint64), "18446744073709551615"}, + {"string", "abc", "abc"}, + {"bool", false, "false"}, + {"[]byte", []byte("abc"), "abc"}, + {"struct", struct{ Name string }{}, `{"Name":""}`}, } for _, test := range tests { - if toString(test.in) != test.out { - t.Fatalf("want: %s, got: %s", test.out, toString(test.in)) - } + t.Run(test.name, func(t *testing.T) { + out := toString(test.in) + if test.out != out { + t.Fatalf("want: %s, got: %s", test.out, out) + } + }) } } diff --git a/contrib/log/tencent/tencent.go b/contrib/log/tencent/tencent.go index 8becf7883..3016926e0 100644 --- a/contrib/log/tencent/tencent.go +++ b/contrib/log/tencent/tencent.go @@ -14,8 +14,8 @@ import ( type Logger interface { log.Logger - GetProducer() *cls.AsyncProducerClient + GetProducer() *cls.AsyncProducerClient Close() error } @@ -66,25 +66,20 @@ func WithAccessSecret(as string) Option { type Option func(cls *options) func (log *tencentLog) Close() error { - err := log.producer.Close(5000) - return err + return log.producer.Close(5000) } func (log *tencentLog) Log(level log.Level, keyvals ...interface{}) error { - buf := level.String() - levelTitle := "level" - contents := make([]*cls.Log_Content, 0) + contents := make([]*cls.Log_Content, 0, len(keyvals)/2+1) contents = append(contents, &cls.Log_Content{ - Key: &levelTitle, - Value: &buf, + Key: newString(level.Key()), + Value: newString(level.String()), }) for i := 0; i < len(keyvals); i += 2 { - key := toString(keyvals[i]) - value := toString(keyvals[i+1]) contents = append(contents, &cls.Log_Content{ - Key: &key, - Value: &value, + Key: newString(toString(keyvals[i])), + Value: newString(toString(keyvals[i+1])), }) } @@ -92,8 +87,7 @@ func (log *tencentLog) Log(level log.Level, keyvals ...interface{}) error { Time: proto.Int64(time.Now().Unix()), Contents: contents, } - err := log.producer.SendLog(log.opts.topicID, logInst, nil) - return err + return log.producer.SendLog(log.opts.topicID, logInst, nil) } func NewLogger(options ...Option) (Logger, error) { @@ -115,6 +109,10 @@ func NewLogger(options ...Option) (Logger, error) { }, nil } +func newString(s string) *string { + return &s +} + // toString convert any type to string func toString(v interface{}) string { var key string @@ -125,23 +123,23 @@ func toString(v interface{}) string { case float64: key = strconv.FormatFloat(v, 'f', -1, 64) case float32: - key = strconv.FormatFloat(float64(v), 'f', -1, 64) + key = strconv.FormatFloat(float64(v), 'f', -1, 32) case int: key = strconv.Itoa(v) case uint: - key = strconv.Itoa(int(v)) + key = strconv.FormatUint(uint64(v), 10) case int8: key = strconv.Itoa(int(v)) case uint8: - key = strconv.Itoa(int(v)) + key = strconv.FormatUint(uint64(v), 10) case int16: key = strconv.Itoa(int(v)) case uint16: - key = strconv.Itoa(int(v)) + key = strconv.FormatUint(uint64(v), 10) case int32: key = strconv.Itoa(int(v)) case uint32: - key = strconv.Itoa(int(v)) + key = strconv.FormatUint(uint64(v), 10) case int64: key = strconv.FormatInt(v, 10) case uint64: diff --git a/contrib/log/tencent/tencent_test.go b/contrib/log/tencent/tencent_test.go index b37c1b22c..76bc13bdd 100644 --- a/contrib/log/tencent/tencent_test.go +++ b/contrib/log/tencent/tencent_test.go @@ -1,6 +1,8 @@ package tencent import ( + "math" + "reflect" "testing" "github.com/go-kratos/kratos/v2/log" @@ -101,3 +103,45 @@ func TestLog(t *testing.T) { t.Errorf("Log() returns error:%v", err) } } + +func TestNewString(t *testing.T) { + ptr := newString("") + if kind := reflect.TypeOf(ptr).Kind(); kind != reflect.Ptr { + t.Errorf("want type: %v, got type: %v", reflect.Ptr, kind) + } +} + +func TestToString(t *testing.T) { + tests := []struct { + name string + in interface{} + out string + }{ + {"float64", 6.66, "6.66"}, + {"max float64", math.MaxFloat64, "179769313486231570000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"}, //nolint:lll + {"float32", float32(6.66), "6.66"}, + {"max float32", math.MaxFloat32, "340282346638528860000000000000000000000"}, + {"int", int(math.MaxInt64), "9223372036854775807"}, + {"uint", uint(math.MaxUint64), "18446744073709551615"}, + {"int8", math.MaxInt8, "127"}, + {"uint8", math.MaxUint8, "255"}, + {"int16", math.MaxInt16, "32767"}, + {"uint16", math.MaxUint16, "65535"}, + {"int32", math.MaxInt32, "2147483647"}, + {"uint32", math.MaxUint32, "4294967295"}, + {"int64", math.MaxInt64, "9223372036854775807"}, + {"uint64", uint64(math.MaxUint64), "18446744073709551615"}, + {"string", "abc", "abc"}, + {"bool", false, "false"}, + {"[]byte", []byte("abc"), "abc"}, + {"struct", struct{ Name string }{}, `{"Name":""}`}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + out := toString(test.in) + if test.out != out { + t.Fatalf("want: %s, got: %s", test.out, out) + } + }) + } +} diff --git a/log/level.go b/log/level.go index 22f41c787..1a9664236 100644 --- a/log/level.go +++ b/log/level.go @@ -21,6 +21,10 @@ const ( LevelFatal ) +func (l Level) Key() string { + return LevelKey +} + func (l Level) String() string { switch l { case LevelDebug: diff --git a/log/level_test.go b/log/level_test.go index d36882912..193503c68 100644 --- a/log/level_test.go +++ b/log/level_test.go @@ -2,6 +2,12 @@ package log import "testing" +func TestLevel_Key(t *testing.T) { + if LevelInfo.Key() != LevelKey { + t.Errorf("want: %s, got: %s", LevelKey, LevelInfo.Key()) + } +} + func TestLevel_String(t *testing.T) { tests := []struct { name string