Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- Add service detection with `WithService` in `go.opentelemetry.io/otel/sdk/resource`. (#7642)

### Changed

- Rename the `INVALID Type` to `EMPTY` in `go.opentelemetry.io/otel/attribute` to reflect that an empty value is now a valid value. (#8038)
Comment thread
pellared marked this conversation as resolved.
Outdated

### Deprecated

- Deprecate `INVALID` in `go.opentelemetry.io/otel/attribute`. Use `EMPTY` instead. (#8038)

<!-- Released section -->
<!-- Don't change this section unless doing release -->

Expand Down
4 changes: 3 additions & 1 deletion attribute/hash.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const (
int64SliceID uint64 = 3762322556277578591 // "_[]int64" (little endian)
float64SliceID uint64 = 7308324551835016539 // "[]double" (little endian)
stringSliceID uint64 = 7453010373645655387 // "[]string" (little endian)
emptyID uint64 = 7305809155345288421 // "__empty_" (little endian)
)

// hashKVs returns a new xxHash64 hash of kvs.
Expand Down Expand Up @@ -80,7 +81,8 @@ func hashKV(h xxhash.Hash, kv KeyValue) xxhash.Hash {
for i := 0; i < rv.Len(); i++ {
h = h.String(rv.Index(i).String())
}
case INVALID:
case EMPTY:
h = h.Uint64(emptyID)
default:
// Logging is an alternative, but using the internal logger here
// causes an import cycle so it is not done.
Expand Down
12 changes: 10 additions & 2 deletions attribute/hash_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ var keyVals = []func(string) KeyValue{
func(k string) KeyValue { return String(k, "bar") },
func(k string) KeyValue { return StringSlice(k, []string{"foo", "bar", "baz"}) },
func(k string) KeyValue { return StringSlice(k, []string{"[]i1"}) },
func(k string) KeyValue { return KeyValue{Key: Key(k)} }, // Empty value.
}

func TestHashKVsEquality(t *testing.T) {
Expand Down Expand Up @@ -155,8 +156,8 @@ func FuzzHashKVs(f *testing.F) {
0, int64(0), math.Inf(1), false, uint8(2))

f.Fuzz(func(t *testing.T, k1, k2, k3, k4, k5, s string, i int, i64 int64, fVal float64, b bool, sliceType uint8) {
// Test variable number of attributes (0-10).
numAttrs := len(k1) % 11 // Use key length to determine number of attributes.
// Test variable number of attributes (0-11).
numAttrs := len(k1) % 12 // Use key length to determine number of attributes.
if numAttrs == 0 && k1 == "" {
// Test empty set.
h := hashKVs(nil)
Expand Down Expand Up @@ -250,6 +251,11 @@ func FuzzHashKVs(f *testing.F) {
kvs = append(kvs, String("empty", ""))
}

// Add empty value.
if numAttrs > 10 {
kvs = append(kvs, KeyValue{Key: Key("empty_value")})
}

// Sort to ensure consistent ordering (as Set would do).
slices.SortFunc(kvs, func(a, b KeyValue) int {
return cmp.Compare(string(a.Key), string(b.Key))
Expand Down Expand Up @@ -301,6 +307,8 @@ func FuzzHashKVs(f *testing.F) {
if !math.IsNaN(val) && !math.IsInf(val, 0) {
modifiedKvs[0] = Float64(string(modifiedKvs[0].Key), val+1.0)
}
case EMPTY:
modifiedKvs[0] = String(string(modifiedKvs[0].Key), "not_empty")
}

h3 := hashKVs(modifiedKvs)
Expand Down
5 changes: 5 additions & 0 deletions attribute/key_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ func TestEmit(t *testing.T) {
v: attribute.StringSliceValue([]string{"foo", "bar"}),
want: `["foo","bar"]`,
},
{
name: `test Key.Emit() can emit a string representing self.EMPTY`,
v: attribute.Value{},
want: "",
},
} {
t.Run(testcase.name, func(t *testing.T) {
// proto: func (v attribute.Value) Emit() string {
Expand Down
2 changes: 1 addition & 1 deletion attribute/kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type KeyValue struct {

// Valid reports whether kv is a valid OpenTelemetry attribute.
func (kv KeyValue) Valid() bool {
return kv.Key.Defined() && kv.Value.Type() != INVALID
return kv.Key.Defined()
}

// Bool creates a KeyValue with a BOOL Value type.
Expand Down
10 changes: 7 additions & 3 deletions attribute/kv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ func TestKeyValueValid(t *testing.T) {
kv: attribute.Key("").Bool(true),
},
{
desc: "INVALID value type should be invalid",
valid: false,
desc: "EMPTY value type should be invalid",
Comment thread
pellared marked this conversation as resolved.
Outdated
valid: true,
kv: attribute.KeyValue{
Key: attribute.Key("valid key"),
// Default type is INVALID.
// Default type is EMPTY.
Value: attribute.Value{},
},
},
Expand Down Expand Up @@ -152,6 +152,10 @@ func TestIncorrectCast(t *testing.T) {
name: "StringSlice",
val: attribute.BoolSliceValue([]bool{true}),
},
{
name: "Empty",
val: attribute.Value{},
},
}
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions attribute/type_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 13 additions & 3 deletions attribute/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
type Type int // nolint: revive // redefines builtin Type.

// Value represents the value part in key-value pairs.
//
// Note that the zero value is a valid empty value.
type Value struct {
vtype Type
numeric uint64
Expand All @@ -26,8 +28,8 @@ type Value struct {
}

const (
// INVALID is used for a Value with no value set.
INVALID Type = iota
// EMPTY is used for a Value with no value set.
EMPTY Type = iota
// BOOL is a boolean Type Value.
BOOL
Comment thread
pellared marked this conversation as resolved.
// INT64 is a 64-bit signed integral Type Value.
Expand All @@ -44,6 +46,10 @@ const (
FLOAT64SLICE
// STRINGSLICE is a slice of strings Type Value.
STRINGSLICE
// INVALID is used for a Value with no value set.
//
// Deprecated: Use EMPTY instead as an empty value is a valid value.
INVALID = EMPTY
)

// BoolValue creates a BOOL Value.
Expand Down Expand Up @@ -217,6 +223,8 @@ func (v Value) AsInterface() any {
return v.stringly
case STRINGSLICE:
return v.asStringSlice()
case EMPTY:
return nil
}
return unknownValueType{}
}
Expand Down Expand Up @@ -252,8 +260,10 @@ func (v Value) Emit() string {
return string(j)
case STRING:
return v.stringly
case EMPTY:
return ""
default:
return "unknown"
panic(fmt.Sprintf("attribute.Value.Emit: unexpected type %v", v.Type()))
Comment thread
pellared marked this conversation as resolved.
Outdated
Comment thread
pellared marked this conversation as resolved.
Outdated
}
}

Expand Down
17 changes: 14 additions & 3 deletions attribute/value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,17 @@ func TestValue(t *testing.T) {
wantType: attribute.STRINGSLICE,
wantValue: []string{"forty-two", "negative three", "twelve"},
},
{
name: "empty value",
value: attribute.Value{},
wantType: attribute.EMPTY,
wantValue: nil,
},
} {
t.Logf("Running test case %s", testcase.name)
if testcase.value.Type() != testcase.wantType {
t.Errorf("wrong value type, got %#v, expected %#v", testcase.value.Type(), testcase.wantType)
}
if testcase.wantType == attribute.INVALID {
continue
}
got := testcase.value.AsInterface()
if diff := cmp.Diff(testcase.wantValue, got); diff != "" {
t.Errorf("+got, -want: %s", diff)
Expand Down Expand Up @@ -143,6 +146,10 @@ func TestEquivalence(t *testing.T) {
attribute.StringSlice("StringSlice", []string{"one", "two", "three"}),
attribute.StringSlice("StringSlice", []string{"one", "two", "three"}),
},
{
attribute.KeyValue{Key: "Empty"},
attribute.KeyValue{Key: "Empty"},
},
}

t.Run("Distinct", func(t *testing.T) {
Expand Down Expand Up @@ -234,6 +241,10 @@ func TestNotEquivalence(t *testing.T) {
attribute.StringSlice("StringSlice", []string{"one", "two", "three"}),
attribute.StringSlice("StringSlice", []string{"one", "two"}),
},
{
attribute.KeyValue{Key: "Empty"},
attribute.String("Empty", ""),
},
}

t.Run("Distinct", func(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion log/keyvalue.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ func (a KeyValue) String() string {
// ValueFromAttribute converts [attribute.Value] to [Value].
func ValueFromAttribute(value attribute.Value) Value {
switch value.Type() {
case attribute.INVALID:
case attribute.EMPTY:
return Value{}
case attribute.BOOL:
return BoolValue(value.AsBool())
Expand Down
Loading