Do not use KeyValue fields directly and use KeyValues as decorator on…

…ly (jaegertracing#810) * Do not use KeyValue fields directly Signed-off-by: Yuri Shkuro <[email protected]> * Use KeyValues as decorator only Signed-off-by: Yuri Shkuro <[email protected]>
liukgg · May 7, 2018 · 5eb2922 · 5eb2922
1 parent 4106c29
commit 5eb2922
Show file tree

Hide file tree

Showing 11 changed files with 35 additions and 23 deletions.
diff --git a/cmd/collector/app/sanitizer/utf8_sanitizer_test.go b/cmd/collector/app/sanitizer/utf8_sanitizer_test.go
@@ -121,7 +121,7 @@ func TestUTF8Sanitizer_SanitizeProcessTags(t *testing.T) {
 			},
 		},
 	)
-	_, exists := actual.Process.Tags.FindByKey(invalidTagKey)
+	_, exists := model.KeyValues(actual.Process.Tags).FindByKey(invalidTagKey)
 	assert.True(t, exists)
 
 }
@@ -133,7 +133,7 @@ func TestUTF8Sanitizer_SanitizeTags(t *testing.T) {
 			Process: &model.Process{},
 		},
 	)
-	_, exists := actual.Tags.FindByKey(invalidTagKey)
+	_, exists := model.KeyValues(actual.Tags).FindByKey(invalidTagKey)
 	assert.True(t, exists)
 
 }

diff --git a/model/adjuster/clockskew.go b/model/adjuster/clockskew.go
@@ -75,7 +75,7 @@ type node struct {
 //
 // TODO convert process tags to a canonical format somewhere else
 func hostKey(span *model.Span) string {
-	if tag, ok := span.Process.Tags.FindByKey("ip"); ok {
+	if tag, ok := model.KeyValues(span.Process.Tags).FindByKey("ip"); ok {
 		if tag.VType == model.StringType {
 			return tag.VStr
 		}

diff --git a/model/adjuster/ip_tag.go b/model/adjuster/ip_tag.go
@@ -41,7 +41,7 @@ func IPTagAdjuster() Adjuster {
 				continue
 			}
 			var buf [4]byte
-			binary.BigEndian.PutUint32(buf[:], uint32(tag.VNum))
+			binary.BigEndian.PutUint32(buf[:], uint32(tag.Int64()))
 			var sBuf bytes.Buffer
 			for i, b := range buf {
 				if i > 0 {
@@ -57,7 +57,7 @@ func IPTagAdjuster() Adjuster {
 		for _, span := range trace.Spans {
 			adjustTags(span.Tags)
 			adjustTags(span.Process.Tags)
-			span.Process.Tags.Sort()
+			model.KeyValues(span.Process.Tags).Sort()
 		}
 		return trace, nil
 	})

diff --git a/model/adjuster/ip_tag_test.go b/model/adjuster/ip_tag_test.go
@@ -53,12 +53,12 @@ func TestIPTagAdjuster(t *testing.T) {
 		model.String("peer.ipv4", "not integer"),
 		model.String("peer.ipv4", "1.2.3.4"),
 	}
-	assert.Equal(t, expectedSpanTags, trace.Spans[0].Tags)
+	assert.Equal(t, expectedSpanTags, model.KeyValues(trace.Spans[0].Tags))
 
 	expectedProcessTags := model.KeyValues{
 		model.Int64("a", 42),
 		model.String("ip", "1.2.3.4"), // sorted
 		model.String("ip", "not integer"),
 	}
-	assert.Equal(t, expectedProcessTags, trace.Spans[0].Process.Tags)
+	assert.Equal(t, expectedProcessTags, model.KeyValues(trace.Spans[0].Process.Tags))
 }
diff --git a/model/converter/json/from_domain_test.go b/model/converter/json/from_domain_test.go
@@ -19,6 +19,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"math"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -41,7 +42,7 @@ func TestFromDomain(t *testing.T) {
 		testOutput(t, i, outStr, uiTrace, false)
 	}
 	// this is just to confirm the uint64 representation of float64(72.5) used as a "temperature" tag
-	assert.Equal(t, int64(4634802150889750528), model.Float64("x", 72.5).VNum)
+	assert.Equal(t, int64(4634802150889750528), int64(math.Float64bits(72.5)))
 }
 
 func TestFromDomainEmbedProcess(t *testing.T) {

diff --git a/model/converter/json/to_domain_test.go b/model/converter/json/to_domain_test.go
@@ -18,6 +18,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"math"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -44,7 +45,7 @@ func TestToDomainEmbeddedProcess(t *testing.T) {
 		CompareModelSpans(t, &expectedSpan, actualSpan)
 	}
 	// this is just to confirm the uint64 representation of float64(72.5) used as a "temperature" tag
-	assert.Equal(t, int64(4634802150889750528), model.Float64("x", 72.5).VNum)
+	assert.Equal(t, int64(4634802150889750528), int64(math.Float64bits(72.5)))
 }
 
 func createGoodSpan(i int) (jModel.Span, error) {

diff --git a/model/converter/thrift/jaeger/from_domain.go b/model/converter/thrift/jaeger/from_domain.go
@@ -72,10 +72,7 @@ func (d domainToJaegerTransformer) keyValueToTag(kv *model.KeyValue) *jaeger.Tag
 	}
 
 	if kv.VType == model.BoolType {
-		boolValue := false
-		if kv.VNum > 0 {
-			boolValue = true
-		}
+		boolValue := kv.Bool()
 		return &jaeger.Tag{
 			Key:   kv.Key,
 			VType: jaeger.TagType_BOOL,

diff --git a/model/converter/thrift/zipkin/to_domain_test.go b/model/converter/thrift/zipkin/to_domain_test.go
@@ -20,6 +20,7 @@ import (
 	"encoding/binary"
 	"encoding/json"
 	"fmt"
+	"math"
 	"os"
 	"testing"
 	"time"
@@ -118,11 +119,11 @@ func TestToDomainMultipleSpanKinds(t *testing.T) {
 		require.Nil(t, err)
 
 		assert.Equal(t, 2, len(trace.Spans))
-		assert.Equal(t, 1, trace.Spans[0].Tags.Len())
+		assert.Equal(t, 1, len(trace.Spans[0].Tags))
 		assert.Equal(t, test.tagFirst.Key, trace.Spans[0].Tags[0].Key)
 		assert.Equal(t, string(test.tagFirst.Value.(ext.SpanKindEnum)), trace.Spans[0].Tags[0].VStr)
 
-		assert.Equal(t, 1, trace.Spans[1].Tags.Len())
+		assert.Equal(t, 1, len(trace.Spans[1].Tags))
 		assert.Equal(t, test.tagSecond.Key, trace.Spans[1].Tags[0].Key)
 		assert.Equal(t, time.Duration(1000), trace.Spans[1].Duration)
 		assert.Equal(t, string(test.tagSecond.Value.(ext.SpanKindEnum)), trace.Spans[1].Tags[0].VStr)
@@ -149,8 +150,8 @@ func TestValidateBase64Values(t *testing.T) {
 	assert.Equal(t, "MDk=", numberToBase64(int16(12345)))
 	assert.Equal(t, "AAAwOQ==", numberToBase64(int32(12345)))
 	assert.Equal(t, "AAAAAAAAMDk=", numberToBase64(int64(12345)))
-	assert.Equal(t, "QMgcgAAAAAA=", numberToBase64(model.Float64("x", 12345).VNum))
-	assert.Equal(t, int64(4668012349850910720), model.Float64("x", 12345).VNum)
+	assert.Equal(t, "QMgcgAAAAAA=", numberToBase64(int64(math.Float64bits(12345))))
+	assert.Equal(t, int64(4668012349850910720), int64(math.Float64bits(12345)), "sanity check")
 }
 
 func loadZipkinSpans(t *testing.T, file string) []*z.Span {

diff --git a/model/keyvalues_test.go b/model/keyvalues_test.go
@@ -131,3 +131,15 @@ func BenchmarkKeyValuesSort(b *testing.B) {
 		list.Sort()
 	}
 }
+
+// No memory allocations from wrapping sliceinto model.KeyValues()
+// 0.53 ns/op	       0 B/op	       0 allocs/op
+func BenchmarkKeyValuesWrapping(b *testing.B) {
+	kv := []model.KeyValue{
+		model.String("x", "y"),
+		model.Int64("n", 42),
+	}
+	for i := 0; i < b.N; i++ {
+		model.KeyValues(kv).Len()
+	}
+}
diff --git a/model/process.go b/model/process.go
@@ -18,8 +18,8 @@ import "io"
 
 // Process describes an instance of an application or service that emits tracing data.
 type Process struct {
-	ServiceName string    `json:"serviceName"`
-	Tags        KeyValues `json:"tags,omitempty"`
+	ServiceName string     `json:"serviceName"`
+	Tags        []KeyValue `json:"tags,omitempty"`
 }
 
 // NewProcess creates a new Process for given serviceName and tags.
@@ -37,13 +37,13 @@ func (p *Process) Equal(other *Process) bool {
 	if p.ServiceName != other.ServiceName {
 		return false
 	}
-	return p.Tags.Equal(other.Tags)
+	return KeyValues(p.Tags).Equal(other.Tags)
 }
 
 // Hash implements Hash from Hashable.
 func (p *Process) Hash(w io.Writer) (err error) {
 	if _, err := w.Write([]byte(p.ServiceName)); err != nil {
 		return err
 	}
-	return p.Tags.Hash(w)
+	return KeyValues(p.Tags).Hash(w)
 }
diff --git a/model/span.go b/model/span.go
@@ -53,7 +53,7 @@ type Span struct {
 	Flags         Flags         `json:"flags,omitempty"`
 	StartTime     time.Time     `json:"startTime"`
 	Duration      time.Duration `json:"duration"`
-	Tags          KeyValues     `json:"tags,omitempty"`
+	Tags          []KeyValue    `json:"tags,omitempty"`
 	Logs          []Log         `json:"logs,omitempty"`
 	Process       *Process      `json:"process"`
 	Warnings      []string      `json:"warnings,omitempty"`
@@ -69,7 +69,7 @@ func (s *Span) Hash(w io.Writer) (err error) {
 
 // HasSpanKind returns true if the span has a `span.kind` tag set to `kind`.
 func (s *Span) HasSpanKind(kind ext.SpanKindEnum) bool {
-	if tag, ok := s.Tags.FindByKey(string(ext.SpanKind)); ok {
+	if tag, ok := KeyValues(s.Tags).FindByKey(string(ext.SpanKind)); ok {
 		return tag.AsString() == string(kind)
 	}
 	return false