Add missing tests

Signed-off-by: Yuri Shkuro <ys@uber.com>
jaegertracing · May 18, 2018 · beb4a57 · beb4a57
1 parent 1babc1f
commit beb4a57
Show file tree

Hide file tree

Showing 11 changed files with 144 additions and 74 deletions.
diff --git a/model/converter/json/fixtures/domain_es_01.json b/model/converter/json/fixtures/domain_es_01.json
@@ -3,20 +3,20 @@
   "spanID": "2",
   "operationName": "test-general-conversion",
   "references": [
-    {
-      "refType": "child-of",
-      "traceID": "ff",
-      "spanID": "ff"
-    },
     {
       "refType": "child-of",
       "traceID": "1",
-      "spanID": "2"
+      "spanID": "3"
     },
     {
       "refType": "follows-from",
       "traceID": "1",
-      "spanID": "2"
+      "spanID": "4"
+    },
+    {
+      "refType": "child-of",
+      "traceID": "ff",
+      "spanID": "ff"
     }
   ],
   "flags": 1,

diff --git a/model/converter/json/fixtures/es_01.json b/model/converter/json/fixtures/es_01.json
@@ -4,20 +4,20 @@
   "flags": 1,
   "operationName": "test-general-conversion",
   "references": [
-    {
-      "refType": "CHILD_OF",
-      "traceID": "ff",
-      "spanID": "ff"
-    },
     {
       "refType": "CHILD_OF",
       "traceID": "1",
-      "spanID": "2"
+      "spanID": "3"
     },
     {
       "refType": "FOLLOWS_FROM",
       "traceID": "1",
-      "spanID": "2"
+      "spanID": "4"
+    },
+    {
+      "refType": "CHILD_OF",
+      "traceID": "ff",
+      "spanID": "ff"
     }
   ],
   "startTime": 1485467191639875,

diff --git a/model/converter/json/to_domain_test.go b/model/converter/json/to_domain_test.go
@@ -28,10 +28,20 @@ import (
 	jModel "github.com/jaegertracing/jaeger/model/json"
 )
 
-func TestToDomainEmbeddedProcess(t *testing.T) {
+func TestToDomain(t *testing.T) {
+	testToDomain(t, false)
+	testToDomain(t, true)
+	// this is just to confirm the uint64 representation of float64(72.5) used as a "temperature" tag
+	assert.Equal(t, int64(4634802150889750528), int64(math.Float64bits(72.5)))
+}
+
+func testToDomain(t *testing.T, testParentSpanID bool) {
 	for i := 1; i <= NumberOfFixtures; i++ {
-		span, err := createGoodSpan(i)
+		span, err := loadESSpanFixture(i)
 		require.NoError(t, err)
+		if testParentSpanID {
+			span.ParentSpanID = "3"
+		}
 
 		actualSpan, err := SpanToDomain(&span)
 		require.NoError(t, err)
@@ -44,11 +54,9 @@ 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), int64(math.Float64bits(72.5)))
 }
 
-func createGoodSpan(i int) (jModel.Span, error) {
+func loadESSpanFixture(i int) (jModel.Span, error) {
 	in := fmt.Sprintf("fixtures/es_%02d.json", i)
 	inStr, err := ioutil.ReadFile(in)
 	if err != nil {
@@ -72,7 +80,7 @@ func failingSpanTransformAnyMsg(t *testing.T, embeddedSpan *jModel.Span) {
 }
 
 func TestFailureBadTypeTags(t *testing.T) {
-	badTagESSpan, err := createGoodSpan(1)
+	badTagESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 
 	badTagESSpan.Tags = []jModel.KeyValue{
@@ -85,7 +93,7 @@ func TestFailureBadTypeTags(t *testing.T) {
 }
 
 func TestFailureBadBoolTags(t *testing.T) {
-	badTagESSpan, err := createGoodSpan(1)
+	badTagESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 
 	badTagESSpan.Tags = []jModel.KeyValue{
@@ -99,7 +107,7 @@ func TestFailureBadBoolTags(t *testing.T) {
 }
 
 func TestFailureBadIntTags(t *testing.T) {
-	badTagESSpan, err := createGoodSpan(1)
+	badTagESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 
 	badTagESSpan.Tags = []jModel.KeyValue{
@@ -113,7 +121,7 @@ func TestFailureBadIntTags(t *testing.T) {
 }
 
 func TestFailureBadFloatTags(t *testing.T) {
-	badTagESSpan, err := createGoodSpan(1)
+	badTagESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 
 	badTagESSpan.Tags = []jModel.KeyValue{
@@ -127,7 +135,7 @@ func TestFailureBadFloatTags(t *testing.T) {
 }
 
 func TestFailureBadBinaryTags(t *testing.T) {
-	badTagESSpan, err := createGoodSpan(1)
+	badTagESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 
 	badTagESSpan.Tags = []jModel.KeyValue{
@@ -141,7 +149,7 @@ func TestFailureBadBinaryTags(t *testing.T) {
 }
 
 func TestFailureBadLogs(t *testing.T) {
-	badLogsESSpan, err := createGoodSpan(1)
+	badLogsESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 	badLogsESSpan.Logs = []jModel.Log{
 		{
@@ -169,7 +177,7 @@ func TestRevertKeyValueOfType(t *testing.T) {
 }
 
 func TestFailureBadRefs(t *testing.T) {
-	badRefsESSpan, err := createGoodSpan(1)
+	badRefsESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 	badRefsESSpan.References = []jModel.Reference{
 		{
@@ -181,7 +189,7 @@ func TestFailureBadRefs(t *testing.T) {
 }
 
 func TestFailureBadTraceIDRefs(t *testing.T) {
-	badRefsESSpan, err := createGoodSpan(1)
+	badRefsESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 	badRefsESSpan.References = []jModel.Reference{
 		{
@@ -194,7 +202,7 @@ func TestFailureBadTraceIDRefs(t *testing.T) {
 }
 
 func TestFailureBadSpanIDRefs(t *testing.T) {
-	badRefsESSpan, err := createGoodSpan(1)
+	badRefsESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 	badRefsESSpan.References = []jModel.Reference{
 		{
@@ -207,7 +215,7 @@ func TestFailureBadSpanIDRefs(t *testing.T) {
 }
 
 func TestFailureBadProcess(t *testing.T) {
-	badProcessESSpan, err := createGoodSpan(1)
+	badProcessESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 
 	badTags := []jModel.KeyValue{
@@ -224,23 +232,30 @@ func TestFailureBadProcess(t *testing.T) {
 }
 
 func TestProcessPointer(t *testing.T) {
-	badProcessESSpan, err := createGoodSpan(1)
+	badProcessESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 
 	badProcessESSpan.Process = nil
 	failingSpanTransform(t, &badProcessESSpan, "Process is nil")
 }
 
 func TestFailureBadTraceID(t *testing.T) {
-	badTraceIDESSpan, err := createGoodSpan(1)
+	badTraceIDESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 	badTraceIDESSpan.TraceID = "zz"
 	failingSpanTransformAnyMsg(t, &badTraceIDESSpan)
 }
 
 func TestFailureBadSpanID(t *testing.T) {
-	badSpanIDESSpan, err := createGoodSpan(1)
+	badSpanIDESSpan, err := loadESSpanFixture(1)
 	require.NoError(t, err)
 	badSpanIDESSpan.SpanID = "zz"
 	failingSpanTransformAnyMsg(t, &badSpanIDESSpan)
 }
+
+func TestFailureBadParentSpanID(t *testing.T) {
+	badParentSpanIDESSpan, err := loadESSpanFixture(1)
+	require.NoError(t, err)
+	badParentSpanIDESSpan.ParentSpanID = "zz"
+	failingSpanTransformAnyMsg(t, &badParentSpanIDESSpan)
+}
diff --git a/model/converter/thrift/jaeger/to_domain.go b/model/converter/thrift/jaeger/to_domain.go
@@ -57,7 +57,6 @@ func (td toDomain) transformSpan(jSpan *jaeger.Span, mProcess *model.Process) *m
 		High: uint64(jSpan.TraceIdHigh),
 		Low:  uint64(jSpan.TraceIdLow),
 	}
-	spanID := model.SpanID(jSpan.SpanId)
 	tags := td.getTags(jSpan.Tags)
 	refs := td.getReferences(jSpan.References)
 	// We no longer store ParentSpanID in the domain model, but the data in Thrift model
@@ -69,7 +68,7 @@ func (td toDomain) transformSpan(jSpan *jaeger.Span, mProcess *model.Process) *m
 	}
 	return &model.Span{
 		TraceID:       traceID,
-		SpanID:        spanID,
+		SpanID:        model.SpanID(jSpan.SpanId),
 		OperationName: jSpan.OperationName,
 		References:    refs,
 		Flags:         model.Flags(jSpan.Flags),

diff --git a/model/span.go b/model/span.go
@@ -66,17 +66,6 @@ func (s *Span) Hash(w io.Writer) (err error) {
 	return enc.Encode(s)
 }
 
-// ParentSpanID returns the ID of a parent span if it exists.
-// It searches the references for a reference pointing to the same trace ID.
-func (s *Span) ParentSpanID() SpanID {
-	for i := range s.References {
-		if s.References[i].TraceID == s.TraceID {
-			return s.References[i].SpanID
-		}
-	}
-	return SpanID(0)
-}
-
 // 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 := KeyValues(s.Tags).FindByKey(string(ext.SpanKind)); ok {
@@ -105,7 +94,20 @@ func (s *Span) NormalizeTimestamps() {
 	}
 }
 
-// ReplaceParentID replaces span ID in the span reference that is considered a parent span.
+// ParentSpanID returns ID of a parent span if it exists.
+// It searches for the first child-of reference pointing to the same trace ID.
+func (s *Span) ParentSpanID() SpanID {
+	for i := range s.References {
+		ref := &s.References[i]
+		if ref.TraceID == s.TraceID && ref.RefType == ChildOf {
+			return ref.SpanID
+		}
+	}
+	return SpanID(0)
+}
+
+// ReplaceParentID replaces span ID in the parent span reference.
+// See also ParentSpanID.
 func (s *Span) ReplaceParentID(newParentID SpanID) {
 	oldParentID := s.ParentSpanID()
 	for i := range s.References {

diff --git a/model/span_test.go b/model/span_test.go
@@ -191,21 +191,46 @@ func TestSpanHash(t *testing.T) {
 	assert.NotEqual(t, codes[0], codes[2])
 }
 
+func TestParentSpanID(t *testing.T) {
+	span := makeSpan(model.String("k", "v"))
+	assert.Equal(t, model.SpanID(123), span.ParentSpanID())
+
+	span.References = []model.SpanRef{
+		model.NewFollowsFromRef(span.TraceID, 777),
+		model.NewChildOfRef(span.TraceID, 888),
+	}
+	assert.Equal(t, model.SpanID(888), span.ParentSpanID())
+
+	span.References = []model.SpanRef{
+		model.NewChildOfRef(model.TraceID{High: 321}, 999),
+	}
+	assert.Equal(t, model.SpanID(0), span.ParentSpanID())
+}
+
+func TestReplaceParentSpanID(t *testing.T) {
+	span := makeSpan(model.String("k", "v"))
+	assert.Equal(t, model.SpanID(123), span.ParentSpanID())
+
+	span.ReplaceParentID(789)
+	assert.Equal(t, model.SpanID(789), span.ParentSpanID())
+
+	span.References = []model.SpanRef{
+		model.NewChildOfRef(model.TraceID{High: 321}, 999),
+	}
+	span.ReplaceParentID(789)
+	assert.Equal(t, model.SpanID(789), span.ParentSpanID())
+}
+
 func makeSpan(someKV model.KeyValue) *model.Span {
+	traceID := model.TraceID{Low: 123}
 	return &model.Span{
-		TraceID:       model.TraceID{Low: 123},
+		TraceID:       traceID,
 		SpanID:        model.SpanID(567),
 		OperationName: "hi",
-		References: []model.SpanRef{
-			{
-				RefType: model.ChildOf,
-				TraceID: model.TraceID{Low: 123},
-				SpanID:  model.SpanID(123),
-			},
-		},
-		StartTime: time.Unix(0, 1000),
-		Duration:  5000,
-		Tags:      model.KeyValues{someKV},
+		References:    []model.SpanRef{model.NewChildOfRef(traceID, 123)},
+		StartTime:     time.Unix(0, 1000),
+		Duration:      5000,
+		Tags:          model.KeyValues{someKV},
 		Logs: []model.Log{
 			{
 				Timestamp: time.Unix(0, 1000),

diff --git a/model/spanref.go b/model/spanref.go
@@ -85,8 +85,9 @@ func MaybeAddParentSpanID(traceID TraceID, parentSpanID SpanID, refs []SpanRef)
 	if parentSpanID == 0 {
 		return refs
 	}
-	for _, r := range refs {
-		if r.SpanID == parentSpanID && r.TraceID == traceID {
+	for i := range refs {
+		r := &refs[i]
+		if r.SpanID == parentSpanID && r.TraceID == traceID && r.RefType == ChildOf {
 			return refs
 		}
 	}
@@ -96,7 +97,7 @@ func MaybeAddParentSpanID(traceID TraceID, parentSpanID SpanID, refs []SpanRef)
 		RefType: ChildOf,
 	}
 	if len(refs) == 0 {
-		return []SpanRef{newRef}
+		return append(refs, newRef)
 	}
 	newRefs := make([]SpanRef, len(refs)+1)
 	newRefs[0] = newRef
@@ -112,3 +113,12 @@ func NewChildOfRef(traceID TraceID, spanID SpanID) SpanRef {
 		SpanID:  spanID,
 	}
 }
+
+// NewFollowsFromRef creates a new follows-from span reference.
+func NewFollowsFromRef(traceID TraceID, spanID SpanID) SpanRef {
+	return SpanRef{
+		RefType: FollowsFrom,
+		TraceID: traceID,
+		SpanID:  spanID,
+	}
+}
diff --git a/model/spanref_test.go b/model/spanref_test.go
@@ -60,3 +60,21 @@ func TestSpanRefTypeToFromJSON(t *testing.T) {
 	err = json.Unmarshal([]byte(`{"refType":"BAD"}`), &sr3)
 	assert.EqualError(t, err, "not a valid SpanRefType string BAD")
 }
+
+func TestMaybeAddParentSpanID(t *testing.T) {
+	span := makeSpan(model.String("k", "v"))
+	assert.Equal(t, model.SpanID(123), span.ParentSpanID())
+
+	span.References = model.MaybeAddParentSpanID(span.TraceID, 0, span.References)
+	assert.Equal(t, model.SpanID(123), span.ParentSpanID())
+
+	span.References = model.MaybeAddParentSpanID(span.TraceID, 123, span.References)
+	assert.Equal(t, model.SpanID(123), span.ParentSpanID())
+
+	span.References = model.MaybeAddParentSpanID(span.TraceID, 123, []model.SpanRef{})
+	assert.Equal(t, model.SpanID(123), span.ParentSpanID())
+
+	span.References = []model.SpanRef{model.NewChildOfRef(model.TraceID{High: 42}, 789)}
+	span.References = model.MaybeAddParentSpanID(span.TraceID, 123, span.References)
+	assert.Equal(t, model.SpanID(123), span.References[0].SpanID, "parent added as first reference")
+}
diff --git a/plugin/storage/cassandra/savetracetest/main.go b/plugin/storage/cassandra/savetracetest/main.go
@@ -107,7 +107,7 @@ func getSomeSpan() *model.Span {
 		TraceID:       traceID,
 		SpanID:        model.SpanID(3),
 		OperationName: "opName",
-		References:    append([]model.SpanRef{model.NewChildOfRef(traceID, 4)}, getReferences()...),
+		References:    model.MaybeAddParentSpanID(traceID, 4, getReferences()),
 		Flags:         model.Flags(uint32(5)),
 		StartTime:     time.Now(),
 		Duration:      50000 * time.Microsecond,