-
Notifications
You must be signed in to change notification settings - Fork 1.2k
attribute: add String method to Value and KeyValue #7812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,11 @@ func (kv KeyValue) Valid() bool { | |||||||||||||||||||||||
| return kv.Key.Defined() && kv.Value.Type() != INVALID | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // String implements the Stringer interface, used when you pass an object to fmt.Println, etc. | ||||||||||||||||||||||||
| func (kv KeyValue) String() string { | ||||||||||||||||||||||||
| return "{" + string(kv.Key) + ":" + kv.Value.String() + "}" | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+21
to
+24
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Why? To follow prior art https://pkg.go.dev/go.opentelemetry.io/otel/log#KeyValue.String (more: #5117). |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Bool creates a KeyValue with a BOOL Value type. | ||||||||||||||||||||||||
| func Bool(k string, v bool) KeyValue { | ||||||||||||||||||||||||
| return Key(k).Bool(v) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -169,3 +169,84 @@ func TestIncorrectCast(t *testing.T) { | |
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestKeyValueString(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| kv attribute.KeyValue | ||
| want string | ||
| }{ | ||
| { | ||
| name: "int positive", | ||
| kv: attribute.Int("key", 42), | ||
| want: "{key:42}", | ||
| }, | ||
| { | ||
| name: "float64 negative", | ||
| kv: attribute.Float64("key", -3.14), | ||
| want: "{key:-3.14}", | ||
| }, | ||
| { | ||
| name: "string simple", | ||
| kv: attribute.String("key", "value"), | ||
| want: "{key:value}", | ||
| }, | ||
| { | ||
| name: "string empty", | ||
| kv: attribute.String("key", ""), | ||
| want: "{key:}", | ||
| }, | ||
| { | ||
| name: "string with spaces", | ||
| kv: attribute.String("key", "hello world"), | ||
| want: "{key:hello world}", | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I find it surprising that strings are not quoted, especially with embedded spaces like here. |
||
| }, | ||
| { | ||
| name: "bool slice", | ||
| kv: attribute.BoolSlice("key", []bool{true, false, true}), | ||
| want: "{key:[true false true]}", | ||
| }, | ||
| { | ||
| name: "int slice", | ||
| kv: attribute.IntSlice("key", []int{1, 2, 3}), | ||
| want: "{key:[1,2,3]}", | ||
| }, | ||
| { | ||
| name: "int64 slice", | ||
| kv: attribute.Int64Slice("key", []int64{1, 2, 3}), | ||
| want: "{key:[1,2,3]}", | ||
| }, | ||
| { | ||
| name: "float64 slice", | ||
| kv: attribute.Float64Slice("key", []float64{1.5, 2.5, 3.5}), | ||
| want: "{key:[1.5,2.5,3.5]}", | ||
| }, | ||
| { | ||
| name: "string slice", | ||
| kv: attribute.StringSlice("key", []string{"foo", "bar"}), | ||
| want: `{key:["foo","bar"]}`, | ||
| }, | ||
| { | ||
| name: "empty key", | ||
| kv: attribute.String("", "value"), | ||
| want: "{:value}", | ||
| }, | ||
| { | ||
| name: "invalid/uninitialized KeyValue", | ||
| kv: attribute.KeyValue{}, | ||
| want: "{:unknown}", | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this should print something clearer?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I propose |
||
| }, | ||
| { | ||
| name: "key with special characters", | ||
| kv: attribute.String("key-with-dashes", "value"), | ||
| want: "{key-with-dashes:value}", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| got := tt.kv.String() | ||
| assert.Equal(t, tt.want, got) | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -222,6 +222,12 @@ func (v Value) AsInterface() any { | |||||||||||||
| return unknownValueType{} | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // String implements the Stringer interface, used when you pass an object to fmt.Println, etc. | ||||||||||||||
| // Do not confuse with AsString, which you should call if you know the value is of String type. | ||||||||||||||
|
Comment on lines
+225
to
+226
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||
| func (v Value) String() string { | ||||||||||||||
| return v.Emit() | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we deprecate |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Emit returns a string representation of Value's data. | ||||||||||||||
| func (v Value) Emit() string { | ||||||||||||||
| switch v.Type() { | ||||||||||||||
|
|
||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -206,3 +206,104 @@ func TestAsSlice(t *testing.T) { | |
| ss2 := kv.Value.AsStringSlice() | ||
| assert.Equal(t, ss1, ss2) | ||
| } | ||
|
|
||
| func TestValueString(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| value attribute.Value | ||
| want string | ||
| }{ | ||
| { | ||
| name: "bool true", | ||
| value: attribute.BoolValue(true), | ||
| want: "true", | ||
| }, | ||
| { | ||
| name: "int64 positive", | ||
| value: attribute.Int64Value(42), | ||
| want: "42", | ||
| }, | ||
| { | ||
| name: "int negative", | ||
| value: attribute.IntValue(-42), | ||
| want: "-42", | ||
| }, | ||
| { | ||
| name: "float64 positive", | ||
| value: attribute.Float64Value(42.5), | ||
| want: "42.5", | ||
| }, | ||
| { | ||
| name: "float64 scientific notation", | ||
| value: attribute.Float64Value(1.23e-10), | ||
| want: "1.23e-10", | ||
| }, | ||
| { | ||
| name: "string empty", | ||
| value: attribute.StringValue(""), | ||
| want: "", | ||
| }, | ||
| { | ||
| name: "string with spaces", | ||
| value: attribute.StringValue("hello world"), | ||
| want: "hello world", | ||
| }, | ||
| { | ||
| name: "string with special characters", | ||
| value: attribute.StringValue("hello\nworld\t!"), | ||
| want: "hello\nworld\t!", | ||
| }, | ||
| { | ||
| name: "bool slice", | ||
| value: attribute.BoolSliceValue([]bool{true, false, true}), | ||
| want: "[true false true]", | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also it was surprising to me that this one has no commas, while the int, float and string slices do. |
||
| }, | ||
| { | ||
| name: "bool slice empty", | ||
| value: attribute.BoolSliceValue([]bool{}), | ||
| want: "[]", | ||
| }, | ||
| { | ||
| name: "int64 slice", | ||
| value: attribute.Int64SliceValue([]int64{1, -2, 3}), | ||
| want: "[1,-2,3]", | ||
| }, | ||
| { | ||
| name: "int64 slice empty", | ||
| value: attribute.Int64SliceValue([]int64{}), | ||
| want: "[]", | ||
| }, | ||
| { | ||
| name: "int slice", | ||
| value: attribute.IntSliceValue([]int{1, -2, 3}), | ||
| want: "[1,-2,3]", | ||
| }, | ||
| { | ||
| name: "float64 slice", | ||
| value: attribute.Float64SliceValue([]float64{1.5, -2.5, 3.0}), | ||
| want: "[1.5,-2.5,3]", | ||
| }, | ||
| { | ||
| name: "string slice", | ||
| value: attribute.StringSliceValue([]string{"foo", "bar", "baz"}), | ||
| want: `["foo","bar","baz"]`, | ||
| }, | ||
| { | ||
| name: "string slice with empty strings", | ||
| value: attribute.StringSliceValue([]string{"", "bar", ""}), | ||
| want: `["","bar",""]`, | ||
| }, | ||
| { | ||
| name: "invalid type", | ||
| value: attribute.Value{}, | ||
| want: "unknown", | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| got := tt.value.String() | ||
| assert.Equal(t, tt.want, got) | ||
| }) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.