Skip to content

Commit

Permalink
Remove appendValueOnConflict parameter as it's always true
Browse files Browse the repository at this point in the history
  • Loading branch information
felixbarny committed Jan 11, 2025
1 parent 16145d2 commit 69ae5ad
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 49 deletions.
32 changes: 15 additions & 17 deletions exporter/elasticsearchexporter/internal/objmodel/objmodel.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,12 @@ func (doc *Document) sort() {
// The filtering only keeps the last value for a key.
//
// Dedup ensure that keys are sorted.
func (doc *Document) Dedup(appendValueOnConflict bool) {
func (doc *Document) Dedup() {
// 1. Always ensure the fields are sorted, Dedup support requires
// Fields to be sorted.
doc.sort()

// 2. rename fields if a primitive value is overwritten by an object if appendValueOnConflict.
// 2. rename fields if a primitive value is overwritten by an object.
// For example the pair (path.x=1, path.x.a="test") becomes:
// (path.x.value=1, path.x.a="test").
//
Expand All @@ -236,19 +236,17 @@ func (doc *Document) Dedup(appendValueOnConflict bool) {
// field in favor of the `value` field in the document.
//
// This step removes potential conflicts when dedotting and serializing fields.
if appendValueOnConflict {
var renamed bool
for i := 0; i < len(doc.fields)-1; i++ {
key, nextKey := doc.fields[i].key, doc.fields[i+1].key
if len(key) < len(nextKey) && strings.HasPrefix(nextKey, key) && nextKey[len(key)] == '.' {
renamed = true
doc.fields[i].key = key + ".value"
}
}
if renamed {
doc.sort()
var renamed bool
for i := 0; i < len(doc.fields)-1; i++ {
key, nextKey := doc.fields[i].key, doc.fields[i+1].key
if len(key) < len(nextKey) && strings.HasPrefix(nextKey, key) && nextKey[len(key)] == '.' {
renamed = true
doc.fields[i].key = key + ".value"
}
}
if renamed {
doc.sort()
}

// 3. mark duplicates as 'ignore'
//
Expand All @@ -262,7 +260,7 @@ func (doc *Document) Dedup(appendValueOnConflict bool) {

// 4. fix objects that might be stored in arrays
for i := range doc.fields {
doc.fields[i].value.Dedup(appendValueOnConflict)
doc.fields[i].value.Dedup()
}
}

Expand Down Expand Up @@ -478,13 +476,13 @@ func (v *Value) sort() {
// Dedup recursively dedups keys in stored documents.
//
// NOTE: The value MUST be sorted.
func (v *Value) Dedup(appendValueOnConflict bool) {
func (v *Value) Dedup() {
switch v.kind {
case KindObject:
v.doc.Dedup(appendValueOnConflict)
v.doc.Dedup()
case KindArr:
for i := range v.arr {
v.arr[i].Dedup(appendValueOnConflict)
v.arr[i].Dedup()
}
}
}
Expand Down
40 changes: 11 additions & 29 deletions exporter/elasticsearchexporter/internal/objmodel/objmodel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,18 +86,16 @@ func TestObjectModel_CreateMap(t *testing.T) {

func TestObjectModel_Dedup(t *testing.T) {
tests := map[string]struct {
build func() Document
appendValueOnConflict bool
want Document
build func() Document
want Document
}{
"no duplicates": {
build: func() (doc Document) {
doc.AddInt("a", 1)
doc.AddInt("c", 3)
return doc
},
appendValueOnConflict: true,
want: Document{fields: []field{{"a", IntValue(1)}, {"c", IntValue(3)}}},
want: Document{fields: []field{{"a", IntValue(1)}, {"c", IntValue(3)}}},
},
"duplicate keys": {
build: func() (doc Document) {
Expand All @@ -106,8 +104,7 @@ func TestObjectModel_Dedup(t *testing.T) {
doc.AddInt("a", 2)
return doc
},
appendValueOnConflict: true,
want: Document{fields: []field{{"a", ignoreValue}, {"a", IntValue(2)}, {"c", IntValue(3)}}},
want: Document{fields: []field{{"a", ignoreValue}, {"a", IntValue(2)}, {"c", IntValue(3)}}},
},
"duplicate after flattening from map: namespace object at end": {
build: func() Document {
Expand All @@ -117,8 +114,7 @@ func TestObjectModel_Dedup(t *testing.T) {
am.PutEmptyMap("namespace").PutInt("a", 23)
return DocumentFromAttributes(am)
},
appendValueOnConflict: true,
want: Document{fields: []field{{"namespace.a", ignoreValue}, {"namespace.a", IntValue(23)}, {"toplevel", StringValue("test")}}},
want: Document{fields: []field{{"namespace.a", ignoreValue}, {"namespace.a", IntValue(23)}, {"toplevel", StringValue("test")}}},
},
"duplicate after flattening from map: namespace object at beginning": {
build: func() Document {
Expand All @@ -128,8 +124,7 @@ func TestObjectModel_Dedup(t *testing.T) {
am.PutStr("toplevel", "test")
return DocumentFromAttributes(am)
},
appendValueOnConflict: true,
want: Document{fields: []field{{"namespace.a", ignoreValue}, {"namespace.a", IntValue(42)}, {"toplevel", StringValue("test")}}},
want: Document{fields: []field{{"namespace.a", ignoreValue}, {"namespace.a", IntValue(42)}, {"toplevel", StringValue("test")}}},
},
"dedup in arrays": {
build: func() (doc Document) {
Expand All @@ -141,7 +136,6 @@ func TestObjectModel_Dedup(t *testing.T) {
doc.Add("arr", ArrValue(Value{kind: KindObject, doc: embedded}))
return doc
},
appendValueOnConflict: true,
want: Document{fields: []field{{"arr", ArrValue(Value{kind: KindObject, doc: Document{fields: []field{
{"a", ignoreValue},
{"a", IntValue(2)},
Expand All @@ -154,8 +148,7 @@ func TestObjectModel_Dedup(t *testing.T) {
doc.AddInt("namespace.a", 2)
return doc
},
appendValueOnConflict: true,
want: Document{fields: []field{{"namespace.a", IntValue(2)}, {"namespace.value", IntValue(1)}}},
want: Document{fields: []field{{"namespace.a", IntValue(2)}, {"namespace.value", IntValue(1)}}},
},
"dedup removes primitive if value exists": {
build: func() (doc Document) {
Expand All @@ -164,25 +157,14 @@ func TestObjectModel_Dedup(t *testing.T) {
doc.AddInt("namespace.value", 3)
return doc
},
appendValueOnConflict: true,
want: Document{fields: []field{{"namespace.a", IntValue(2)}, {"namespace.value", ignoreValue}, {"namespace.value", IntValue(3)}}},
},
"dedup without append value on conflict": {
build: func() (doc Document) {
doc.AddInt("namespace", 1)
doc.AddInt("namespace.a", 2)
doc.AddInt("namespace.value", 3)
return doc
},
appendValueOnConflict: false,
want: Document{fields: []field{{"namespace", IntValue(1)}, {"namespace.a", IntValue(2)}, {"namespace.value", IntValue(3)}}},
want: Document{fields: []field{{"namespace.a", IntValue(2)}, {"namespace.value", ignoreValue}, {"namespace.value", IntValue(3)}}},
},
}

for name, test := range tests {
t.Run(name, func(t *testing.T) {
doc := test.build()
doc.Dedup(test.appendValueOnConflict)
doc.Dedup()
assert.Equal(t, test.want, doc)
})
}
Expand Down Expand Up @@ -300,7 +282,7 @@ func TestDocument_Serialize_Flat(t *testing.T) {
m := pcommon.NewMap()
assert.NoError(t, m.FromRaw(test.attrs))
doc := DocumentFromAttributes(m)
doc.Dedup(true)
doc.Dedup()
err := doc.Serialize(&buf, false)
require.NoError(t, err)

Expand Down Expand Up @@ -361,7 +343,7 @@ func TestDocument_Serialize_Dedot(t *testing.T) {
m := pcommon.NewMap()
assert.NoError(t, m.FromRaw(test.attrs))
doc := DocumentFromAttributes(m)
doc.Dedup(true)
doc.Dedup()
err := doc.Serialize(&buf, true)
require.NoError(t, err)

Expand Down
6 changes: 3 additions & 3 deletions exporter/elasticsearchexporter/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (m *encodeModel) encodeLog(resource pcommon.Resource, resourceSchemaURL str
default:
document = m.encodeLogDefaultMode(resource, record, scope)
}
document.Dedup(true)
document.Dedup()

return document.Serialize(buf, m.dedot)
}
Expand Down Expand Up @@ -202,7 +202,7 @@ func (m *encodeModel) encodeLogECSMode(resource pcommon.Resource, record plog.Lo
}

func (m *encodeModel) encodeDocument(document objmodel.Document, buf *bytes.Buffer) error {
document.Dedup(true)
document.Dedup()

err := document.Serialize(buf, m.dedot)
if err != nil {
Expand Down Expand Up @@ -491,7 +491,7 @@ func (m *encodeModel) encodeSpan(resource pcommon.Resource, resourceSchemaURL st
default:
document = m.encodeSpanDefaultMode(resource, span, scope)
}
document.Dedup(true)
document.Dedup()
err := document.Serialize(buf, m.dedot)
return err
}
Expand Down

0 comments on commit 69ae5ad

Please sign in to comment.